On Tue, Jul 17, 2012 at 12:36 PM, Markus Armbruster <arm...@redhat.com> wrote: > Peter Maydell <peter.mayd...@linaro.org> writes: > >> On 14 July 2012 13:34, Blue Swirl <blauwir...@gmail.com> wrote: >>> bitops.h uses inconsistently 'unsigned long' and 'int' for bit numbers. >>> >>> Unify to 'unsigned long' because it generates better code on x86_64. >>> Adjust asserts accordingly. >> >> Still disagree with this patch, for the record. > > So do I. > > Changing tried-and-true code for some unproven performance gain is a bad > idea.
No it isn't and that is not the case. > > In this particular case, it additionally deviates from the code's > source. I'm getting a strong feeling that it's a bad idea to reuse any Linux kernel sources since they are seen as divine and untouchable, unlike for example BSD queue macros. Perhaps BSD have also bit field ops which we could use instead? There's also the licensing problem with GPLv2only in some cases. > > If you feel your patch is a worthwhile improvement, please take it to > LKML, so the kernel and future borrowers of this code can profit. > Copying free code without at least trying to contribute improvements > back to the source isn't proper. The original kernel functions use 'unsigned long'. The bit field functions introduced by Peter use 'int' but those are not in the kernel tree, so there's nothing to fix (except the two first hunks) from the kernel point of view. Also 'volatile' makes sense when kernel is banging HW registers.
