| Message ID | 1632831797-32192-1-git-send-email-liangcaifan19@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: Set min_free_kbytes with user_min_free_kbytes when user_min_free_kbytes is preferred | expand |
On Tue, 28 Sep 2021 20:23:17 +0800 Liangcai Fan <liangcaifan19@gmail.com> wrote: > The 'min_free_kbytes' and 'user_min_free_kbytes' maybe inconsistent > after a few times of memory hotplug. What does "inconsistent" mean here? Please describe the problem in more detail, perhaps with examples. > When 'new_min_free_kbytes' is not larger than 'user_min_free_kbytes', > set 'min_free_kbytes' with 'user_min_free_kbytes' rather than leave > it as the 'new_min_free_kbytes' calculated for the last time. > > Signed-off-by: Liangcai Fan <liangcaifan19@gmail.com> > Cc: Chunyan Zhang <zhang.lyra@gmail.com> > --- > mm/page_alloc.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index b37435c..ddf9dc1 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -8467,6 +8467,12 @@ int __meminit init_per_zone_wmark_min(void) > if (min_free_kbytes > 262144) > min_free_kbytes = 262144; > } else { > + /* > + * Set 'min_free_kbytes' with 'user_min_free_kbytes' rather than > + * leave it as the 'new_min_free_kbytes' calculated for the last > + * time. > + */ This comment explains what the code is doing, which is almost always obvious from reading the code! A better comment will describe *why* the code is doing whatever is does. "why, not what", please. > + min_free_kbytes = user_min_free_kbytes; > pr_warn("min_free_kbytes is not updated to %d because user defined value %d is preferred\n", > new_min_free_kbytes, user_min_free_kbytes); > }
Andrew Morton <akpm@linux-foundation.org> 于2021年9月29日周三 上午6:29写道： > > On Tue, 28 Sep 2021 20:23:17 +0800 Liangcai Fan <liangcaifan19@gmail.com> wrote: > > > The 'min_free_kbytes' and 'user_min_free_kbytes' maybe inconsistent > > after a few times of memory hotplug. > > What does "inconsistent" mean here? > > Please describe the problem in more detail, perhaps with examples. > example: 1. Turn on the device with 1G memory, and the calculated 'new_min_free_kbytes' is 3452kb, at this time 'min_free_kbytes' = 'new_min_free_kbytes' = 3452kb, 'user_min_free_kbytes' is the initial value -1. 2. The user sets 'min_free_kbytes' to 4096kb through the node, at this time 'user_min_free_kbytes' = 'min_free_kbytes' = 4096kb. 3. The memory has increased by 2G, and the memory has become 3G. Recalculate 'new_min_free_kbytes' to be 6568kb, which is greater than 'user_min_free_kbytes'(4096kb), at this time 'min_free_kbytes' = 'new_min_free_kbytes' = 6568kb, 'user_min_free_kbytes' = 4096kb. 4. The memory has been reduced by 2G, and now the memory has become 1G. Recalculate 'new_min_free_kbytes' to be 3452kb, which is less than 'user_min_free_kbytes'(4096kb). The original logic does not modify 'min_free_kbytes'. At this time, 'min_free_kbytes' is the last 'new_min_free_kbytes'(6568kb). The memory is 1G ,'min_free_kbytes' should be equal to 'user_min_free_kbytes'(4096kb) set by the user. > > When 'new_min_free_kbytes' is not larger than 'user_min_free_kbytes', > > set 'min_free_kbytes' with 'user_min_free_kbytes' rather than leave > > it as the 'new_min_free_kbytes' calculated for the last time. > > > > Signed-off-by: Liangcai Fan <liangcaifan19@gmail.com> > > Cc: Chunyan Zhang <zhang.lyra@gmail.com> > > --- > > mm/page_alloc.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > > index b37435c..ddf9dc1 100644 > > --- a/mm/page_alloc.c > > +++ b/mm/page_alloc.c > > @@ -8467,6 +8467,12 @@ int __meminit init_per_zone_wmark_min(void) > > if (min_free_kbytes > 262144) > > min_free_kbytes = 262144; > > } else { > > + /* > > + * Set 'min_free_kbytes' with 'user_min_free_kbytes' rather than > > + * leave it as the 'new_min_free_kbytes' calculated for the last > > + * time. > > + */ > > This comment explains what the code is doing, which is almost always > obvious from reading the code! A better comment will describe *why* > the code is doing whatever is does. "why, not what", please. > I think this comment is redundant and will be deleted. > > + min_free_kbytes = user_min_free_kbytes; > > pr_warn("min_free_kbytes is not updated to %d because user defined value %d is preferred\n", > > new_min_free_kbytes, user_min_free_kbytes); > > } >
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index b37435c..ddf9dc1 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -8467,6 +8467,12 @@ int __meminit init_per_zone_wmark_min(void) if (min_free_kbytes > 262144) min_free_kbytes = 262144; } else { + /* + * Set 'min_free_kbytes' with 'user_min_free_kbytes' rather than + * leave it as the 'new_min_free_kbytes' calculated for the last + * time. + */ + min_free_kbytes = user_min_free_kbytes; pr_warn("min_free_kbytes is not updated to %d because user defined value %d is preferred\n", new_min_free_kbytes, user_min_free_kbytes); }
The 'min_free_kbytes' and 'user_min_free_kbytes' maybe inconsistent after a few times of memory hotplug. When 'new_min_free_kbytes' is not larger than 'user_min_free_kbytes', set 'min_free_kbytes' with 'user_min_free_kbytes' rather than leave it as the 'new_min_free_kbytes' calculated for the last time. Signed-off-by: Liangcai Fan <liangcaifan19@gmail.com> Cc: Chunyan Zhang <zhang.lyra@gmail.com> --- mm/page_alloc.c | 6 ++++++ 1 file changed, 6 insertions(+)