lkml.org 
[lkml]   [2019]   [Mar]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v4] mm/hugetlb: Fix unsigned overflow in __nr_hugepages_store_common()
From
Date
On 3/6/19 1:41 AM, Oscar Salvador wrote:
> On Mon, Mar 04, 2019 at 08:15:40PM -0800, Mike Kravetz wrote:
>> In addition, the code in __nr_hugepages_store_common() which tries to
>> handle the case of not being able to allocate a node mask would likely
>> result in incorrect behavior. Luckily, it is very unlikely we will
>> ever take this path. If we do, simply return ENOMEM.
>
> Hi Mike,
>
> I still thnk that we could just get rid of the NODEMASK_ALLOC machinery
> here, it adds a needlessly complexity IMHO.
> Note that before "(5df66d306ec9: mm: fix comment for NODEMASK_ALLOC)",
> the comment about the size was wrong, showing a much bigger size that it
> actually was, and I would not be surprised if people started to add
> NODEMASK_ALLOC here and there because of that.
>
> Actually, there was a little talk about removing NODEMASK_ALLOC altogether,
> but some further checks must be done before.

Thanks for the information. I too saw or remembered a large byte value. :(
A quick grep doesn't reveal any configurable way to get NODE_SHIFT larger
than 10. Of course, that could change. So, it does seem a bit funny that
NODEMASK_ALLOC() kicks into dynamic allocation mode with NODE_SHIFT > 8.
Although, my desktop distro has NODE_SHIFT set to 10.

>> Reported-by: Jing Xiangfeng <jingxiangfeng@huawei.com>
>> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
>
> But the overall change looks good to me:
>
> Reviewed-by: Oscar Salvador <osalvador@suse.de>

Thanks.
I'm going to leave as is for now and put off removal of the dynamic allocation
for a later time. Unless, you get around to removing NODEMASK_ALLOC
altogether. :)
--
Mike Kravetz

\
 
 \ /
  Last update: 2019-03-07 01:18    [W:0.063 / U:1.840 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site