Re: b43: do not stack-allocate pio rx/tx header and tail buffers

From: Albert Herranz
Date: Fri Oct 09 2009 - 14:54:20 EST


Michael Buesch wrote:
>> The reason I posted the initial patch for review was because you kind of told me [2].
>>
>> [20:06] <mb_> Anyway, I'm not going to fix this. If you need it fixed, please send patches
>
> Yeah, but that doesn't mean that either hack is acceptable. It just means that my time is limited
> and I added this non-issue (which I still think it is) to the very bottom of my priority list.

The patch got elaborated and discussed publicly (successfully or not) by following your instructions.

>> My point here is that there's no reason for such strong words concerning the quality of the patch code. It's really not that bad for such wording.
>> I'm sure that if the patch was really crap it would have been immediately NAK'ed by you or by any sane maintainer.
>
> It _was_ immediately NAK'ed by me. I did not know that I need to NAK
> every single new version of a patch explicitely.
> I thought NAK-ing a patch would put it into some special state that only an explicit ACK could
> take it out of.

We all sure had a communication issue here.

What you thought it was an (implicit) NAK for the _initial_ version of the patch, others took that as "fix-those-concerns-and-its-fine".
And the expressed concerns where addressed later in the merged patch, sub-optimally (not crappily).

Looking at your new "[PATCH] b43: Optimize PIO scratchbuffer usage" to address the changes introduced by the merged patch, the merged solution is not that _blatantly_ far from your solution.
The patch would have probably got there in one iteration if you have had the chance again to express your new concerns about v2.

I'm sure we can avoid this in the future by being a bit more explicit.

Thanks,
Albert

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/