Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Sun, 12 Jan 2014, Russell King - ARM Linux wrote: > This patch makes their types match exactly with x86's definitions of > the same, which is the basic problem: on ARM, they all took "int" values > and returned "int"s, which leads to min() in nobootmem.c complaining. > > arch/arm/include/asm/bitops.h | 54 > +++ > 1 file changed, 44 insertions(+), 10 deletions(-) For the record: Acked-by: Nicolas Pitre The reason why macros were used at the time this was originally written is because gcc used to have issues forwarding the constant nature of a variable down multiple levels of inline functions and __builtin_constant_p() always returned false. But that was quite a long time ago. > diff --git a/arch/arm/include/asm/bitops.h b/arch/arm/include/asm/bitops.h > index e691ec91e4d3..b2e298a90d76 100644 > --- a/arch/arm/include/asm/bitops.h > +++ b/arch/arm/include/asm/bitops.h > @@ -254,25 +254,59 @@ static inline int constant_fls(int x) > } > > /* > - * On ARMv5 and above those functions can be implemented around > - * the clz instruction for much better code efficiency. > + * On ARMv5 and above those functions can be implemented around the > + * clz instruction for much better code efficiency. __clz returns > + * the number of leading zeros, zero input will return 32, and > + * 0x8000 will return 0. > */ > +static inline unsigned int __clz(unsigned int x) > +{ > + unsigned int ret; > + > + asm("clz\t%0, %1" : "=r" (ret) : "r" (x)); > > + return ret; > +} > + > +/* > + * fls() returns zero if the input is zero, otherwise returns the bit > + * position of the last set bit, where the LSB is 1 and MSB is 32. > + */ > static inline int fls(int x) > { > - int ret; > - > if (__builtin_constant_p(x)) > return constant_fls(x); > > - asm("clz\t%0, %1" : "=r" (ret) : "r" (x)); > - ret = 32 - ret; > - return ret; > + return 32 - __clz(x); > +} > + > +/* > + * __fls() returns the bit position of the last bit set, where the > + * LSB is 0 and MSB is 31. Zero input is undefined. > + */ > +static inline unsigned long __fls(unsigned long x) > +{ > + return fls(x) - 1; > +} > + > +/* > + * ffs() returns zero if the input was zero, otherwise returns the bit > + * position of the first set bit, where the LSB is 1 and MSB is 32. > + */ > +static inline int ffs(int x) > +{ > + return fls(x & -x); > +} > + > +/* > + * __ffs() returns the bit position of the first bit set, where the > + * LSB is 0 and MSB is 31. Zero input is undefined. > + */ > +static inline unsigned long __ffs(unsigned long x) > +{ > + return ffs(x) - 1; > } > > -#define __fls(x) (fls(x) - 1) > -#define ffs(x) ({ unsigned long __t = (x); fls(__t & -__t); }) > -#define __ffs(x) (ffs(x) - 1) > #define ffz(x) __ffs( ~(x) ) > > #endif > > > -- > FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation > in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. > Estimate before purchase was "up to 13.2Mbit". > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Sun, 12 Jan 2014, Russell King - ARM Linux wrote: This patch makes their types match exactly with x86's definitions of the same, which is the basic problem: on ARM, they all took int values and returned ints, which leads to min() in nobootmem.c complaining. arch/arm/include/asm/bitops.h | 54 +++ 1 file changed, 44 insertions(+), 10 deletions(-) For the record: Acked-by: Nicolas Pitre n...@linaro.org The reason why macros were used at the time this was originally written is because gcc used to have issues forwarding the constant nature of a variable down multiple levels of inline functions and __builtin_constant_p() always returned false. But that was quite a long time ago. diff --git a/arch/arm/include/asm/bitops.h b/arch/arm/include/asm/bitops.h index e691ec91e4d3..b2e298a90d76 100644 --- a/arch/arm/include/asm/bitops.h +++ b/arch/arm/include/asm/bitops.h @@ -254,25 +254,59 @@ static inline int constant_fls(int x) } /* - * On ARMv5 and above those functions can be implemented around - * the clz instruction for much better code efficiency. + * On ARMv5 and above those functions can be implemented around the + * clz instruction for much better code efficiency. __clz returns + * the number of leading zeros, zero input will return 32, and + * 0x8000 will return 0. */ +static inline unsigned int __clz(unsigned int x) +{ + unsigned int ret; + + asm(clz\t%0, %1 : =r (ret) : r (x)); + return ret; +} + +/* + * fls() returns zero if the input is zero, otherwise returns the bit + * position of the last set bit, where the LSB is 1 and MSB is 32. + */ static inline int fls(int x) { - int ret; - if (__builtin_constant_p(x)) return constant_fls(x); - asm(clz\t%0, %1 : =r (ret) : r (x)); - ret = 32 - ret; - return ret; + return 32 - __clz(x); +} + +/* + * __fls() returns the bit position of the last bit set, where the + * LSB is 0 and MSB is 31. Zero input is undefined. + */ +static inline unsigned long __fls(unsigned long x) +{ + return fls(x) - 1; +} + +/* + * ffs() returns zero if the input was zero, otherwise returns the bit + * position of the first set bit, where the LSB is 1 and MSB is 32. + */ +static inline int ffs(int x) +{ + return fls(x -x); +} + +/* + * __ffs() returns the bit position of the first bit set, where the + * LSB is 0 and MSB is 31. Zero input is undefined. + */ +static inline unsigned long __ffs(unsigned long x) +{ + return ffs(x) - 1; } -#define __fls(x) (fls(x) - 1) -#define ffs(x) ({ unsigned long __t = (x); fls(__t -__t); }) -#define __ffs(x) (ffs(x) - 1) #define ffz(x) __ffs( ~(x) ) #endif -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Monday 13 January 2014 06:33 PM, Russell King - ARM Linux wrote: > On Mon, Jan 13, 2014 at 03:31:28PM -0800, Andrew Morton wrote: >> On Mon, 13 Jan 2014 09:27:44 -0500 Santosh Shilimkar >> wrote: >> It seems to me to be absolutely silly to have code introduce a warning yet push the fix for the warning via a completely different tree... >>> I mixed it up. Sorry. Some how I thought there was some other build >>> configuration thrown the same warning with memblock series and hence >>> suggested the patch to go via Andrew's tree. >> >> Yes, I too had assumed that the warning was caused by the bootmem >> patches in -mm. >> >> But it in fact occurs in Linus's current tree. I'll drop >> mm-arm-fix-arms-__ffs-to-conform-to-avoid-warning-with-no_bootmem.patch >> and I'll assume that rmk will fix this up at an appropriate time. > > Thanks. I'll apply my version and then I can pull Santosh's nobootmem > changes (which I've had a couple of times already) without adding to > the warnings. > Thanks Andrew and Russell for sorting out the patch queue. Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Mon, Jan 13, 2014 at 03:31:28PM -0800, Andrew Morton wrote: > On Mon, 13 Jan 2014 09:27:44 -0500 Santosh Shilimkar > wrote: > > > > It seems to me to be absolutely silly to have code introduce a warning > > > yet push the fix for the warning via a completely different tree... > > > > > I mixed it up. Sorry. Some how I thought there was some other build > > configuration thrown the same warning with memblock series and hence > > suggested the patch to go via Andrew's tree. > > Yes, I too had assumed that the warning was caused by the bootmem > patches in -mm. > > But it in fact occurs in Linus's current tree. I'll drop > mm-arm-fix-arms-__ffs-to-conform-to-avoid-warning-with-no_bootmem.patch > and I'll assume that rmk will fix this up at an appropriate time. Thanks. I'll apply my version and then I can pull Santosh's nobootmem changes (which I've had a couple of times already) without adding to the warnings. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Mon, 13 Jan 2014 09:27:44 -0500 Santosh Shilimkar wrote: > > It seems to me to be absolutely silly to have code introduce a warning > > yet push the fix for the warning via a completely different tree... > > > I mixed it up. Sorry. Some how I thought there was some other build > configuration thrown the same warning with memblock series and hence > suggested the patch to go via Andrew's tree. Yes, I too had assumed that the warning was caused by the bootmem patches in -mm. But it in fact occurs in Linus's current tree. I'll drop mm-arm-fix-arms-__ffs-to-conform-to-avoid-warning-with-no_bootmem.patch and I'll assume that rmk will fix this up at an appropriate time. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Monday 13 January 2014 07:37 AM, Russell King - ARM Linux wrote: > On Sun, Jan 12, 2014 at 10:42:00AM -0500, Santosh Shilimkar wrote: >> On Sunday 12 January 2014 05:59 AM, Russell King - ARM Linux wrote: >>> On Mon, Dec 09, 2013 at 08:02:30PM -0500, Santosh Shilimkar wrote: On Monday 09 December 2013 07:54 PM, Russell King - ARM Linux wrote: > The underlying reason is that - as I've already explained - ARM's __ffs() > differs from other architectures in that it ends up being an int, whereas > almost everyone else is unsigned long. > > The fix is to fix ARMs __ffs() to conform to other architectures. > I was just about to cross-post your reply here. Obviously I didn't think this far when I made $subject fix. So lets ignore the $subject patch which is not correct. Sorry for noise >>> >>> Well, here we are, a month on, and this still remains unfixed despite >>> my comments pointing to what the problem is. So, here's a patch to fix >>> this problem the correct way. I took the time to add some comments to >>> these functions as I find that I wonder about their return values, and >>> these comments make the patch a little larger than it otherwise would be. >>> >> The $subject warning fix [1] is already picked by Andrew with your ack >> and its in his queue [2] >> >>> This patch makes their types match exactly with x86's definitions of >>> the same, which is the basic problem: on ARM, they all took "int" values >>> and returned "int"s, which leads to min() in nobootmem.c complaining. >>> >> Not sure if you missed the thread but the right fix was picked. Ofcourse >> you do have additional clz optimisation in updated patch and some comments >> on those functions. > > The problem here is that the patch fixing this is going via akpm's tree > (why?) yet you want the code which introduces the warning to be merged > via my tree. > > It seems to me to be absolutely silly to have code introduce a warning > yet push the fix for the warning via a completely different tree... > I mixed it up. Sorry. Some how I thought there was some other build configuration thrown the same warning with memblock series and hence suggested the patch to go via Andrew's tree. Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Sun, Jan 12, 2014 at 10:42:00AM -0500, Santosh Shilimkar wrote: > On Sunday 12 January 2014 05:59 AM, Russell King - ARM Linux wrote: > > On Mon, Dec 09, 2013 at 08:02:30PM -0500, Santosh Shilimkar wrote: > >> On Monday 09 December 2013 07:54 PM, Russell King - ARM Linux wrote: > >>> The underlying reason is that - as I've already explained - ARM's __ffs() > >>> differs from other architectures in that it ends up being an int, whereas > >>> almost everyone else is unsigned long. > >>> > >>> The fix is to fix ARMs __ffs() to conform to other architectures. > >>> > >> I was just about to cross-post your reply here. Obviously I didn't think > >> this far when I made $subject fix. > >> > >> So lets ignore the $subject patch which is not correct. Sorry for noise > > > > Well, here we are, a month on, and this still remains unfixed despite > > my comments pointing to what the problem is. So, here's a patch to fix > > this problem the correct way. I took the time to add some comments to > > these functions as I find that I wonder about their return values, and > > these comments make the patch a little larger than it otherwise would be. > > > The $subject warning fix [1] is already picked by Andrew with your ack > and its in his queue [2] > > > This patch makes their types match exactly with x86's definitions of > > the same, which is the basic problem: on ARM, they all took "int" values > > and returned "int"s, which leads to min() in nobootmem.c complaining. > > > Not sure if you missed the thread but the right fix was picked. Ofcourse > you do have additional clz optimisation in updated patch and some comments > on those functions. The problem here is that the patch fixing this is going via akpm's tree (why?) yet you want the code which introduces the warning to be merged via my tree. It seems to me to be absolutely silly to have code introduce a warning yet push the fix for the warning via a completely different tree... -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Sun, Jan 12, 2014 at 10:42:00AM -0500, Santosh Shilimkar wrote: On Sunday 12 January 2014 05:59 AM, Russell King - ARM Linux wrote: On Mon, Dec 09, 2013 at 08:02:30PM -0500, Santosh Shilimkar wrote: On Monday 09 December 2013 07:54 PM, Russell King - ARM Linux wrote: The underlying reason is that - as I've already explained - ARM's __ffs() differs from other architectures in that it ends up being an int, whereas almost everyone else is unsigned long. The fix is to fix ARMs __ffs() to conform to other architectures. I was just about to cross-post your reply here. Obviously I didn't think this far when I made $subject fix. So lets ignore the $subject patch which is not correct. Sorry for noise Well, here we are, a month on, and this still remains unfixed despite my comments pointing to what the problem is. So, here's a patch to fix this problem the correct way. I took the time to add some comments to these functions as I find that I wonder about their return values, and these comments make the patch a little larger than it otherwise would be. The $subject warning fix [1] is already picked by Andrew with your ack and its in his queue [2] This patch makes their types match exactly with x86's definitions of the same, which is the basic problem: on ARM, they all took int values and returned ints, which leads to min() in nobootmem.c complaining. Not sure if you missed the thread but the right fix was picked. Ofcourse you do have additional clz optimisation in updated patch and some comments on those functions. The problem here is that the patch fixing this is going via akpm's tree (why?) yet you want the code which introduces the warning to be merged via my tree. It seems to me to be absolutely silly to have code introduce a warning yet push the fix for the warning via a completely different tree... -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Monday 13 January 2014 07:37 AM, Russell King - ARM Linux wrote: On Sun, Jan 12, 2014 at 10:42:00AM -0500, Santosh Shilimkar wrote: On Sunday 12 January 2014 05:59 AM, Russell King - ARM Linux wrote: On Mon, Dec 09, 2013 at 08:02:30PM -0500, Santosh Shilimkar wrote: On Monday 09 December 2013 07:54 PM, Russell King - ARM Linux wrote: The underlying reason is that - as I've already explained - ARM's __ffs() differs from other architectures in that it ends up being an int, whereas almost everyone else is unsigned long. The fix is to fix ARMs __ffs() to conform to other architectures. I was just about to cross-post your reply here. Obviously I didn't think this far when I made $subject fix. So lets ignore the $subject patch which is not correct. Sorry for noise Well, here we are, a month on, and this still remains unfixed despite my comments pointing to what the problem is. So, here's a patch to fix this problem the correct way. I took the time to add some comments to these functions as I find that I wonder about their return values, and these comments make the patch a little larger than it otherwise would be. The $subject warning fix [1] is already picked by Andrew with your ack and its in his queue [2] This patch makes their types match exactly with x86's definitions of the same, which is the basic problem: on ARM, they all took int values and returned ints, which leads to min() in nobootmem.c complaining. Not sure if you missed the thread but the right fix was picked. Ofcourse you do have additional clz optimisation in updated patch and some comments on those functions. The problem here is that the patch fixing this is going via akpm's tree (why?) yet you want the code which introduces the warning to be merged via my tree. It seems to me to be absolutely silly to have code introduce a warning yet push the fix for the warning via a completely different tree... I mixed it up. Sorry. Some how I thought there was some other build configuration thrown the same warning with memblock series and hence suggested the patch to go via Andrew's tree. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Mon, 13 Jan 2014 09:27:44 -0500 Santosh Shilimkar santosh.shilim...@ti.com wrote: It seems to me to be absolutely silly to have code introduce a warning yet push the fix for the warning via a completely different tree... I mixed it up. Sorry. Some how I thought there was some other build configuration thrown the same warning with memblock series and hence suggested the patch to go via Andrew's tree. Yes, I too had assumed that the warning was caused by the bootmem patches in -mm. But it in fact occurs in Linus's current tree. I'll drop mm-arm-fix-arms-__ffs-to-conform-to-avoid-warning-with-no_bootmem.patch and I'll assume that rmk will fix this up at an appropriate time. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Mon, Jan 13, 2014 at 03:31:28PM -0800, Andrew Morton wrote: On Mon, 13 Jan 2014 09:27:44 -0500 Santosh Shilimkar santosh.shilim...@ti.com wrote: It seems to me to be absolutely silly to have code introduce a warning yet push the fix for the warning via a completely different tree... I mixed it up. Sorry. Some how I thought there was some other build configuration thrown the same warning with memblock series and hence suggested the patch to go via Andrew's tree. Yes, I too had assumed that the warning was caused by the bootmem patches in -mm. But it in fact occurs in Linus's current tree. I'll drop mm-arm-fix-arms-__ffs-to-conform-to-avoid-warning-with-no_bootmem.patch and I'll assume that rmk will fix this up at an appropriate time. Thanks. I'll apply my version and then I can pull Santosh's nobootmem changes (which I've had a couple of times already) without adding to the warnings. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Monday 13 January 2014 06:33 PM, Russell King - ARM Linux wrote: On Mon, Jan 13, 2014 at 03:31:28PM -0800, Andrew Morton wrote: On Mon, 13 Jan 2014 09:27:44 -0500 Santosh Shilimkar santosh.shilim...@ti.com wrote: It seems to me to be absolutely silly to have code introduce a warning yet push the fix for the warning via a completely different tree... I mixed it up. Sorry. Some how I thought there was some other build configuration thrown the same warning with memblock series and hence suggested the patch to go via Andrew's tree. Yes, I too had assumed that the warning was caused by the bootmem patches in -mm. But it in fact occurs in Linus's current tree. I'll drop mm-arm-fix-arms-__ffs-to-conform-to-avoid-warning-with-no_bootmem.patch and I'll assume that rmk will fix this up at an appropriate time. Thanks. I'll apply my version and then I can pull Santosh's nobootmem changes (which I've had a couple of times already) without adding to the warnings. Thanks Andrew and Russell for sorting out the patch queue. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Sunday 12 January 2014 10:42 AM, Santosh Shilimkar wrote: > On Sunday 12 January 2014 05:59 AM, Russell King - ARM Linux wrote: >> On Mon, Dec 09, 2013 at 08:02:30PM -0500, Santosh Shilimkar wrote: >>> On Monday 09 December 2013 07:54 PM, Russell King - ARM Linux wrote: The underlying reason is that - as I've already explained - ARM's __ffs() differs from other architectures in that it ends up being an int, whereas almost everyone else is unsigned long. The fix is to fix ARMs __ffs() to conform to other architectures. >>> I was just about to cross-post your reply here. Obviously I didn't think >>> this far when I made $subject fix. >>> >>> So lets ignore the $subject patch which is not correct. Sorry for noise >> >> Well, here we are, a month on, and this still remains unfixed despite >> my comments pointing to what the problem is. So, here's a patch to fix >> this problem the correct way. I took the time to add some comments to >> these functions as I find that I wonder about their return values, and >> these comments make the patch a little larger than it otherwise would be. >> > The $subject warning fix [1] is already picked by Andrew with your ack > and its in his queue [2] > >> This patch makes their types match exactly with x86's definitions of >> the same, which is the basic problem: on ARM, they all took "int" values >> and returned "int"s, which leads to min() in nobootmem.c complaining. >> > Not sure if you missed the thread but the right fix was picked. Ofcourse > you do have additional clz optimisation in updated patch and some comments > on those functions. > > Regards, > Santosh > > [1] https://lkml.org/lkml/2013/12/9/811 fixing the link since above was the link to the $subject thread and below is the correct link for updated patch [1] https://lkml.org/lkml/2013/12/20/497 > [2] > http://ozlabs.org/~akpm/mmotm/broken-out/mm-arm-fix-arms-__ffs-to-conform-to-avoid-warning-with-no_bootmem.patch > > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Sunday 12 January 2014 05:59 AM, Russell King - ARM Linux wrote: > On Mon, Dec 09, 2013 at 08:02:30PM -0500, Santosh Shilimkar wrote: >> On Monday 09 December 2013 07:54 PM, Russell King - ARM Linux wrote: >>> The underlying reason is that - as I've already explained - ARM's __ffs() >>> differs from other architectures in that it ends up being an int, whereas >>> almost everyone else is unsigned long. >>> >>> The fix is to fix ARMs __ffs() to conform to other architectures. >>> >> I was just about to cross-post your reply here. Obviously I didn't think >> this far when I made $subject fix. >> >> So lets ignore the $subject patch which is not correct. Sorry for noise > > Well, here we are, a month on, and this still remains unfixed despite > my comments pointing to what the problem is. So, here's a patch to fix > this problem the correct way. I took the time to add some comments to > these functions as I find that I wonder about their return values, and > these comments make the patch a little larger than it otherwise would be. > The $subject warning fix [1] is already picked by Andrew with your ack and its in his queue [2] > This patch makes their types match exactly with x86's definitions of > the same, which is the basic problem: on ARM, they all took "int" values > and returned "int"s, which leads to min() in nobootmem.c complaining. > Not sure if you missed the thread but the right fix was picked. Ofcourse you do have additional clz optimisation in updated patch and some comments on those functions. Regards, Santosh [1] https://lkml.org/lkml/2013/12/9/811 [2] http://ozlabs.org/~akpm/mmotm/broken-out/mm-arm-fix-arms-__ffs-to-conform-to-avoid-warning-with-no_bootmem.patch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Mon, Dec 09, 2013 at 08:02:30PM -0500, Santosh Shilimkar wrote: > On Monday 09 December 2013 07:54 PM, Russell King - ARM Linux wrote: > > The underlying reason is that - as I've already explained - ARM's __ffs() > > differs from other architectures in that it ends up being an int, whereas > > almost everyone else is unsigned long. > > > > The fix is to fix ARMs __ffs() to conform to other architectures. > > > I was just about to cross-post your reply here. Obviously I didn't think > this far when I made $subject fix. > > So lets ignore the $subject patch which is not correct. Sorry for noise Well, here we are, a month on, and this still remains unfixed despite my comments pointing to what the problem is. So, here's a patch to fix this problem the correct way. I took the time to add some comments to these functions as I find that I wonder about their return values, and these comments make the patch a little larger than it otherwise would be. This patch makes their types match exactly with x86's definitions of the same, which is the basic problem: on ARM, they all took "int" values and returned "int"s, which leads to min() in nobootmem.c complaining. arch/arm/include/asm/bitops.h | 54 +++ 1 file changed, 44 insertions(+), 10 deletions(-) diff --git a/arch/arm/include/asm/bitops.h b/arch/arm/include/asm/bitops.h index e691ec91e4d3..b2e298a90d76 100644 --- a/arch/arm/include/asm/bitops.h +++ b/arch/arm/include/asm/bitops.h @@ -254,25 +254,59 @@ static inline int constant_fls(int x) } /* - * On ARMv5 and above those functions can be implemented around - * the clz instruction for much better code efficiency. + * On ARMv5 and above those functions can be implemented around the + * clz instruction for much better code efficiency. __clz returns + * the number of leading zeros, zero input will return 32, and + * 0x8000 will return 0. */ +static inline unsigned int __clz(unsigned int x) +{ + unsigned int ret; + + asm("clz\t%0, %1" : "=r" (ret) : "r" (x)); + return ret; +} + +/* + * fls() returns zero if the input is zero, otherwise returns the bit + * position of the last set bit, where the LSB is 1 and MSB is 32. + */ static inline int fls(int x) { - int ret; - if (__builtin_constant_p(x)) return constant_fls(x); - asm("clz\t%0, %1" : "=r" (ret) : "r" (x)); - ret = 32 - ret; - return ret; + return 32 - __clz(x); +} + +/* + * __fls() returns the bit position of the last bit set, where the + * LSB is 0 and MSB is 31. Zero input is undefined. + */ +static inline unsigned long __fls(unsigned long x) +{ + return fls(x) - 1; +} + +/* + * ffs() returns zero if the input was zero, otherwise returns the bit + * position of the first set bit, where the LSB is 1 and MSB is 32. + */ +static inline int ffs(int x) +{ + return fls(x & -x); +} + +/* + * __ffs() returns the bit position of the first bit set, where the + * LSB is 0 and MSB is 31. Zero input is undefined. + */ +static inline unsigned long __ffs(unsigned long x) +{ + return ffs(x) - 1; } -#define __fls(x) (fls(x) - 1) -#define ffs(x) ({ unsigned long __t = (x); fls(__t & -__t); }) -#define __ffs(x) (ffs(x) - 1) #define ffz(x) __ffs( ~(x) ) #endif -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Mon, Dec 09, 2013 at 08:02:30PM -0500, Santosh Shilimkar wrote: On Monday 09 December 2013 07:54 PM, Russell King - ARM Linux wrote: The underlying reason is that - as I've already explained - ARM's __ffs() differs from other architectures in that it ends up being an int, whereas almost everyone else is unsigned long. The fix is to fix ARMs __ffs() to conform to other architectures. I was just about to cross-post your reply here. Obviously I didn't think this far when I made $subject fix. So lets ignore the $subject patch which is not correct. Sorry for noise Well, here we are, a month on, and this still remains unfixed despite my comments pointing to what the problem is. So, here's a patch to fix this problem the correct way. I took the time to add some comments to these functions as I find that I wonder about their return values, and these comments make the patch a little larger than it otherwise would be. This patch makes their types match exactly with x86's definitions of the same, which is the basic problem: on ARM, they all took int values and returned ints, which leads to min() in nobootmem.c complaining. arch/arm/include/asm/bitops.h | 54 +++ 1 file changed, 44 insertions(+), 10 deletions(-) diff --git a/arch/arm/include/asm/bitops.h b/arch/arm/include/asm/bitops.h index e691ec91e4d3..b2e298a90d76 100644 --- a/arch/arm/include/asm/bitops.h +++ b/arch/arm/include/asm/bitops.h @@ -254,25 +254,59 @@ static inline int constant_fls(int x) } /* - * On ARMv5 and above those functions can be implemented around - * the clz instruction for much better code efficiency. + * On ARMv5 and above those functions can be implemented around the + * clz instruction for much better code efficiency. __clz returns + * the number of leading zeros, zero input will return 32, and + * 0x8000 will return 0. */ +static inline unsigned int __clz(unsigned int x) +{ + unsigned int ret; + + asm(clz\t%0, %1 : =r (ret) : r (x)); + return ret; +} + +/* + * fls() returns zero if the input is zero, otherwise returns the bit + * position of the last set bit, where the LSB is 1 and MSB is 32. + */ static inline int fls(int x) { - int ret; - if (__builtin_constant_p(x)) return constant_fls(x); - asm(clz\t%0, %1 : =r (ret) : r (x)); - ret = 32 - ret; - return ret; + return 32 - __clz(x); +} + +/* + * __fls() returns the bit position of the last bit set, where the + * LSB is 0 and MSB is 31. Zero input is undefined. + */ +static inline unsigned long __fls(unsigned long x) +{ + return fls(x) - 1; +} + +/* + * ffs() returns zero if the input was zero, otherwise returns the bit + * position of the first set bit, where the LSB is 1 and MSB is 32. + */ +static inline int ffs(int x) +{ + return fls(x -x); +} + +/* + * __ffs() returns the bit position of the first bit set, where the + * LSB is 0 and MSB is 31. Zero input is undefined. + */ +static inline unsigned long __ffs(unsigned long x) +{ + return ffs(x) - 1; } -#define __fls(x) (fls(x) - 1) -#define ffs(x) ({ unsigned long __t = (x); fls(__t -__t); }) -#define __ffs(x) (ffs(x) - 1) #define ffz(x) __ffs( ~(x) ) #endif -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Sunday 12 January 2014 05:59 AM, Russell King - ARM Linux wrote: On Mon, Dec 09, 2013 at 08:02:30PM -0500, Santosh Shilimkar wrote: On Monday 09 December 2013 07:54 PM, Russell King - ARM Linux wrote: The underlying reason is that - as I've already explained - ARM's __ffs() differs from other architectures in that it ends up being an int, whereas almost everyone else is unsigned long. The fix is to fix ARMs __ffs() to conform to other architectures. I was just about to cross-post your reply here. Obviously I didn't think this far when I made $subject fix. So lets ignore the $subject patch which is not correct. Sorry for noise Well, here we are, a month on, and this still remains unfixed despite my comments pointing to what the problem is. So, here's a patch to fix this problem the correct way. I took the time to add some comments to these functions as I find that I wonder about their return values, and these comments make the patch a little larger than it otherwise would be. The $subject warning fix [1] is already picked by Andrew with your ack and its in his queue [2] This patch makes their types match exactly with x86's definitions of the same, which is the basic problem: on ARM, they all took int values and returned ints, which leads to min() in nobootmem.c complaining. Not sure if you missed the thread but the right fix was picked. Ofcourse you do have additional clz optimisation in updated patch and some comments on those functions. Regards, Santosh [1] https://lkml.org/lkml/2013/12/9/811 [2] http://ozlabs.org/~akpm/mmotm/broken-out/mm-arm-fix-arms-__ffs-to-conform-to-avoid-warning-with-no_bootmem.patch -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Sunday 12 January 2014 10:42 AM, Santosh Shilimkar wrote: On Sunday 12 January 2014 05:59 AM, Russell King - ARM Linux wrote: On Mon, Dec 09, 2013 at 08:02:30PM -0500, Santosh Shilimkar wrote: On Monday 09 December 2013 07:54 PM, Russell King - ARM Linux wrote: The underlying reason is that - as I've already explained - ARM's __ffs() differs from other architectures in that it ends up being an int, whereas almost everyone else is unsigned long. The fix is to fix ARMs __ffs() to conform to other architectures. I was just about to cross-post your reply here. Obviously I didn't think this far when I made $subject fix. So lets ignore the $subject patch which is not correct. Sorry for noise Well, here we are, a month on, and this still remains unfixed despite my comments pointing to what the problem is. So, here's a patch to fix this problem the correct way. I took the time to add some comments to these functions as I find that I wonder about their return values, and these comments make the patch a little larger than it otherwise would be. The $subject warning fix [1] is already picked by Andrew with your ack and its in his queue [2] This patch makes their types match exactly with x86's definitions of the same, which is the basic problem: on ARM, they all took int values and returned ints, which leads to min() in nobootmem.c complaining. Not sure if you missed the thread but the right fix was picked. Ofcourse you do have additional clz optimisation in updated patch and some comments on those functions. Regards, Santosh [1] https://lkml.org/lkml/2013/12/9/811 fixing the link since above was the link to the $subject thread and below is the correct link for updated patch [1] https://lkml.org/lkml/2013/12/20/497 [2] http://ozlabs.org/~akpm/mmotm/broken-out/mm-arm-fix-arms-__ffs-to-conform-to-avoid-warning-with-no_bootmem.patch ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Monday 09 December 2013 07:54 PM, Russell King - ARM Linux wrote: > On Mon, Dec 09, 2013 at 04:50:44PM -0800, Andrew Morton wrote: >> On Mon, 25 Nov 2013 08:57:54 -0500 Santosh Shilimkar >> wrote: >> >>> On Sunday 24 November 2013 10:14 AM, Sergei Shtylyov wrote: >>>> Hello. >>>> >>>> On 24-11-2013 3:28, Santosh Shilimkar wrote: >>>> >>>>> Building ARM with NO_BOOTMEM generates below warning. Using min_t >>>> >>>>Where is that below? :-) >>>> >>> Damn.. Posted a wrong version of the patch ;-( >>> Here is the one with warning message included. >>> >>> >From 571dfdf4cf8ac7dfd50bd9b7519717c42824f1c3 Mon Sep 17 00:00:00 2001 >>> From: Santosh Shilimkar >>> Date: Sat, 23 Nov 2013 18:16:50 -0500 >>> Subject: [PATCH] mm: nobootmem: avoid type warning about alignment value >>> >>> Building ARM with NO_BOOTMEM generates below warning. >>> >>> mm/nobootmem.c: In function _free_pages_memory___: >>> mm/nobootmem.c:88:11: warning: comparison of distinct pointer types lacks a >>> cast >>> >>> Using min_t to find the correct alignment avoids the warning. >>> >>> Cc: Tejun Heo >>> Cc: Andrew Morton >>> Signed-off-by: Santosh Shilimkar >>> --- >>> mm/nobootmem.c |2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/mm/nobootmem.c b/mm/nobootmem.c >>> index 2c254d3..8954e43 100644 >>> --- a/mm/nobootmem.c >>> +++ b/mm/nobootmem.c >>> @@ -85,7 +85,7 @@ static void __init __free_pages_memory(unsigned long >>> start, unsigned long end) >>> int order; >>> >>> while (start < end) { >>> - order = min(MAX_ORDER - 1UL, __ffs(start)); >>> + order = min_t(size_t, MAX_ORDER - 1UL, __ffs(start)); >>> >> >> size_t makes no sense. Neither `order', `MAX_ORDER', 1UL nor __ffs() >> have that type. >> >> min() warnings often indicate that the chosen types are inappropriate, >> and suppressing them with min_t() should be a last resort. >> >> MAX_ORDER-1UL has type `unsigned long' (yes?) and __ffs() should return >> unsigned long (except arch/arc which decided to be different). >> >> Why does it warn? What's the underlying reason? > > The underlying reason is that - as I've already explained - ARM's __ffs() > differs from other architectures in that it ends up being an int, whereas > almost everyone else is unsigned long. > > The fix is to fix ARMs __ffs() to conform to other architectures. > I was just about to cross-post your reply here. Obviously I didn't think this far when I made $subject fix. So lets ignore the $subject patch which is not correct. Sorry for noise Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Mon, Dec 09, 2013 at 04:50:44PM -0800, Andrew Morton wrote: > On Mon, 25 Nov 2013 08:57:54 -0500 Santosh Shilimkar > wrote: > > > On Sunday 24 November 2013 10:14 AM, Sergei Shtylyov wrote: > > > Hello. > > > > > > On 24-11-2013 3:28, Santosh Shilimkar wrote: > > > > > >> Building ARM with NO_BOOTMEM generates below warning. Using min_t > > > > > >Where is that below? :-) > > > > > Damn.. Posted a wrong version of the patch ;-( > > Here is the one with warning message included. > > > > >From 571dfdf4cf8ac7dfd50bd9b7519717c42824f1c3 Mon Sep 17 00:00:00 2001 > > From: Santosh Shilimkar > > Date: Sat, 23 Nov 2013 18:16:50 -0500 > > Subject: [PATCH] mm: nobootmem: avoid type warning about alignment value > > > > Building ARM with NO_BOOTMEM generates below warning. > > > > mm/nobootmem.c: In function _free_pages_memory___: > > mm/nobootmem.c:88:11: warning: comparison of distinct pointer types lacks a > > cast > > > > Using min_t to find the correct alignment avoids the warning. > > > > Cc: Tejun Heo > > Cc: Andrew Morton > > Signed-off-by: Santosh Shilimkar > > --- > > mm/nobootmem.c |2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/mm/nobootmem.c b/mm/nobootmem.c > > index 2c254d3..8954e43 100644 > > --- a/mm/nobootmem.c > > +++ b/mm/nobootmem.c > > @@ -85,7 +85,7 @@ static void __init __free_pages_memory(unsigned long > > start, unsigned long end) > > int order; > > > > while (start < end) { > > - order = min(MAX_ORDER - 1UL, __ffs(start)); > > + order = min_t(size_t, MAX_ORDER - 1UL, __ffs(start)); > > > > size_t makes no sense. Neither `order', `MAX_ORDER', 1UL nor __ffs() > have that type. > > min() warnings often indicate that the chosen types are inappropriate, > and suppressing them with min_t() should be a last resort. > > MAX_ORDER-1UL has type `unsigned long' (yes?) and __ffs() should return > unsigned long (except arch/arc which decided to be different). > > Why does it warn? What's the underlying reason? The underlying reason is that - as I've already explained - ARM's __ffs() differs from other architectures in that it ends up being an int, whereas almost everyone else is unsigned long. The fix is to fix ARMs __ffs() to conform to other architectures. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Mon, 25 Nov 2013 08:57:54 -0500 Santosh Shilimkar wrote: > On Sunday 24 November 2013 10:14 AM, Sergei Shtylyov wrote: > > Hello. > > > > On 24-11-2013 3:28, Santosh Shilimkar wrote: > > > >> Building ARM with NO_BOOTMEM generates below warning. Using min_t > > > >Where is that below? :-) > > > Damn.. Posted a wrong version of the patch ;-( > Here is the one with warning message included. > > >From 571dfdf4cf8ac7dfd50bd9b7519717c42824f1c3 Mon Sep 17 00:00:00 2001 > From: Santosh Shilimkar > Date: Sat, 23 Nov 2013 18:16:50 -0500 > Subject: [PATCH] mm: nobootmem: avoid type warning about alignment value > > Building ARM with NO_BOOTMEM generates below warning. > > mm/nobootmem.c: In function _free_pages_memory___: > mm/nobootmem.c:88:11: warning: comparison of distinct pointer types lacks a > cast > > Using min_t to find the correct alignment avoids the warning. > > Cc: Tejun Heo > Cc: Andrew Morton > Signed-off-by: Santosh Shilimkar > --- > mm/nobootmem.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/nobootmem.c b/mm/nobootmem.c > index 2c254d3..8954e43 100644 > --- a/mm/nobootmem.c > +++ b/mm/nobootmem.c > @@ -85,7 +85,7 @@ static void __init __free_pages_memory(unsigned long start, > unsigned long end) > int order; > > while (start < end) { > - order = min(MAX_ORDER - 1UL, __ffs(start)); > + order = min_t(size_t, MAX_ORDER - 1UL, __ffs(start)); > size_t makes no sense. Neither `order', `MAX_ORDER', 1UL nor __ffs() have that type. min() warnings often indicate that the chosen types are inappropriate, and suppressing them with min_t() should be a last resort. MAX_ORDER-1UL has type `unsigned long' (yes?) and __ffs() should return unsigned long (except arch/arc which decided to be different). Why does it warn? What's the underlying reason? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
Andrew, On Monday 25 November 2013 10:56 AM, Tejun Heo wrote: > On Mon, Nov 25, 2013 at 08:57:54AM -0500, Santosh Shilimkar wrote: >> On Sunday 24 November 2013 10:14 AM, Sergei Shtylyov wrote: >>> Hello. >>> >>> On 24-11-2013 3:28, Santosh Shilimkar wrote: >>> >>>> Building ARM with NO_BOOTMEM generates below warning. Using min_t >>> >>>Where is that below? :-) >>> >> Damn.. Posted a wrong version of the patch ;-( >> Here is the one with warning message included. >> >> From 571dfdf4cf8ac7dfd50bd9b7519717c42824f1c3 Mon Sep 17 00:00:00 2001 >> From: Santosh Shilimkar >> Date: Sat, 23 Nov 2013 18:16:50 -0500 >> Subject: [PATCH] mm: nobootmem: avoid type warning about alignment value >> >> Building ARM with NO_BOOTMEM generates below warning. >> >> mm/nobootmem.c: In function ‘__free_pages_memory’: >> mm/nobootmem.c:88:11: warning: comparison of distinct pointer types lacks a >> cast >> >> Using min_t to find the correct alignment avoids the warning. >> >> Cc: Tejun Heo >> Cc: Andrew Morton >> Signed-off-by: Santosh Shilimkar > > Acked-by: Tejun Heo > Can you please this warning fix as well in your mm tree ? Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
Andrew, On Monday 25 November 2013 10:56 AM, Tejun Heo wrote: On Mon, Nov 25, 2013 at 08:57:54AM -0500, Santosh Shilimkar wrote: On Sunday 24 November 2013 10:14 AM, Sergei Shtylyov wrote: Hello. On 24-11-2013 3:28, Santosh Shilimkar wrote: Building ARM with NO_BOOTMEM generates below warning. Using min_t Where is that below? :-) Damn.. Posted a wrong version of the patch ;-( Here is the one with warning message included. From 571dfdf4cf8ac7dfd50bd9b7519717c42824f1c3 Mon Sep 17 00:00:00 2001 From: Santosh Shilimkar santosh.shilim...@ti.com Date: Sat, 23 Nov 2013 18:16:50 -0500 Subject: [PATCH] mm: nobootmem: avoid type warning about alignment value Building ARM with NO_BOOTMEM generates below warning. mm/nobootmem.c: In function ‘__free_pages_memory’: mm/nobootmem.c:88:11: warning: comparison of distinct pointer types lacks a cast Using min_t to find the correct alignment avoids the warning. Cc: Tejun Heo t...@kernel.org Cc: Andrew Morton a...@linux-foundation.org Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com Acked-by: Tejun Heo t...@kernel.org Can you please this warning fix as well in your mm tree ? Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Mon, 25 Nov 2013 08:57:54 -0500 Santosh Shilimkar santosh.shilim...@ti.com wrote: On Sunday 24 November 2013 10:14 AM, Sergei Shtylyov wrote: Hello. On 24-11-2013 3:28, Santosh Shilimkar wrote: Building ARM with NO_BOOTMEM generates below warning. Using min_t Where is that below? :-) Damn.. Posted a wrong version of the patch ;-( Here is the one with warning message included. From 571dfdf4cf8ac7dfd50bd9b7519717c42824f1c3 Mon Sep 17 00:00:00 2001 From: Santosh Shilimkar santosh.shilim...@ti.com Date: Sat, 23 Nov 2013 18:16:50 -0500 Subject: [PATCH] mm: nobootmem: avoid type warning about alignment value Building ARM with NO_BOOTMEM generates below warning. mm/nobootmem.c: In function _free_pages_memory___: mm/nobootmem.c:88:11: warning: comparison of distinct pointer types lacks a cast Using min_t to find the correct alignment avoids the warning. Cc: Tejun Heo t...@kernel.org Cc: Andrew Morton a...@linux-foundation.org Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com --- mm/nobootmem.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/nobootmem.c b/mm/nobootmem.c index 2c254d3..8954e43 100644 --- a/mm/nobootmem.c +++ b/mm/nobootmem.c @@ -85,7 +85,7 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end) int order; while (start end) { - order = min(MAX_ORDER - 1UL, __ffs(start)); + order = min_t(size_t, MAX_ORDER - 1UL, __ffs(start)); size_t makes no sense. Neither `order', `MAX_ORDER', 1UL nor __ffs() have that type. min() warnings often indicate that the chosen types are inappropriate, and suppressing them with min_t() should be a last resort. MAX_ORDER-1UL has type `unsigned long' (yes?) and __ffs() should return unsigned long (except arch/arc which decided to be different). Why does it warn? What's the underlying reason? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Mon, Dec 09, 2013 at 04:50:44PM -0800, Andrew Morton wrote: On Mon, 25 Nov 2013 08:57:54 -0500 Santosh Shilimkar santosh.shilim...@ti.com wrote: On Sunday 24 November 2013 10:14 AM, Sergei Shtylyov wrote: Hello. On 24-11-2013 3:28, Santosh Shilimkar wrote: Building ARM with NO_BOOTMEM generates below warning. Using min_t Where is that below? :-) Damn.. Posted a wrong version of the patch ;-( Here is the one with warning message included. From 571dfdf4cf8ac7dfd50bd9b7519717c42824f1c3 Mon Sep 17 00:00:00 2001 From: Santosh Shilimkar santosh.shilim...@ti.com Date: Sat, 23 Nov 2013 18:16:50 -0500 Subject: [PATCH] mm: nobootmem: avoid type warning about alignment value Building ARM with NO_BOOTMEM generates below warning. mm/nobootmem.c: In function _free_pages_memory___: mm/nobootmem.c:88:11: warning: comparison of distinct pointer types lacks a cast Using min_t to find the correct alignment avoids the warning. Cc: Tejun Heo t...@kernel.org Cc: Andrew Morton a...@linux-foundation.org Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com --- mm/nobootmem.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/nobootmem.c b/mm/nobootmem.c index 2c254d3..8954e43 100644 --- a/mm/nobootmem.c +++ b/mm/nobootmem.c @@ -85,7 +85,7 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end) int order; while (start end) { - order = min(MAX_ORDER - 1UL, __ffs(start)); + order = min_t(size_t, MAX_ORDER - 1UL, __ffs(start)); size_t makes no sense. Neither `order', `MAX_ORDER', 1UL nor __ffs() have that type. min() warnings often indicate that the chosen types are inappropriate, and suppressing them with min_t() should be a last resort. MAX_ORDER-1UL has type `unsigned long' (yes?) and __ffs() should return unsigned long (except arch/arc which decided to be different). Why does it warn? What's the underlying reason? The underlying reason is that - as I've already explained - ARM's __ffs() differs from other architectures in that it ends up being an int, whereas almost everyone else is unsigned long. The fix is to fix ARMs __ffs() to conform to other architectures. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Monday 09 December 2013 07:54 PM, Russell King - ARM Linux wrote: On Mon, Dec 09, 2013 at 04:50:44PM -0800, Andrew Morton wrote: On Mon, 25 Nov 2013 08:57:54 -0500 Santosh Shilimkar santosh.shilim...@ti.com wrote: On Sunday 24 November 2013 10:14 AM, Sergei Shtylyov wrote: Hello. On 24-11-2013 3:28, Santosh Shilimkar wrote: Building ARM with NO_BOOTMEM generates below warning. Using min_t Where is that below? :-) Damn.. Posted a wrong version of the patch ;-( Here is the one with warning message included. From 571dfdf4cf8ac7dfd50bd9b7519717c42824f1c3 Mon Sep 17 00:00:00 2001 From: Santosh Shilimkar santosh.shilim...@ti.com Date: Sat, 23 Nov 2013 18:16:50 -0500 Subject: [PATCH] mm: nobootmem: avoid type warning about alignment value Building ARM with NO_BOOTMEM generates below warning. mm/nobootmem.c: In function _free_pages_memory___: mm/nobootmem.c:88:11: warning: comparison of distinct pointer types lacks a cast Using min_t to find the correct alignment avoids the warning. Cc: Tejun Heo t...@kernel.org Cc: Andrew Morton a...@linux-foundation.org Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com --- mm/nobootmem.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/nobootmem.c b/mm/nobootmem.c index 2c254d3..8954e43 100644 --- a/mm/nobootmem.c +++ b/mm/nobootmem.c @@ -85,7 +85,7 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end) int order; while (start end) { - order = min(MAX_ORDER - 1UL, __ffs(start)); + order = min_t(size_t, MAX_ORDER - 1UL, __ffs(start)); size_t makes no sense. Neither `order', `MAX_ORDER', 1UL nor __ffs() have that type. min() warnings often indicate that the chosen types are inappropriate, and suppressing them with min_t() should be a last resort. MAX_ORDER-1UL has type `unsigned long' (yes?) and __ffs() should return unsigned long (except arch/arc which decided to be different). Why does it warn? What's the underlying reason? The underlying reason is that - as I've already explained - ARM's __ffs() differs from other architectures in that it ends up being an int, whereas almost everyone else is unsigned long. The fix is to fix ARMs __ffs() to conform to other architectures. I was just about to cross-post your reply here. Obviously I didn't think this far when I made $subject fix. So lets ignore the $subject patch which is not correct. Sorry for noise Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Mon, Nov 25, 2013 at 08:57:54AM -0500, Santosh Shilimkar wrote: > On Sunday 24 November 2013 10:14 AM, Sergei Shtylyov wrote: > > Hello. > > > > On 24-11-2013 3:28, Santosh Shilimkar wrote: > > > >> Building ARM with NO_BOOTMEM generates below warning. Using min_t > > > >Where is that below? :-) > > > Damn.. Posted a wrong version of the patch ;-( > Here is the one with warning message included. > > From 571dfdf4cf8ac7dfd50bd9b7519717c42824f1c3 Mon Sep 17 00:00:00 2001 > From: Santosh Shilimkar > Date: Sat, 23 Nov 2013 18:16:50 -0500 > Subject: [PATCH] mm: nobootmem: avoid type warning about alignment value > > Building ARM with NO_BOOTMEM generates below warning. > > mm/nobootmem.c: In function ‘__free_pages_memory’: > mm/nobootmem.c:88:11: warning: comparison of distinct pointer types lacks a > cast > > Using min_t to find the correct alignment avoids the warning. > > Cc: Tejun Heo > Cc: Andrew Morton > Signed-off-by: Santosh Shilimkar Acked-by: Tejun Heo Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Sunday 24 November 2013 10:14 AM, Sergei Shtylyov wrote: > Hello. > > On 24-11-2013 3:28, Santosh Shilimkar wrote: > >> Building ARM with NO_BOOTMEM generates below warning. Using min_t > >Where is that below? :-) > Damn.. Posted a wrong version of the patch ;-( Here is the one with warning message included. >From 571dfdf4cf8ac7dfd50bd9b7519717c42824f1c3 Mon Sep 17 00:00:00 2001 From: Santosh Shilimkar Date: Sat, 23 Nov 2013 18:16:50 -0500 Subject: [PATCH] mm: nobootmem: avoid type warning about alignment value Building ARM with NO_BOOTMEM generates below warning. mm/nobootmem.c: In function ‘__free_pages_memory’: mm/nobootmem.c:88:11: warning: comparison of distinct pointer types lacks a cast Using min_t to find the correct alignment avoids the warning. Cc: Tejun Heo Cc: Andrew Morton Signed-off-by: Santosh Shilimkar --- mm/nobootmem.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/nobootmem.c b/mm/nobootmem.c index 2c254d3..8954e43 100644 --- a/mm/nobootmem.c +++ b/mm/nobootmem.c @@ -85,7 +85,7 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end) int order; while (start < end) { - order = min(MAX_ORDER - 1UL, __ffs(start)); + order = min_t(size_t, MAX_ORDER - 1UL, __ffs(start)); while (start + (1UL << order) > end) order--; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Sunday 24 November 2013 10:14 AM, Sergei Shtylyov wrote: Hello. On 24-11-2013 3:28, Santosh Shilimkar wrote: Building ARM with NO_BOOTMEM generates below warning. Using min_t Where is that below? :-) Damn.. Posted a wrong version of the patch ;-( Here is the one with warning message included. From 571dfdf4cf8ac7dfd50bd9b7519717c42824f1c3 Mon Sep 17 00:00:00 2001 From: Santosh Shilimkar santosh.shilim...@ti.com Date: Sat, 23 Nov 2013 18:16:50 -0500 Subject: [PATCH] mm: nobootmem: avoid type warning about alignment value Building ARM with NO_BOOTMEM generates below warning. mm/nobootmem.c: In function ‘__free_pages_memory’: mm/nobootmem.c:88:11: warning: comparison of distinct pointer types lacks a cast Using min_t to find the correct alignment avoids the warning. Cc: Tejun Heo t...@kernel.org Cc: Andrew Morton a...@linux-foundation.org Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com --- mm/nobootmem.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/nobootmem.c b/mm/nobootmem.c index 2c254d3..8954e43 100644 --- a/mm/nobootmem.c +++ b/mm/nobootmem.c @@ -85,7 +85,7 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end) int order; while (start end) { - order = min(MAX_ORDER - 1UL, __ffs(start)); + order = min_t(size_t, MAX_ORDER - 1UL, __ffs(start)); while (start + (1UL order) end) order--; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
On Mon, Nov 25, 2013 at 08:57:54AM -0500, Santosh Shilimkar wrote: On Sunday 24 November 2013 10:14 AM, Sergei Shtylyov wrote: Hello. On 24-11-2013 3:28, Santosh Shilimkar wrote: Building ARM with NO_BOOTMEM generates below warning. Using min_t Where is that below? :-) Damn.. Posted a wrong version of the patch ;-( Here is the one with warning message included. From 571dfdf4cf8ac7dfd50bd9b7519717c42824f1c3 Mon Sep 17 00:00:00 2001 From: Santosh Shilimkar santosh.shilim...@ti.com Date: Sat, 23 Nov 2013 18:16:50 -0500 Subject: [PATCH] mm: nobootmem: avoid type warning about alignment value Building ARM with NO_BOOTMEM generates below warning. mm/nobootmem.c: In function ‘__free_pages_memory’: mm/nobootmem.c:88:11: warning: comparison of distinct pointer types lacks a cast Using min_t to find the correct alignment avoids the warning. Cc: Tejun Heo t...@kernel.org Cc: Andrew Morton a...@linux-foundation.org Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com Acked-by: Tejun Heo t...@kernel.org Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
Hello. On 24-11-2013 3:28, Santosh Shilimkar wrote: Building ARM with NO_BOOTMEM generates below warning. Using min_t Where is that below? :-) to find the correct alignment avoids the warning. Cc: Tejun Heo Cc: Andrew Morton Signed-off-by: Santosh Shilimkar WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: nobootmem: avoid type warning about alignment value
Hello. On 24-11-2013 3:28, Santosh Shilimkar wrote: Building ARM with NO_BOOTMEM generates below warning. Using min_t Where is that below? :-) to find the correct alignment avoids the warning. Cc: Tejun Heo t...@kernel.org Cc: Andrew Morton a...@linux-foundation.org Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] mm: nobootmem: avoid type warning about alignment value
Building ARM with NO_BOOTMEM generates below warning. Using min_t to find the correct alignment avoids the warning. Cc: Tejun Heo Cc: Andrew Morton Signed-off-by: Santosh Shilimkar --- mm/nobootmem.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/nobootmem.c b/mm/nobootmem.c index 2c254d3..8954e43 100644 --- a/mm/nobootmem.c +++ b/mm/nobootmem.c @@ -85,7 +85,7 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end) int order; while (start < end) { - order = min(MAX_ORDER - 1UL, __ffs(start)); + order = min_t(size_t, MAX_ORDER - 1UL, __ffs(start)); while (start + (1UL << order) > end) order--; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] mm: nobootmem: avoid type warning about alignment value
Building ARM with NO_BOOTMEM generates below warning. Using min_t to find the correct alignment avoids the warning. Cc: Tejun Heo t...@kernel.org Cc: Andrew Morton a...@linux-foundation.org Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com --- mm/nobootmem.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/nobootmem.c b/mm/nobootmem.c index 2c254d3..8954e43 100644 --- a/mm/nobootmem.c +++ b/mm/nobootmem.c @@ -85,7 +85,7 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end) int order; while (start end) { - order = min(MAX_ORDER - 1UL, __ffs(start)); + order = min_t(size_t, MAX_ORDER - 1UL, __ffs(start)); while (start + (1UL order) end) order--; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/