Messages in this thread | | | Date | Thu, 31 Mar 2022 19:23:48 +0800 | Subject | Re: [PATCH] hugetlb: Fix hugepages_setup when deal with pernode | From | "liupeng (DM)" <> |
| |
On 2022/3/30 1:43, Mike Kravetz wrote: > On 3/28/22 20:59, liupeng (DM) wrote: >> On 2022/3/29 10:46, Mike Kravetz wrote: >>> Yes, I agree that the change is needed and the current behavior is >>> unacceptable. >>> >>> One remaining question is the change from returning '0' to '1' in the case >>> of error. I do understand this is to prevent the invalid parameter string >>> from being passed to init. It may not be correct/right, but in every other >>> case where an invalid parameter in encountered in hugetlb command line >>> processing we return "0". Should we perhaps change all these other places >>> to be consistent? I honestly do not know what is the appropriate behavior >>> in these situations. >> Thank you for your carefulness and question. >> >> I have checked default_hugepagesz_setup and hugepages_setup will both print >> some information before return '0', so there is no need to print again in >> "Unknown kernel command line parameters". >> >> Should I send another patch to repair the rest "return 0" in hugetlb? > I would suggest two patches: > > 1) Fix the issue with invalid nodes specified. However, leave the "return 0" > behavior in hugepages_setup to be consistent with the rest of the code. > This patch can be sent to stable with "Fixes: b5389086ad7b" tag as it > addresses an existing issue. > 2) Clean up the places where we return 0 and it would be better to return 1. > No cc stable here and just let the changes target future releases. I have tried to write a patch as your suggestion, but the best way I can carry it out is the original patch. To meet "Fix invalid nodes issue and leave thereturn 0 behavior", I have to add the following redundant code:
invalid: pr_warn("HugeTLB: Invalid hugepages parameter %s\n", p); + + /* Allocate gigantic hstates for successfully parsed parameters*/ + if (hugetlb_max_hstate && hstate_is_gigantic(parsed_hstate)) + hugetlb_hstate_alloc_pages(parsed_hstate); + last_mhp = mhp; return 0;
Any ideas?
| |