| Message ID | 20210714091800.42645-2-songmuchun@bytedance.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Free the 2nd vmemmap page associated with each HugeTLB page | expand |
On 7/14/21 2:17 AM, Muchun Song wrote: > Instead of hard-coding ((1UL << NR_PAGEFLAGS) - 1) everywhere, introducing > PAGEFLAGS_MASK to make the code clear to get the page flags. > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > --- > include/linux/page-flags.h | 4 +++- > include/trace/events/page_ref.h | 4 ++-- > lib/test_printf.c | 2 +- > lib/vsprintf.c | 2 +- > 4 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h > index 5922031ffab6..358d3f6fa976 100644 > --- a/include/linux/page-flags.h > +++ b/include/linux/page-flags.h > @@ -178,6 +178,8 @@ enum pageflags { > PG_reported = PG_uptodate, > }; > > +#define PAGEFLAGS_MASK (~((1UL << NR_PAGEFLAGS) - 1)) Can you explain why you chose this definition instead of #define PAGEFLAGS_MASK ((1UL << NR_PAGEFLAGS) - 1) and mostly use ~PAGEFLAGS_MASK below?
On Tue, Jul 27, 2021 at 5:04 AM Mike Kravetz <mike.kravetz@oracle.com> wrote: > > On 7/14/21 2:17 AM, Muchun Song wrote: > > Instead of hard-coding ((1UL << NR_PAGEFLAGS) - 1) everywhere, introducing > > PAGEFLAGS_MASK to make the code clear to get the page flags. > > > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > > --- > > include/linux/page-flags.h | 4 +++- > > include/trace/events/page_ref.h | 4 ++-- > > lib/test_printf.c | 2 +- > > lib/vsprintf.c | 2 +- > > 4 files changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h > > index 5922031ffab6..358d3f6fa976 100644 > > --- a/include/linux/page-flags.h > > +++ b/include/linux/page-flags.h > > @@ -178,6 +178,8 @@ enum pageflags { > > PG_reported = PG_uptodate, > > }; > > > > +#define PAGEFLAGS_MASK (~((1UL << NR_PAGEFLAGS) - 1)) > > Can you explain why you chose this definition instead of > > #define PAGEFLAGS_MASK ((1UL << NR_PAGEFLAGS) - 1) > > and mostly use ~PAGEFLAGS_MASK below? Hi Mike, Actually, I learned from PAGE_MASK. So I thought the macro like xxx_MASK should be the format of 0x00...00ff...ff. I don't know if this is an unwritten rule. Please correct me if I am wrong. Thanks. > -- > Mike Kravetz
On Tue, Jul 27, 2021 at 2:27 PM Muchun Song <songmuchun@bytedance.com> wrote: > > On Tue, Jul 27, 2021 at 5:04 AM Mike Kravetz <mike.kravetz@oracle.com> wrote: > > > > On 7/14/21 2:17 AM, Muchun Song wrote: > > > Instead of hard-coding ((1UL << NR_PAGEFLAGS) - 1) everywhere, introducing > > > PAGEFLAGS_MASK to make the code clear to get the page flags. > > > > > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > > > --- > > > include/linux/page-flags.h | 4 +++- > > > include/trace/events/page_ref.h | 4 ++-- > > > lib/test_printf.c | 2 +- > > > lib/vsprintf.c | 2 +- > > > 4 files changed, 7 insertions(+), 5 deletions(-) > > > > > > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h > > > index 5922031ffab6..358d3f6fa976 100644 > > > --- a/include/linux/page-flags.h > > > +++ b/include/linux/page-flags.h > > > @@ -178,6 +178,8 @@ enum pageflags { > > > PG_reported = PG_uptodate, > > > }; > > > > > > +#define PAGEFLAGS_MASK (~((1UL << NR_PAGEFLAGS) - 1)) > > > > Can you explain why you chose this definition instead of > > > > #define PAGEFLAGS_MASK ((1UL << NR_PAGEFLAGS) - 1) > > > > and mostly use ~PAGEFLAGS_MASK below? > > Hi Mike, > > Actually, I learned from PAGE_MASK. So I thought the macro > like xxx_MASK should be the format of 0x00...00ff...ff. I don't ^^^ Sorry. I mean 0xff...ff00...00 here. > know if this is an unwritten rule. Please correct me if I am > wrong. > > Thanks. > > > -- > > Mike Kravetz
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h index 5922031ffab6..358d3f6fa976 100644 --- a/include/linux/page-flags.h +++ b/include/linux/page-flags.h @@ -178,6 +178,8 @@ enum pageflags { PG_reported = PG_uptodate, }; +#define PAGEFLAGS_MASK (~((1UL << NR_PAGEFLAGS) - 1)) + #ifndef __GENERATING_BOUNDS_H static inline unsigned long _compound_head(const struct page *page) @@ -859,7 +861,7 @@ static inline void ClearPageSlabPfmemalloc(struct page *page) * alloc-free cycle to prevent from reusing the page. */ #define PAGE_FLAGS_CHECK_AT_PREP \ - (((1UL << NR_PAGEFLAGS) - 1) & ~__PG_HWPOISON) + (~(PAGEFLAGS_MASK | __PG_HWPOISON)) #define PAGE_FLAGS_PRIVATE \ (1UL << PG_private | 1UL << PG_private_2) diff --git a/include/trace/events/page_ref.h b/include/trace/events/page_ref.h index 5d2ea93956ce..643b1b4e9f27 100644 --- a/include/trace/events/page_ref.h +++ b/include/trace/events/page_ref.h @@ -38,7 +38,7 @@ DECLARE_EVENT_CLASS(page_ref_mod_template, TP_printk("pfn=0x%lx flags=%s count=%d mapcount=%d mapping=%p mt=%d val=%d", __entry->pfn, - show_page_flags(__entry->flags & ((1UL << NR_PAGEFLAGS) - 1)), + show_page_flags(__entry->flags & ~PAGEFLAGS_MASK), __entry->count, __entry->mapcount, __entry->mapping, __entry->mt, __entry->val) @@ -88,7 +88,7 @@ DECLARE_EVENT_CLASS(page_ref_mod_and_test_template, TP_printk("pfn=0x%lx flags=%s count=%d mapcount=%d mapping=%p mt=%d val=%d ret=%d", __entry->pfn, - show_page_flags(__entry->flags & ((1UL << NR_PAGEFLAGS) - 1)), + show_page_flags(__entry->flags & ~PAGEFLAGS_MASK), __entry->count, __entry->mapcount, __entry->mapping, __entry->mt, __entry->val, __entry->ret) diff --git a/lib/test_printf.c b/lib/test_printf.c index 8ac71aee46af..eadcc6458b10 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c @@ -614,7 +614,7 @@ page_flags_test(int section, int node, int zone, int last_cpupid, bool append = false; int i; - flags &= BIT(NR_PAGEFLAGS) - 1; + flags &= ~PAGEFLAGS_MASK; if (flags) { page_flags |= flags; snprintf(cmp_buf + size, BUF_SIZE - size, "%s", name); diff --git a/lib/vsprintf.c b/lib/vsprintf.c index dd006adfe853..ff7c180a2971 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -2019,7 +2019,7 @@ static const struct page_flags_fields pff[] = { static char *format_page_flags(char *buf, char *end, unsigned long flags) { - unsigned long main_flags = flags & (BIT(NR_PAGEFLAGS) - 1); + unsigned long main_flags = flags & ~PAGEFLAGS_MASK; bool append = false; int i;
Instead of hard-coding ((1UL << NR_PAGEFLAGS) - 1) everywhere, introducing PAGEFLAGS_MASK to make the code clear to get the page flags. Signed-off-by: Muchun Song <songmuchun@bytedance.com> --- include/linux/page-flags.h | 4 +++- include/trace/events/page_ref.h | 4 ++-- lib/test_printf.c | 2 +- lib/vsprintf.c | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-)