| Message ID | 20210623023418.350616-3-gshan@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/page_reporting: Make page reporting work on arm64 with 64KB page size | expand |
On Tue, Jun 22, 2021 at 5:34 PM Gavin Shan <gshan@redhat.com> wrote: > > The macro PAGE_REPORTING_MIN_ORDER is defined as the page reporting > threshold. It can't be adjusted at runtime. > > This introduces a variable (@page_reporting_order) to replace the > marcro (PAGE_REPORTING_MIN_ORDER). It's also exported so that the > page reporting order can be adjusted at runtime. > > Suggested-by: David Hildenbrand <david@redhat.com> > Signed-off-by: Gavin Shan <gshan@redhat.com> > --- > Documentation/admin-guide/kernel-parameters.txt | 6 ++++++ > mm/page_reporting.c | 8 ++++++-- > mm/page_reporting.h | 5 ++--- > 3 files changed, 14 insertions(+), 5 deletions(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index cb89dbdedc46..566c4b9af3cd 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -3566,6 +3566,12 @@ > off: turn off poisoning (default) > on: turn on poisoning > > + page_reporting.page_reporting_order= > + [KNL] Minimal page reporting order > + Format: <integer> > + Adjust the minimal page reporting order. The page > + reporting is disabled when it exceeds (MAX_ORDER-1). > + Based on the issue found by Stephen I think we may need to tweak this a bit. I think we may want to just default this value to MAX_ORDER. We can override this value at registration time with the value provided either by the reporting device or pageblock_order if prdev->order is not set. > diff --git a/mm/page_reporting.c b/mm/page_reporting.c > index df9c5054e1b4..293a8713ef7c 100644 > --- a/mm/page_reporting.c > +++ b/mm/page_reporting.c > @@ -10,6 +10,10 @@ > #include "page_reporting.h" > #include "internal.h" > > +unsigned int page_reporting_order = pageblock_order; Rather than setting this to pageblock_order directly you can set this to MAX_ORDER which should be constant. Then we can just add some checks in page_reporting_register to update it when pageblock_order is less than page_reporting_order. Then in the next patch you could tweak it so that it will use "prdev->order ? : pageblock_order" instead of just pageblock_order.
On Thu, Jun 24, 2021 at 06:55:13AM -0700, Alexander Duyck wrote: > On Tue, Jun 22, 2021 at 5:34 PM Gavin Shan <gshan@redhat.com> wrote: > > > > The macro PAGE_REPORTING_MIN_ORDER is defined as the page reporting > > threshold. It can't be adjusted at runtime. > > > > This introduces a variable (@page_reporting_order) to replace the > > marcro (PAGE_REPORTING_MIN_ORDER). It's also exported so that the > > page reporting order can be adjusted at runtime. > > > > Suggested-by: David Hildenbrand <david@redhat.com> > > Signed-off-by: Gavin Shan <gshan@redhat.com> > > --- > > Documentation/admin-guide/kernel-parameters.txt | 6 ++++++ > > mm/page_reporting.c | 8 ++++++-- > > mm/page_reporting.h | 5 ++--- > > 3 files changed, 14 insertions(+), 5 deletions(-) > > > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > > index cb89dbdedc46..566c4b9af3cd 100644 > > --- a/Documentation/admin-guide/kernel-parameters.txt > > +++ b/Documentation/admin-guide/kernel-parameters.txt > > @@ -3566,6 +3566,12 @@ > > off: turn off poisoning (default) > > on: turn on poisoning > > > > + page_reporting.page_reporting_order= > > + [KNL] Minimal page reporting order > > + Format: <integer> > > + Adjust the minimal page reporting order. The page > > + reporting is disabled when it exceeds (MAX_ORDER-1). > > + > > Based on the issue found by Stephen I think we may need to tweak this > a bit. I think we may want to just default this value to MAX_ORDER. We > can override this value at registration time with the value provided > either by the reporting device or pageblock_order if prdev->order is > not set. > > > diff --git a/mm/page_reporting.c b/mm/page_reporting.c > > index df9c5054e1b4..293a8713ef7c 100644 > > --- a/mm/page_reporting.c > > +++ b/mm/page_reporting.c > > @@ -10,6 +10,10 @@ > > #include "page_reporting.h" > > #include "internal.h" > > > > +unsigned int page_reporting_order = pageblock_order; > > Rather than setting this to pageblock_order directly you can set this > to MAX_ORDER which should be constant. Then we can just add some > checks in page_reporting_register to update it when pageblock_order is > less than page_reporting_order. > > Then in the next patch you could tweak it so that it will use > "prdev->order ? : pageblock_order" instead of just pageblock_order. I like that! Much cleaner ... the patch is in -mm now, I think it's a good idea to drop it and update to use this idea.
On 6/25/21 5:02 AM, Michael S. Tsirkin wrote: > On Thu, Jun 24, 2021 at 06:55:13AM -0700, Alexander Duyck wrote: >> On Tue, Jun 22, 2021 at 5:34 PM Gavin Shan <gshan@redhat.com> wrote: >>> >>> The macro PAGE_REPORTING_MIN_ORDER is defined as the page reporting >>> threshold. It can't be adjusted at runtime. >>> >>> This introduces a variable (@page_reporting_order) to replace the >>> marcro (PAGE_REPORTING_MIN_ORDER). It's also exported so that the >>> page reporting order can be adjusted at runtime. >>> >>> Suggested-by: David Hildenbrand <david@redhat.com> >>> Signed-off-by: Gavin Shan <gshan@redhat.com> >>> --- >>> Documentation/admin-guide/kernel-parameters.txt | 6 ++++++ >>> mm/page_reporting.c | 8 ++++++-- >>> mm/page_reporting.h | 5 ++--- >>> 3 files changed, 14 insertions(+), 5 deletions(-) >>> >>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt >>> index cb89dbdedc46..566c4b9af3cd 100644 >>> --- a/Documentation/admin-guide/kernel-parameters.txt >>> +++ b/Documentation/admin-guide/kernel-parameters.txt >>> @@ -3566,6 +3566,12 @@ >>> off: turn off poisoning (default) >>> on: turn on poisoning >>> >>> + page_reporting.page_reporting_order= >>> + [KNL] Minimal page reporting order >>> + Format: <integer> >>> + Adjust the minimal page reporting order. The page >>> + reporting is disabled when it exceeds (MAX_ORDER-1). >>> + >> >> Based on the issue found by Stephen I think we may need to tweak this >> a bit. I think we may want to just default this value to MAX_ORDER. We >> can override this value at registration time with the value provided >> either by the reporting device or pageblock_order if prdev->order is >> not set. >> >>> diff --git a/mm/page_reporting.c b/mm/page_reporting.c >>> index df9c5054e1b4..293a8713ef7c 100644 >>> --- a/mm/page_reporting.c >>> +++ b/mm/page_reporting.c >>> @@ -10,6 +10,10 @@ >>> #include "page_reporting.h" >>> #include "internal.h" >>> >>> +unsigned int page_reporting_order = pageblock_order; >> >> Rather than setting this to pageblock_order directly you can set this >> to MAX_ORDER which should be constant. Then we can just add some >> checks in page_reporting_register to update it when pageblock_order is >> less than page_reporting_order. >> >> Then in the next patch you could tweak it so that it will use >> "prdev->order ? : pageblock_order" instead of just pageblock_order. > > I like that! Much cleaner ... the patch is in -mm now, I think it's a > good idea to drop it and update to use this idea. > Thanks, Alex and Michael. v4 was posted to include the changes. Besides, "module.h" is missed in page_reporting.c as Andrew found. All the changes are squeezed to PATCH[2] and PATCH[3]. Andrew, could you help to replace the patches with v4 in linux-mm tree? Sorry for work to you :) Thanks, Gavin
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index cb89dbdedc46..566c4b9af3cd 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -3566,6 +3566,12 @@ off: turn off poisoning (default) on: turn on poisoning + page_reporting.page_reporting_order= + [KNL] Minimal page reporting order + Format: <integer> + Adjust the minimal page reporting order. The page + reporting is disabled when it exceeds (MAX_ORDER-1). + panic= [KNL] Kernel behaviour on panic: delay <timeout> timeout > 0: seconds before rebooting timeout = 0: wait forever diff --git a/mm/page_reporting.c b/mm/page_reporting.c index df9c5054e1b4..293a8713ef7c 100644 --- a/mm/page_reporting.c +++ b/mm/page_reporting.c @@ -10,6 +10,10 @@ #include "page_reporting.h" #include "internal.h" +unsigned int page_reporting_order = pageblock_order; +module_param(page_reporting_order, uint, 0644); +MODULE_PARM_DESC(page_reporting_order, "Set page reporting order"); + #define PAGE_REPORTING_DELAY (2 * HZ) static struct page_reporting_dev_info __rcu *pr_dev_info __read_mostly; @@ -229,7 +233,7 @@ page_reporting_process_zone(struct page_reporting_dev_info *prdev, /* Generate minimum watermark to be able to guarantee progress */ watermark = low_wmark_pages(zone) + - (PAGE_REPORTING_CAPACITY << PAGE_REPORTING_MIN_ORDER); + (PAGE_REPORTING_CAPACITY << page_reporting_order); /* * Cancel request if insufficient free memory or if we failed @@ -239,7 +243,7 @@ page_reporting_process_zone(struct page_reporting_dev_info *prdev, return err; /* Process each free list starting from lowest order/mt */ - for (order = PAGE_REPORTING_MIN_ORDER; order < MAX_ORDER; order++) { + for (order = page_reporting_order; order < MAX_ORDER; order++) { for (mt = 0; mt < MIGRATE_TYPES; mt++) { /* We do not pull pages from the isolate free list */ if (is_migrate_isolate(mt)) diff --git a/mm/page_reporting.h b/mm/page_reporting.h index 2c385dd4ddbd..c51dbc228b94 100644 --- a/mm/page_reporting.h +++ b/mm/page_reporting.h @@ -10,10 +10,9 @@ #include <linux/pgtable.h> #include <linux/scatterlist.h> -#define PAGE_REPORTING_MIN_ORDER pageblock_order - #ifdef CONFIG_PAGE_REPORTING DECLARE_STATIC_KEY_FALSE(page_reporting_enabled); +extern unsigned int page_reporting_order; void __page_reporting_notify(void); static inline bool page_reported(struct page *page) @@ -38,7 +37,7 @@ static inline void page_reporting_notify_free(unsigned int order) return; /* Determine if we have crossed reporting threshold */ - if (order < PAGE_REPORTING_MIN_ORDER) + if (order < page_reporting_order) return; /* This will add a few cycles, but should be called infrequently */
The macro PAGE_REPORTING_MIN_ORDER is defined as the page reporting threshold. It can't be adjusted at runtime. This introduces a variable (@page_reporting_order) to replace the marcro (PAGE_REPORTING_MIN_ORDER). It's also exported so that the page reporting order can be adjusted at runtime. Suggested-by: David Hildenbrand <david@redhat.com> Signed-off-by: Gavin Shan <gshan@redhat.com> --- Documentation/admin-guide/kernel-parameters.txt | 6 ++++++ mm/page_reporting.c | 8 ++++++-- mm/page_reporting.h | 5 ++--- 3 files changed, 14 insertions(+), 5 deletions(-)