| Message ID | 1600145748-26518-1-git-send-email-vijayb@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] mm: khugepaged: avoid overriding min_free_kbytes set by user | expand |
On Tue, Sep 15, 2020 at 12:55 AM Vijay Balakrishna <vijayb@linux.microsoft.com> wrote: > > set_recommended_min_free_kbytes need to honor min_free_kbytes set by the > user. Post start-of-day THP enable or memory hotplug operations can > lose user specified min_free_kbytes, in particular when it is higher than > calculated recommended value. > > Signed-off-by: Vijay Balakrishna <vijayb@linux.microsoft.com> > Cc: stable@vger.kernel.org > --- > mm/khugepaged.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > index 4f7107476a6f..b4b753ba411a 100644 > --- a/mm/khugepaged.c > +++ b/mm/khugepaged.c > @@ -2253,7 +2253,7 @@ static void set_recommended_min_free_kbytes(void) > { > struct zone *zone; > int nr_zones = 0; > - unsigned long recommended_min; > + int recommended_min; Hi Vijay, Please explain in log the type change. user_min_free_kbytes is of "int" type and is initialized to -1, so comparing to unsigned leads to undesired effect. > > for_each_populated_zone(zone) { > /* > @@ -2280,12 +2280,12 @@ static void set_recommended_min_free_kbytes(void) > > /* don't ever allow to reserve more than 5% of the lowmem */ > recommended_min = min(recommended_min, > - (unsigned long) nr_free_buffer_pages() / 20); > + (int) nr_free_buffer_pages() / 20); > recommended_min <<= (PAGE_SHIFT-10); > > - if (recommended_min > min_free_kbytes) { > + if (recommended_min > user_min_free_kbytes) { Took me a while to understand, but this change is correct. It is the same logic that is done in init_per_zone_wmark_min(). Please join the two patches you have in one patch series. Reviewed-by: Pavel Tatashin <pasha.tatashin@soleen.com> Thank you, Pasha
On 9/15/2020 11:27 AM, Pavel Tatashin wrote: > On Tue, Sep 15, 2020 at 12:55 AM Vijay Balakrishna > <vijayb@linux.microsoft.com> wrote: >> >> set_recommended_min_free_kbytes need to honor min_free_kbytes set by the >> user. Post start-of-day THP enable or memory hotplug operations can >> lose user specified min_free_kbytes, in particular when it is higher than >> calculated recommended value. >> >> Signed-off-by: Vijay Balakrishna <vijayb@linux.microsoft.com> >> Cc: stable@vger.kernel.org >> --- >> mm/khugepaged.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/mm/khugepaged.c b/mm/khugepaged.c >> index 4f7107476a6f..b4b753ba411a 100644 >> --- a/mm/khugepaged.c >> +++ b/mm/khugepaged.c >> @@ -2253,7 +2253,7 @@ static void set_recommended_min_free_kbytes(void) >> { >> struct zone *zone; >> int nr_zones = 0; >> - unsigned long recommended_min; >> + int recommended_min; > > Hi Vijay, > > Please explain in log the type change. user_min_free_kbytes is of > "int" type and is initialized to -1, so comparing to unsigned leads to > undesired effect. Hi Pasha, I will update log. I uncovered it when trying to understand why one-line change to address issue raised by Kirill didn't work! > > >> >> for_each_populated_zone(zone) { >> /* >> @@ -2280,12 +2280,12 @@ static void set_recommended_min_free_kbytes(void) >> >> /* don't ever allow to reserve more than 5% of the lowmem */ >> recommended_min = min(recommended_min, >> - (unsigned long) nr_free_buffer_pages() / 20); >> + (int) nr_free_buffer_pages() / 20); >> recommended_min <<= (PAGE_SHIFT-10); >> >> - if (recommended_min > min_free_kbytes) { >> + if (recommended_min > user_min_free_kbytes) { > > Took me a while to understand, but this change is correct. It is the > same logic that is done in init_per_zone_wmark_min(). > > Please join the two patches you have in one patch series. > Reviewed-by: Pavel Tatashin <pasha.tatashin@soleen.com> Will do. Thanks, Vijay > > Thank you, > Pasha >
diff --git a/mm/khugepaged.c b/mm/khugepaged.c index 4f7107476a6f..b4b753ba411a 100644 --- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -2253,7 +2253,7 @@ static void set_recommended_min_free_kbytes(void) { struct zone *zone; int nr_zones = 0; - unsigned long recommended_min; + int recommended_min; for_each_populated_zone(zone) { /* @@ -2280,12 +2280,12 @@ static void set_recommended_min_free_kbytes(void) /* don't ever allow to reserve more than 5% of the lowmem */ recommended_min = min(recommended_min, - (unsigned long) nr_free_buffer_pages() / 20); + (int) nr_free_buffer_pages() / 20); recommended_min <<= (PAGE_SHIFT-10); - if (recommended_min > min_free_kbytes) { + if (recommended_min > user_min_free_kbytes) { if (user_min_free_kbytes >= 0) - pr_info("raising min_free_kbytes from %d to %lu to help transparent hugepage allocations\n", + pr_info("raising min_free_kbytes from %d to %d to help transparent hugepage allocations\n", min_free_kbytes, recommended_min); min_free_kbytes = recommended_min;
set_recommended_min_free_kbytes need to honor min_free_kbytes set by the user. Post start-of-day THP enable or memory hotplug operations can lose user specified min_free_kbytes, in particular when it is higher than calculated recommended value. Signed-off-by: Vijay Balakrishna <vijayb@linux.microsoft.com> Cc: stable@vger.kernel.org --- mm/khugepaged.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)