Fixes warnings about shifting unsigned literals being undefined
behavior.
Signed-off-by: Nick Desaulniers <[email protected]>
---
mm/zsmalloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 685049a..5d31458 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1056,7 +1056,7 @@ static void init_zspage(struct size_class *class, struct zspage *zspage)
* Reset OBJ_TAG_BITS bit to last link to tell
* whether it's allocated object or not.
*/
- link->next = -1 << OBJ_TAG_BITS;
+ link->next = -1U << OBJ_TAG_BITS;
}
kunmap_atomic(vaddr);
page = next_page;
--
2.7.4
On Sat, Dec 23, 2017 at 09:33:40PM -0500, Nick Desaulniers wrote:
> Fixes warnings about shifting unsigned literals being undefined
> behavior.
Do you mean signed literals?
> */
> - link->next = -1 << OBJ_TAG_BITS;
> + link->next = -1U << OBJ_TAG_BITS;
> }
I don't understand what -1U means. Seems like a contradiction in terms,
a negative unsigned number. Is this supposed to be ~0U?
On Sat, Dec 23, 2017 at 10:24 PM, Matthew Wilcox <[email protected]> wrote:
> On Sat, Dec 23, 2017 at 09:33:40PM -0500, Nick Desaulniers wrote:
>> Fixes warnings about shifting unsigned literals being undefined
>> behavior.
>
> Do you mean signed literals?
A sorry, s/unsigned/negative signed/g. The warning is:
mm/zsmalloc.c:1059:20: warning: shifting a negative signed value is undefined
[-Wshift-negative-value]
link->next = -1 << OBJ_TAG_BITS;
~~ ^
>
>> */
>> - link->next = -1 << OBJ_TAG_BITS;
>> + link->next = -1U << OBJ_TAG_BITS;
>> }
>
> I don't understand what -1U means. Seems like a contradiction in terms,
> a negative unsigned number. Is this supposed to be ~0U?
$ ag \\-1U[^L]
The code base is full of that literal. I think of it as:
(unsigned) -1
Hello,
What are the next steps for this patch?
Note: the MAINTAINERS file does not contain a T: (tree) entry for
ZSMALLOC, so I can't check to see if this has already been merged or
not. Unless you work out of the mm tree?
Hello,
Sorry for the delay. I have missed this until now. ;-(
On Sun, Dec 24, 2017 at 11:33 AM, Nick Desaulniers
<[email protected]> wrote:
> Fixes warnings about shifting unsigned literals being undefined
> behavior.
>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> mm/zsmalloc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 685049a..5d31458 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -1056,7 +1056,7 @@ static void init_zspage(struct size_class *class, struct zspage *zspage)
> * Reset OBJ_TAG_BITS bit to last link to tell
> * whether it's allocated object or not.
> */
> - link->next = -1 << OBJ_TAG_BITS;
> + link->next = -1U << OBJ_TAG_BITS;
-1UL?
Please, resend it with including Andrew Morton
<[email protected]> who merges zsmalloc patch into his tree.
Thanks.
On Sun, Jan 7, 2018 at 5:04 PM, Minchan Kim <[email protected]> wrote:
>> - link->next = -1 << OBJ_TAG_BITS;
>> + link->next = -1U << OBJ_TAG_BITS;
>
> -1UL?
Oh, boy, shouldn't be rather GENMASK() / GENMASK_ULL() in a way how
it's done, for example, here:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/commit/?h=for-next&id=d2b3c353595a855794f8b9df5b5bdbe8deb0c413
--
With Best Regards,
Andy Shevchenko
On Sun, Jan 7, 2018 at 7:04 AM, Minchan Kim <[email protected]> wrote:
> Sorry for the delay. I have missed this until now. ;-(
No worries, figured patches would need a post holiday bump for review.
>
> On Sun, Dec 24, 2017 at 11:33 AM, Nick Desaulniers
> <[email protected]> wrote:
>> - link->next = -1 << OBJ_TAG_BITS;
>> + link->next = -1U << OBJ_TAG_BITS;
>
> -1UL?
Oops, good catch.
> Please, resend it with including Andrew Morton
> <[email protected]> who merges zsmalloc patch into his tree.
Will do.
On Sun, Jan 7, 2018 at 3:02 PM, Andy Shevchenko
<[email protected]> wrote:
> Oh, boy, shouldn't be rather GENMASK() / GENMASK_ULL() in a way how
Thanks for the suggestion. `GENMASK(BITS_PER_LONG - 1, OBJ_TAG_BITS);`
is equivalent. Whether that is more readable, I'll wait for Minchan
to decide. If that's preferred, I'll make sure to credit you with the
Suggested-By tag in the commit message.
Hi Nick,
On Mon, Jan 08, 2018 at 08:35:19PM -0800, Nick Desaulniers wrote:
> On Sun, Jan 7, 2018 at 7:04 AM, Minchan Kim <[email protected]> wrote:
> > Sorry for the delay. I have missed this until now. ;-(
>
> No worries, figured patches would need a post holiday bump for review.
>
> >
> > On Sun, Dec 24, 2017 at 11:33 AM, Nick Desaulniers
> > <[email protected]> wrote:
> >> - link->next = -1 << OBJ_TAG_BITS;
> >> + link->next = -1U << OBJ_TAG_BITS;
> >
> > -1UL?
>
> Oops, good catch.
>
> > Please, resend it with including Andrew Morton
> > <[email protected]> who merges zsmalloc patch into his tree.
>
> Will do.
>
> On Sun, Jan 7, 2018 at 3:02 PM, Andy Shevchenko
> <[email protected]> wrote:
> > Oh, boy, shouldn't be rather GENMASK() / GENMASK_ULL() in a way how
>
> Thanks for the suggestion. `GENMASK(BITS_PER_LONG - 1, OBJ_TAG_BITS);`
> is equivalent. Whether that is more readable, I'll wait for Minchan
> to decide. If that's preferred, I'll make sure to credit you with the
> Suggested-By tag in the commit message.
I don't see any benefit with GENMASK in our usecase.
If it's not a good justfication, I'd like to use just -1UL which
would be more readable without effort to understand new API.
Thanks.
Fixes warnings about shifting unsigned literals being undefined
behavior.
Suggested-by: Minchan Kim <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
Changes since v1:
* Use L suffix in addition to U, as suggested (link->next is unsigned long).
mm/zsmalloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 683c065..b9040bd 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1057,7 +1057,7 @@ static void init_zspage(struct size_class *class, struct zspage *zspage)
* Reset OBJ_TAG_BITS bit to last link to tell
* whether it's allocated object or not.
*/
- link->next = -1 << OBJ_TAG_BITS;
+ link->next = -1UL << OBJ_TAG_BITS;
}
kunmap_atomic(vaddr);
page = next_page;
--
2.7.4
On (01/10/18 19:41), Nick Desaulniers wrote:
> Fixes warnings about shifting unsigned literals being undefined
> behavior.
>
> Suggested-by: Minchan Kim <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
looks good to me.
Reviewed-by: Sergey Senozhatsky <[email protected]>
-ss