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@xxxxxxxxxx> >> Signed-off-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx> > > But the overall change looks good to me: > > Reviewed-by: Oscar Salvador <osalvador@xxxxxxx> 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
