Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1276394 > unrolled thread

[PATCH v2 0/9] page_owner improvements for debugging

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/9] page_owner improvements for debugging Vlastimil Babka <vbabka@suse.cz> - 2015-11-24 13:40 +0100
    [PATCH v2 4/9] mm, page_owner: copy page owner info during migration Vlastimil Babka <vbabka@suse.cz> - 2015-11-24 13:40 +0100
    [PATCH v2 3/9] mm, page_owner: convert page_owner_inited to static key Vlastimil Babka <vbabka@suse.cz> - 2015-11-24 13:40 +0100
      Re: [PATCH v2 3/9] mm, page_owner: convert page_owner_inited to  static key Michal Hocko <mhocko@kernel.org> - 2015-11-25 16:00 +0100
        Re: [PATCH v2 3/9] mm, page_owner: convert page_owner_inited to  static key Vlastimil Babka <vbabka@suse.cz> - 2015-11-25 16:10 +0100
          Re: [PATCH v2 3/9] mm, page_owner: convert page_owner_inited to  static key Peter Zijlstra <peterz@infradead.org> - 2015-11-25 16:30 +0100
            Re: [PATCH v2 3/9] mm, page_owner: convert page_owner_inited to  static key Michal Hocko <mhocko@kernel.org> - 2015-11-25 16:50 +0100
    [PATCH v2 2/9] mm, page_owner: print symbolic migratetype of both page and pageblock Vlastimil Babka <vbabka@suse.cz> - 2015-11-24 13:40 +0100
      Re: [PATCH v2 2/9] mm, page_owner: print symbolic migratetype of  both page and pageblock Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2015-11-25 09:20 +0100
    [PATCH v2 8/9] mm, page_alloc: print symbolic gfp_flags on allocation failure Vlastimil Babka <vbabka@suse.cz> - 2015-11-24 13:50 +0100
      Re: [PATCH v2 8/9] mm, page_alloc: print symbolic gfp_flags on  allocation failure Michal Hocko <mhocko@kernel.org> - 2015-11-25 15:40 +0100
    [PATCH v2 6/9] mm, debug: introduce dump_gfpflag_names() for symbolic printing of gfp_flags Vlastimil Babka <vbabka@suse.cz> - 2015-11-24 13:50 +0100
      Re: [PATCH v2 6/9] mm, debug: introduce dump_gfpflag_names() for  symbolic printing of gfp_flags Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2015-11-25 09:20 +0100
        Re: [PATCH v2 6/9] mm, debug: introduce dump_gfpflag_names() for  symbolic printing of gfp_flags Vlastimil Babka <vbabka@suse.cz> - 2015-11-25 11:30 +0100
          Re: [PATCH v2 6/9] mm, debug: introduce dump_gfpflag_names() for symbolic printing of gfp_flags yalin wang <yalin.wang2010@gmail.com> - 2015-11-27 04:50 +0100
    Re: [PATCH v2 0/9] page_owner improvements for debugging Michal Hocko <mhocko@kernel.org> - 2015-11-25 15:40 +0100

#1276394 — [PATCH v2 0/9] page_owner improvements for debugging

This is the second version of patchset which originally aimed to improve the
page_owner functionality. Thanks to feedback from v1 and some bugs I
discovered along the way, it is now larger in scope and number of patches.
It's based on next-20151124.

For page_owner, the main changes are
o Use static key to further reduce overhead when compiled in but not enabled.
o Improve output wrt. page and pageblock migratetypes
o Transfer the info on page migrations and track last migration reason.
o Dump the info as part of dump_page() to hopefully help debugging.

For the last point, Kirill requested a human readable printing of gfp_mask and
migratetype after v1. At that point it probably makes a lot of sense to do the
same for page alloc failure and OOM warnings. The flags have been undergoing
revisions recently, and we might be getting reports from various kernel
versions that differ. The ./scripts/gfp-translate tool needs to be pointed at
the corresponding sources to be accurate.  The downside is potentially breaking
scripts that grep these warnings, but it's not a first change done there over
the years.

Note I'm not entirely happy about the dump_gfpflag_names() implementation, due
to usage of pr_cont() unreliable on SMP (and I've seen spurious newlines in
dmesg output, while being correct on serial console or /var/log/messages).
It also doesn't allow plugging the gfp_mask translation into
/sys/kernel/debug/page_owner where it also could make sense. Maybe a new
*printf formatting flag? Too specialized maybe? Or just prepare the string in
a buffer on stack with strscpy?

Other changes since v1:
o Change placement of page owner migration calls to cover missing cases (Hugh)
o Move dump_page_owner() call up from dump_page_badflags(), so the latter can
  be used for adding debugging prints without page owner info (Kirill)

Vlastimil Babka (9):
  mm, debug: fix wrongly filtered flags in dump_vma()
  mm, page_owner: print symbolic migratetype of both page and pageblock
  mm, page_owner: convert page_owner_inited to static key
  mm, page_owner: copy page owner info during migration
  mm, page_owner: track and print last migrate reason
  mm, debug: introduce dump_gfpflag_names() for symbolic printing of
    gfp_flags
  mm, page_owner: dump page owner info from dump_page()
  mm, page_alloc: print symbolic gfp_flags on allocation failure
  mm, oom: print symbolic gfp_flags in oom warning

 Documentation/vm/page_owner.txt |  9 +++--
 include/linux/migrate.h         |  6 ++-
 include/linux/mmdebug.h         |  1 +
 include/linux/mmzone.h          |  3 ++
 include/linux/page_ext.h        |  1 +
 include/linux/page_owner.h      | 50 ++++++++++++++++++-------
 include/trace/events/gfpflags.h | 14 +++++--
 mm/debug.c                      | 44 ++++++++++++++++------
 mm/migrate.c                    | 23 ++++++++++--
 mm/oom_kill.c                   | 10 +++--
 mm/page_alloc.c                 | 18 ++++++++-
 mm/page_owner.c                 | 82 +++++++++++++++++++++++++++++++++++++----
 mm/vmstat.c                     | 15 +-------
 13 files changed, 213 insertions(+), 63 deletions(-)

-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1276397 — [PATCH v2 4/9] mm, page_owner: copy page owner info during migration

The page_owner mechanism stores gfp_flags of an allocation and stack trace
that lead to it. During page migration, the original information is
practically replaced by the allocation of free page as the migration target.
Arguably this is less useful and might lead to all the page_owner info for
migratable pages gradually converge towards compaction or numa balancing
migrations. It has also lead to inaccuracies such as one fixed by commit
e2cfc91120fa ("mm/page_owner: set correct gfp_mask on page_owner").

This patch thus introduces copying the page_owner info during migration.
However, since the fact that the page has been migrated from its original
place might be useful for debugging, the next patch will introduce a way to
track that information as well.

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 include/linux/page_owner.h | 10 +++++++++-
 mm/migrate.c               |  3 +++
 mm/page_owner.c            | 25 +++++++++++++++++++++++++
 3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/include/linux/page_owner.h b/include/linux/page_owner.h
index 8e2eb15..6440daa 100644
--- a/include/linux/page_owner.h
+++ b/include/linux/page_owner.h
@@ -11,6 +11,7 @@ extern void __reset_page_owner(struct page *page, unsigned int order);
 extern void __set_page_owner(struct page *page,
 			unsigned int order, gfp_t gfp_mask);
 extern gfp_t __get_page_owner_gfp(struct page *page);
+extern void __copy_page_owner(struct page *oldpage, struct page *newpage);
 
 static inline void reset_page_owner(struct page *page, unsigned int order)
 {
@@ -32,6 +33,11 @@ static inline gfp_t get_page_owner_gfp(struct page *page)
 	else
 		return 0;
 }
+static inline void copy_page_owner(struct page *oldpage, struct page *newpage)
+{
+	if (static_branch_unlikely(&page_owner_inited))
+		__copy_page_owner(oldpage, newpage);
+}
 #else
 static inline void reset_page_owner(struct page *page, unsigned int order)
 {
@@ -44,6 +50,8 @@ static inline gfp_t get_page_owner_gfp(struct page *page)
 {
 	return 0;
 }
-
+static inline void copy_page_owner(struct page *oldpage, struct page *newpage)
+{
+}
 #endif /* CONFIG_PAGE_OWNER */
 #endif /* __LINUX_PAGE_OWNER_H */
diff --git a/mm/migrate.c b/mm/migrate.c
index b1034f9..863a0f1 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -38,6 +38,7 @@
 #include <linux/balloon_compaction.h>
 #include <linux/mmu_notifier.h>
 #include <linux/page_idle.h>
+#include <linux/page_owner.h>
 
 #include <asm/tlbflush.h>
 
@@ -578,6 +579,8 @@ void migrate_page_copy(struct page *newpage, struct page *page)
 	 */
 	if (PageWriteback(newpage))
 		end_page_writeback(newpage);
+
+	copy_page_owner(page, newpage);
 }
 
 /************************************************************
diff --git a/mm/page_owner.c b/mm/page_owner.c
index 10a6a46..f571e55 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -84,6 +84,31 @@ gfp_t __get_page_owner_gfp(struct page *page)
 	return page_ext->gfp_mask;
 }
 
+void __copy_page_owner(struct page *oldpage, struct page *newpage)
+{
+	struct page_ext *old_ext = lookup_page_ext(oldpage);
+	struct page_ext *new_ext = lookup_page_ext(newpage);
+	int i;
+
+	new_ext->order = old_ext->order;
+	new_ext->gfp_mask = old_ext->gfp_mask;
+	new_ext->nr_entries = old_ext->nr_entries;
+
+	for (i = 0; i < ARRAY_SIZE(new_ext->trace_entries); i++)
+		new_ext->trace_entries[i] = old_ext->trace_entries[i];
+
+	/*
+	 * We don't clear the bit on the oldpage as it's going to be freed
+	 * after migration. Until then, the info can be useful in case of
+	 * a bug, and the overal stats will be off a bit only temporarily.
+	 * Also, migrate_misplaced_transhuge_page() can still fail the
+	 * migration and then we want the oldpage to retain the info. But
+	 * in that case we also don't need to explicitly clear the info from
+	 * the new page, which will be freed.
+	 */
+	__set_bit(PAGE_EXT_OWNER, &new_ext->flags);
+}
+
 static ssize_t
 print_page_owner(char __user *buf, size_t count, unsigned long pfn,
 		struct page *page, struct page_ext *page_ext)
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1276399 — [PATCH v2 3/9] mm, page_owner: convert page_owner_inited to static key

CONFIG_PAGE_OWNER attempts to impose negligible runtime overhead when enabled
during compilation, but not actually enabled during runtime by boot param
page_owner=on. This overhead can be further reduced using the static key
mechanism, which this patch does.

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 Documentation/vm/page_owner.txt |  9 +++++----
 include/linux/page_owner.h      | 22 ++++++++++------------
 mm/page_owner.c                 |  9 +++++----
 mm/vmstat.c                     |  2 +-
 4 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/Documentation/vm/page_owner.txt b/Documentation/vm/page_owner.txt
index 8f3ce9b..ffff143 100644
--- a/Documentation/vm/page_owner.txt
+++ b/Documentation/vm/page_owner.txt
@@ -28,10 +28,11 @@ with page owner and page owner is disabled in runtime due to no enabling
 boot option, runtime overhead is marginal. If disabled in runtime, it
 doesn't require memory to store owner information, so there is no runtime
 memory overhead. And, page owner inserts just two unlikely branches into
-the page allocator hotpath and if it returns false then allocation is
-done like as the kernel without page owner. These two unlikely branches
-would not affect to allocation performance. Following is the kernel's
-code size change due to this facility.
+the page allocator hotpath and if not enabled, then allocation is done
+like as the kernel without page owner. These two unlikely branches should
+not affect to allocation performance, especially if the static keys jump
+label patching functionality is available. Following is the kernel's code
+size change due to this facility.
 
 - Without page owner
    text    data     bss     dec     hex filename
diff --git a/include/linux/page_owner.h b/include/linux/page_owner.h
index cacaabe..8e2eb15 100644
--- a/include/linux/page_owner.h
+++ b/include/linux/page_owner.h
@@ -1,8 +1,10 @@
 #ifndef __LINUX_PAGE_OWNER_H
 #define __LINUX_PAGE_OWNER_H
 
+#include <linux/jump_label.h>
+
 #ifdef CONFIG_PAGE_OWNER
-extern bool page_owner_inited;
+extern struct static_key_false page_owner_inited;
 extern struct page_ext_operations page_owner_ops;
 
 extern void __reset_page_owner(struct page *page, unsigned int order);
@@ -12,27 +14,23 @@ extern gfp_t __get_page_owner_gfp(struct page *page);
 
 static inline void reset_page_owner(struct page *page, unsigned int order)
 {
-	if (likely(!page_owner_inited))
-		return;
-
-	__reset_page_owner(page, order);
+	if (static_branch_unlikely(&page_owner_inited))
+		__reset_page_owner(page, order);
 }
 
 static inline void set_page_owner(struct page *page,
 			unsigned int order, gfp_t gfp_mask)
 {
-	if (likely(!page_owner_inited))
-		return;
-
-	__set_page_owner(page, order, gfp_mask);
+	if (static_branch_unlikely(&page_owner_inited))
+		__set_page_owner(page, order, gfp_mask);
 }
 
 static inline gfp_t get_page_owner_gfp(struct page *page)
 {
-	if (likely(!page_owner_inited))
+	if (static_branch_unlikely(&page_owner_inited))
+		return __get_page_owner_gfp(page);
+	else
 		return 0;
-
-	return __get_page_owner_gfp(page);
 }
 #else
 static inline void reset_page_owner(struct page *page, unsigned int order)
diff --git a/mm/page_owner.c b/mm/page_owner.c
index f35826e..10a6a46 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -5,10 +5,11 @@
 #include <linux/bootmem.h>
 #include <linux/stacktrace.h>
 #include <linux/page_owner.h>
+#include <linux/jump_label.h>
 #include "internal.h"
 
 static bool page_owner_disabled = true;
-bool page_owner_inited __read_mostly;
+DEFINE_STATIC_KEY_FALSE(page_owner_inited);
 
 static void init_early_allocated_pages(void);
 
@@ -37,7 +38,7 @@ static void init_page_owner(void)
 	if (page_owner_disabled)
 		return;
 
-	page_owner_inited = true;
+	static_branch_enable(&page_owner_inited);
 	init_early_allocated_pages();
 }
 
@@ -157,7 +158,7 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	struct page *page;
 	struct page_ext *page_ext;
 
-	if (!page_owner_inited)
+	if (!static_branch_unlikely(&page_owner_inited))
 		return -EINVAL;
 
 	page = NULL;
@@ -305,7 +306,7 @@ static int __init pageowner_init(void)
 {
 	struct dentry *dentry;
 
-	if (!page_owner_inited) {
+	if (!static_branch_unlikely(&page_owner_inited)) {
 		pr_info("page_owner is disabled\n");
 		return 0;
 	}
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 53b722b..3f7ec14 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1117,7 +1117,7 @@ static void pagetypeinfo_showmixedcount(struct seq_file *m, pg_data_t *pgdat)
 #ifdef CONFIG_PAGE_OWNER
 	int mtype;
 
-	if (!page_owner_inited)
+	if (!static_branch_unlikely(&page_owner_inited))
 		return;
 
 	drain_all_pages(NULL);
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277440 — Re: [PATCH v2 3/9] mm, page_owner: convert page_owner_inited to static key

On Tue 24-11-15 13:36:15, Vlastimil Babka wrote:
> CONFIG_PAGE_OWNER attempts to impose negligible runtime overhead when enabled
> during compilation, but not actually enabled during runtime by boot param
> page_owner=on. This overhead can be further reduced using the static key
> mechanism, which this patch does.

Is this really worth doing? If we do not have jump labels then the check
will be atomic rather than a simple access, so it would be more costly,
no? Or am I missing something?
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277451 — Re: [PATCH v2 3/9] mm, page_owner: convert page_owner_inited to static key

[+CC PeterZ]

On 11/25/2015 03:52 PM, Michal Hocko wrote:
> On Tue 24-11-15 13:36:15, Vlastimil Babka wrote:
>> CONFIG_PAGE_OWNER attempts to impose negligible runtime overhead when enabled
>> during compilation, but not actually enabled during runtime by boot param
>> page_owner=on. This overhead can be further reduced using the static key
>> mechanism, which this patch does.
> 
> Is this really worth doing?

Well, I assume that jump labels exist for a reason, and allocation hot paths are
sufficiently sensitive to be worth it? It's not an extra maintenance burden for
us anyway. Just a bit different content of the if () line.

> If we do not have jump labels then the check
> will be atomic rather than a simple access, so it would be more costly,
> no? Or am I missing something?

Well, atomic read is a simple READ_ONCE on x86_64. That excludes some compiler
optimizations, but it's not expensive for the CPU. The optimization would be
caching the value of the flag to a register, which would only potentially affect
multiple checks from the same function (and its inlines). Which doesn't happen
AFAIK, as it's just once in the allocation and once in the free path?

Now I admit I have no idea if there are architectures that don't support jump
labels *and* have an expensive atomic read, and whether we care?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277479 — Re: [PATCH v2 3/9] mm, page_owner: convert page_owner_inited to static key

On Wed, Nov 25, 2015 at 04:08:11PM +0100, Vlastimil Babka wrote:
> Now I admit I have no idea if there are architectures that don't support jump
> labels *and* have an expensive atomic read, and whether we care?

atomic_read() is basically always READ_ONCE(), there's a few archs that
implement it in asm with a 'weird' load instruction, but its still a
load. The worst is I think an uncached load for blackfin or somesuch.

There's plenty archs that do not support the jump label bits, but
typically you don't care much about those. I'm not aware of an arch that
cannot fundamentally implement jump_label support if they wanted to.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277511 — Re: [PATCH v2 3/9] mm, page_owner: convert page_owner_inited to static key

On Wed 25-11-15 16:25:33, Peter Zijlstra wrote:
> On Wed, Nov 25, 2015 at 04:08:11PM +0100, Vlastimil Babka wrote:
> > Now I admit I have no idea if there are architectures that don't support jump
> > labels *and* have an expensive atomic read, and whether we care?
> 
> atomic_read() is basically always READ_ONCE(), there's a few archs that
> implement it in asm with a 'weird' load instruction, but its still a
> load. The worst is I think an uncached load for blackfin or somesuch.
> 
> There's plenty archs that do not support the jump label bits, but
> typically you don't care much about those. I'm not aware of an arch that
> cannot fundamentally implement jump_label support if they wanted to.

OK, I see. Thanks for the clarification! Then I do not have any
objections.

-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1276401 — [PATCH v2 2/9] mm, page_owner: print symbolic migratetype of both page and pageblock

The information in /sys/kernel/debug/page_owner includes the migratetype of
the pageblock the page belongs to. This is also checked against the page's
migratetype (as declared by gfp_flags during its allocation), and the page is
reported as Fallback if its migratetype differs from the pageblock's one.

This is somewhat misleading because in fact fallback allocation is not the only
reason why these two can differ. It also doesn't direcly provide the page's
migratetype, although it's possible to derive that from the gfp_flags.

It's arguably better to print both page and pageblock's migratetype and leave
the interpretation to the consumer than to suggest fallback allocation as the
only possible reason. While at it, we can print the migratetypes as string
the same way as /proc/pagetypeinfo does, as some of the numeric values depend
on kernel configuration. For that, this patch moves the migratetype_names
array from #ifdef CONFIG_PROC_FS part of mm/vmstat.c to mm/page_alloc.c and
exports it.

Example page_owner entry after the patch:

Page allocated via order 0, mask 0x2420848
PFN 512 type Reclaimable Block 1 type Reclaimable Flags   R  LA
 [<ffffffff81164e8a>] __alloc_pages_nodemask+0x15a/0xa30
 [<ffffffff811ab808>] alloc_pages_current+0x88/0x120
 [<ffffffff8115bc36>] __page_cache_alloc+0xe6/0x120
 [<ffffffff8115c226>] pagecache_get_page+0x56/0x200
 [<ffffffff81205892>] __getblk_slow+0xd2/0x2b0
 [<ffffffff81205ab0>] __getblk_gfp+0x40/0x50
 [<ffffffff81206ad7>] __breadahead+0x17/0x50
 [<ffffffffa0437b27>] __ext4_get_inode_loc+0x397/0x3e0 [ext4]

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 include/linux/mmzone.h |  3 +++
 mm/page_alloc.c        | 13 +++++++++++++
 mm/page_owner.c        |  6 +++---
 mm/vmstat.c            | 13 -------------
 4 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 3b6fb71..2bfad18 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -63,6 +63,9 @@ enum {
 	MIGRATE_TYPES
 };
 
+/* In mm/page_alloc.c; keep in sync also with show_migration_types() there */
+extern char * const migratetype_names[MIGRATE_TYPES];
+
 #ifdef CONFIG_CMA
 #  define is_migrate_cma(migratetype) unlikely((migratetype) == MIGRATE_CMA)
 #else
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 35ab351..61a023a 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -229,6 +229,19 @@ static char * const zone_names[MAX_NR_ZONES] = {
 #endif
 };
 
+char * const migratetype_names[MIGRATE_TYPES] = {
+	"Unmovable",
+	"Movable",
+	"Reclaimable",
+	"HighAtomic",
+#ifdef CONFIG_CMA
+	"CMA",
+#endif
+#ifdef CONFIG_MEMORY_ISOLATION
+	"Isolate",
+#endif
+};
+
 compound_page_dtor * const compound_page_dtors[] = {
 	NULL,
 	free_compound_page,
diff --git a/mm/page_owner.c b/mm/page_owner.c
index 983c3a1..f35826e 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -110,11 +110,11 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
 	pageblock_mt = get_pfnblock_migratetype(page, pfn);
 	page_mt  = gfpflags_to_migratetype(page_ext->gfp_mask);
 	ret += snprintf(kbuf + ret, count - ret,
-			"PFN %lu Block %lu type %d %s Flags %s%s%s%s%s%s%s%s%s%s%s%s\n",
+			"PFN %lu type %s Block %lu type %s Flags %s%s%s%s%s%s%s%s%s%s%s%s\n",
 			pfn,
+			migratetype_names[page_mt],
 			pfn >> pageblock_order,
-			pageblock_mt,
-			pageblock_mt != page_mt ? "Fallback" : "        ",
+			migratetype_names[pageblock_mt],
 			PageLocked(page)	? "K" : " ",
 			PageError(page)		? "E" : " ",
 			PageReferenced(page)	? "R" : " ",
diff --git a/mm/vmstat.c b/mm/vmstat.c
index f7ebad2..53b722b 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -921,19 +921,6 @@ static void walk_zones_in_node(struct seq_file *m, pg_data_t *pgdat,
 #endif
 
 #ifdef CONFIG_PROC_FS
-static char * const migratetype_names[MIGRATE_TYPES] = {
-	"Unmovable",
-	"Movable",
-	"Reclaimable",
-	"HighAtomic",
-#ifdef CONFIG_CMA
-	"CMA",
-#endif
-#ifdef CONFIG_MEMORY_ISOLATION
-	"Isolate",
-#endif
-};
-
 static void frag_show_print(struct seq_file *m, pg_data_t *pgdat,
 						struct zone *zone)
 {
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277092 — Re: [PATCH v2 2/9] mm, page_owner: print symbolic migratetype of both page and pageblock

On Tue, Nov 24, 2015 at 01:36:14PM +0100, Vlastimil Babka wrote:
> The information in /sys/kernel/debug/page_owner includes the migratetype of
> the pageblock the page belongs to. This is also checked against the page's
> migratetype (as declared by gfp_flags during its allocation), and the page is
> reported as Fallback if its migratetype differs from the pageblock's one.
> 
> This is somewhat misleading because in fact fallback allocation is not the only
> reason why these two can differ. It also doesn't direcly provide the page's
> migratetype, although it's possible to derive that from the gfp_flags.
> 
> It's arguably better to print both page and pageblock's migratetype and leave
> the interpretation to the consumer than to suggest fallback allocation as the
> only possible reason. While at it, we can print the migratetypes as string
> the same way as /proc/pagetypeinfo does, as some of the numeric values depend
> on kernel configuration. For that, this patch moves the migratetype_names
> array from #ifdef CONFIG_PROC_FS part of mm/vmstat.c to mm/page_alloc.c and
> exports it.
> 
> Example page_owner entry after the patch:
> 
> Page allocated via order 0, mask 0x2420848
> PFN 512 type Reclaimable Block 1 type Reclaimable Flags   R  LA
>  [<ffffffff81164e8a>] __alloc_pages_nodemask+0x15a/0xa30
>  [<ffffffff811ab808>] alloc_pages_current+0x88/0x120
>  [<ffffffff8115bc36>] __page_cache_alloc+0xe6/0x120
>  [<ffffffff8115c226>] pagecache_get_page+0x56/0x200
>  [<ffffffff81205892>] __getblk_slow+0xd2/0x2b0
>  [<ffffffff81205ab0>] __getblk_gfp+0x40/0x50
>  [<ffffffff81206ad7>] __breadahead+0x17/0x50
>  [<ffffffffa0437b27>] __ext4_get_inode_loc+0x397/0x3e0 [ext4]
> 
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> ---
>  include/linux/mmzone.h |  3 +++
>  mm/page_alloc.c        | 13 +++++++++++++
>  mm/page_owner.c        |  6 +++---
>  mm/vmstat.c            | 13 -------------
>  4 files changed, 19 insertions(+), 16 deletions(-)
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 3b6fb71..2bfad18 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -63,6 +63,9 @@ enum {
>  	MIGRATE_TYPES
>  };
>  
> +/* In mm/page_alloc.c; keep in sync also with show_migration_types() there */
> +extern char * const migratetype_names[MIGRATE_TYPES];
> +
>  #ifdef CONFIG_CMA
>  #  define is_migrate_cma(migratetype) unlikely((migratetype) == MIGRATE_CMA)
>  #else
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 35ab351..61a023a 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -229,6 +229,19 @@ static char * const zone_names[MAX_NR_ZONES] = {
>  #endif
>  };
>  
> +char * const migratetype_names[MIGRATE_TYPES] = {
> +	"Unmovable",
> +	"Movable",
> +	"Reclaimable",
> +	"HighAtomic",
> +#ifdef CONFIG_CMA
> +	"CMA",
> +#endif
> +#ifdef CONFIG_MEMORY_ISOLATION
> +	"Isolate",
> +#endif
> +};
> +
>  compound_page_dtor * const compound_page_dtors[] = {
>  	NULL,
>  	free_compound_page,
> diff --git a/mm/page_owner.c b/mm/page_owner.c
> index 983c3a1..f35826e 100644
> --- a/mm/page_owner.c
> +++ b/mm/page_owner.c
> @@ -110,11 +110,11 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
>  	pageblock_mt = get_pfnblock_migratetype(page, pfn);
>  	page_mt  = gfpflags_to_migratetype(page_ext->gfp_mask);
>  	ret += snprintf(kbuf + ret, count - ret,
> -			"PFN %lu Block %lu type %d %s Flags %s%s%s%s%s%s%s%s%s%s%s%s\n",
> +			"PFN %lu type %s Block %lu type %s Flags %s%s%s%s%s%s%s%s%s%s%s%s\n",

How about generalizing dump_flag_names() more and using it here?
This output is neat than dump_flag_names() but not complete.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1276406 — [PATCH v2 8/9] mm, page_alloc: print symbolic gfp_flags on allocation failure

It would be useful to translate gfp_flags into string representation when
printing in case of an allocation failure, especially as the flags have been
undergoing some changes recently and the script ./scripts/gfp-translate needs
a matching source version to be accurate.

Example output:

stapio: page allocation failure: order:9, mode:0x2080020(GFP_ATOMIC)

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 mm/page_alloc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index f806a1a..80349ac 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2711,9 +2711,9 @@ void warn_alloc_failed(gfp_t gfp_mask, unsigned int order, const char *fmt, ...)
 		va_end(args);
 	}
 
-	pr_warn("%s: page allocation failure: order:%u, mode:0x%x\n",
+	pr_warn("%s: page allocation failure: order:%u, mode:0x%x",
 		current->comm, order, gfp_mask);
-
+	dump_gfpflag_names(gfp_mask);
 	dump_stack();
 	if (!should_suppress_show_mem())
 		show_mem(filter);
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277434 — Re: [PATCH v2 8/9] mm, page_alloc: print symbolic gfp_flags on allocation failure

On Tue 24-11-15 13:36:20, Vlastimil Babka wrote:
> It would be useful to translate gfp_flags into string representation when
> printing in case of an allocation failure, especially as the flags have been
> undergoing some changes recently and the script ./scripts/gfp-translate needs
> a matching source version to be accurate.
> 
> Example output:
> 
> stapio: page allocation failure: order:9, mode:0x2080020(GFP_ATOMIC)

I like this _very much_

> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>

If this can be done with a printk formatter it would be even nicer but
this is good enough for the OOM purpose.

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  mm/page_alloc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index f806a1a..80349ac 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2711,9 +2711,9 @@ void warn_alloc_failed(gfp_t gfp_mask, unsigned int order, const char *fmt, ...)
>  		va_end(args);
>  	}
>  
> -	pr_warn("%s: page allocation failure: order:%u, mode:0x%x\n",
> +	pr_warn("%s: page allocation failure: order:%u, mode:0x%x",
>  		current->comm, order, gfp_mask);
> -
> +	dump_gfpflag_names(gfp_mask);
>  	dump_stack();
>  	if (!should_suppress_show_mem())
>  		show_mem(filter);
> -- 
> 2.6.3

-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1276407 — [PATCH v2 6/9] mm, debug: introduce dump_gfpflag_names() for symbolic printing of gfp_flags

It would be useful to convert gfp_flags into string representation when
printing them in case of allocation failure, OOM etc. There's a script
./scripts/gfp-translate to make this simpler, but it needs the matching version
of the sources to be accurate, and the flags have been undergoing some changes
recently.

The ftrace framework already has this translation in the form of
show_gfp_flags() defined in include/trace/events/gfpflags.h which defines the
translation table internally. Allow reusing the table outside ftrace by putting
it behind __def_gfpflag_names definition and introduce dump_gfpflag_names() to
handle the printing.

While at it, also fill in the names for the flags and flag combinations that
have been missing in the table. GFP_NOWAIT no longer equals to "no flags", so
change the output for no flags to "none".

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 include/linux/mmdebug.h         |  1 +
 include/trace/events/gfpflags.h | 14 +++++++++++---
 mm/debug.c                      | 10 ++++++++++
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h
index c447d80..3b77fab 100644
--- a/include/linux/mmdebug.h
+++ b/include/linux/mmdebug.h
@@ -10,6 +10,7 @@ struct mm_struct;
 extern void dump_page(struct page *page, const char *reason);
 extern void dump_page_badflags(struct page *page, const char *reason,
 			       unsigned long badflags);
+extern void dump_gfpflag_names(unsigned long gfp_flags);
 void dump_vma(const struct vm_area_struct *vma);
 void dump_mm(const struct mm_struct *mm);
 
diff --git a/include/trace/events/gfpflags.h b/include/trace/events/gfpflags.h
index dde6bf0..3d580fd 100644
--- a/include/trace/events/gfpflags.h
+++ b/include/trace/events/gfpflags.h
@@ -8,8 +8,8 @@
  *
  * Thus most bits set go first.
  */
-#define show_gfp_flags(flags)						\
-	(flags) ? __print_flags(flags, "|",				\
+
+#define __def_gfpflag_names						\
 	{(unsigned long)GFP_TRANSHUGE,		"GFP_TRANSHUGE"},	\
 	{(unsigned long)GFP_HIGHUSER_MOVABLE,	"GFP_HIGHUSER_MOVABLE"}, \
 	{(unsigned long)GFP_HIGHUSER,		"GFP_HIGHUSER"},	\
@@ -19,9 +19,13 @@
 	{(unsigned long)GFP_NOFS,		"GFP_NOFS"},		\
 	{(unsigned long)GFP_ATOMIC,		"GFP_ATOMIC"},		\
 	{(unsigned long)GFP_NOIO,		"GFP_NOIO"},		\
+	{(unsigned long)GFP_NOWAIT,		"GFP_NOWAIT"},		\
+	{(unsigned long)__GFP_DMA,		"GFP_DMA"},		\
+	{(unsigned long)__GFP_DMA32,		"GFP_DMA32"},		\
 	{(unsigned long)__GFP_HIGH,		"GFP_HIGH"},		\
 	{(unsigned long)__GFP_ATOMIC,		"GFP_ATOMIC"},		\
 	{(unsigned long)__GFP_IO,		"GFP_IO"},		\
+	{(unsigned long)__GFP_FS,		"GFP_FS"},		\
 	{(unsigned long)__GFP_COLD,		"GFP_COLD"},		\
 	{(unsigned long)__GFP_NOWARN,		"GFP_NOWARN"},		\
 	{(unsigned long)__GFP_REPEAT,		"GFP_REPEAT"},		\
@@ -36,8 +40,12 @@
 	{(unsigned long)__GFP_RECLAIMABLE,	"GFP_RECLAIMABLE"},	\
 	{(unsigned long)__GFP_MOVABLE,		"GFP_MOVABLE"},		\
 	{(unsigned long)__GFP_NOTRACK,		"GFP_NOTRACK"},		\
+	{(unsigned long)__GFP_WRITE,		"GFP_WRITE"},		\
 	{(unsigned long)__GFP_DIRECT_RECLAIM,	"GFP_DIRECT_RECLAIM"},	\
 	{(unsigned long)__GFP_KSWAPD_RECLAIM,	"GFP_KSWAPD_RECLAIM"},	\
 	{(unsigned long)__GFP_OTHER_NODE,	"GFP_OTHER_NODE"}	\
-	) : "GFP_NOWAIT"
 
+#define show_gfp_flags(flags)						\
+	(flags) ? __print_flags(flags, "|",				\
+	__def_gfpflag_names						\
+	) : "none"
diff --git a/mm/debug.c b/mm/debug.c
index d9718fc..1a71a3b 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -9,6 +9,7 @@
 #include <linux/mm.h>
 #include <linux/trace_events.h>
 #include <linux/memcontrol.h>
+#include <trace/events/gfpflags.h>
 
 static const struct trace_print_flags pageflag_names[] = {
 	{1UL << PG_locked,		"locked"	},
@@ -46,6 +47,10 @@ static const struct trace_print_flags pageflag_names[] = {
 #endif
 };
 
+static const struct trace_print_flags gfpflag_names[] = {
+	__def_gfpflag_names
+};
+
 static void dump_flag_names(unsigned long flags,
 			const struct trace_print_flags *names, int count)
 {
@@ -73,6 +78,11 @@ static void dump_flag_names(unsigned long flags,
 	pr_cont(")\n");
 }
 
+void dump_gfpflag_names(unsigned long gfp_flags)
+{
+	dump_flag_names(gfp_flags, gfpflag_names, ARRAY_SIZE(gfpflag_names));
+}
+
 void dump_page_badflags(struct page *page, const char *reason,
 		unsigned long badflags)
 {
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277084 — Re: [PATCH v2 6/9] mm, debug: introduce dump_gfpflag_names() for symbolic printing of gfp_flags

On Tue, Nov 24, 2015 at 01:36:18PM +0100, Vlastimil Babka wrote:
> It would be useful to convert gfp_flags into string representation when
> printing them in case of allocation failure, OOM etc. There's a script
> ./scripts/gfp-translate to make this simpler, but it needs the matching version
> of the sources to be accurate, and the flags have been undergoing some changes
> recently.
> 
> The ftrace framework already has this translation in the form of
> show_gfp_flags() defined in include/trace/events/gfpflags.h which defines the
> translation table internally. Allow reusing the table outside ftrace by putting
> it behind __def_gfpflag_names definition and introduce dump_gfpflag_names() to
> handle the printing.
> 
> While at it, also fill in the names for the flags and flag combinations that
> have been missing in the table. GFP_NOWAIT no longer equals to "no flags", so
> change the output for no flags to "none".
> 
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> ---
>  include/linux/mmdebug.h         |  1 +
>  include/trace/events/gfpflags.h | 14 +++++++++++---
>  mm/debug.c                      | 10 ++++++++++
>  3 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h
> index c447d80..3b77fab 100644
> --- a/include/linux/mmdebug.h
> +++ b/include/linux/mmdebug.h
> @@ -10,6 +10,7 @@ struct mm_struct;
>  extern void dump_page(struct page *page, const char *reason);
>  extern void dump_page_badflags(struct page *page, const char *reason,
>  			       unsigned long badflags);
> +extern void dump_gfpflag_names(unsigned long gfp_flags);
>  void dump_vma(const struct vm_area_struct *vma);
>  void dump_mm(const struct mm_struct *mm);
>  
> diff --git a/include/trace/events/gfpflags.h b/include/trace/events/gfpflags.h
> index dde6bf0..3d580fd 100644
> --- a/include/trace/events/gfpflags.h
> +++ b/include/trace/events/gfpflags.h
> @@ -8,8 +8,8 @@
>   *
>   * Thus most bits set go first.
>   */
> -#define show_gfp_flags(flags)						\
> -	(flags) ? __print_flags(flags, "|",				\
> +
> +#define __def_gfpflag_names						\
>  	{(unsigned long)GFP_TRANSHUGE,		"GFP_TRANSHUGE"},	\
>  	{(unsigned long)GFP_HIGHUSER_MOVABLE,	"GFP_HIGHUSER_MOVABLE"}, \
>  	{(unsigned long)GFP_HIGHUSER,		"GFP_HIGHUSER"},	\
> @@ -19,9 +19,13 @@
>  	{(unsigned long)GFP_NOFS,		"GFP_NOFS"},		\
>  	{(unsigned long)GFP_ATOMIC,		"GFP_ATOMIC"},		\
>  	{(unsigned long)GFP_NOIO,		"GFP_NOIO"},		\
> +	{(unsigned long)GFP_NOWAIT,		"GFP_NOWAIT"},		\
> +	{(unsigned long)__GFP_DMA,		"GFP_DMA"},		\
> +	{(unsigned long)__GFP_DMA32,		"GFP_DMA32"},		\
>  	{(unsigned long)__GFP_HIGH,		"GFP_HIGH"},		\
>  	{(unsigned long)__GFP_ATOMIC,		"GFP_ATOMIC"},		\
>  	{(unsigned long)__GFP_IO,		"GFP_IO"},		\
> +	{(unsigned long)__GFP_FS,		"GFP_FS"},		\
>  	{(unsigned long)__GFP_COLD,		"GFP_COLD"},		\
>  	{(unsigned long)__GFP_NOWARN,		"GFP_NOWARN"},		\
>  	{(unsigned long)__GFP_REPEAT,		"GFP_REPEAT"},		\
> @@ -36,8 +40,12 @@
>  	{(unsigned long)__GFP_RECLAIMABLE,	"GFP_RECLAIMABLE"},	\
>  	{(unsigned long)__GFP_MOVABLE,		"GFP_MOVABLE"},		\
>  	{(unsigned long)__GFP_NOTRACK,		"GFP_NOTRACK"},		\
> +	{(unsigned long)__GFP_WRITE,		"GFP_WRITE"},		\
>  	{(unsigned long)__GFP_DIRECT_RECLAIM,	"GFP_DIRECT_RECLAIM"},	\
>  	{(unsigned long)__GFP_KSWAPD_RECLAIM,	"GFP_KSWAPD_RECLAIM"},	\
>  	{(unsigned long)__GFP_OTHER_NODE,	"GFP_OTHER_NODE"}	\
> -	) : "GFP_NOWAIT"
>  
> +#define show_gfp_flags(flags)						\
> +	(flags) ? __print_flags(flags, "|",				\
> +	__def_gfpflag_names						\
> +	) : "none"

How about moving this to gfp.h or something?
Now, we use it in out of tracepoints so there is no need to keep it
in include/trace/events/xxx.

Thanks.


> diff --git a/mm/debug.c b/mm/debug.c
> index d9718fc..1a71a3b 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -9,6 +9,7 @@
>  #include <linux/mm.h>
>  #include <linux/trace_events.h>
>  #include <linux/memcontrol.h>
> +#include <trace/events/gfpflags.h>
>  
>  static const struct trace_print_flags pageflag_names[] = {
>  	{1UL << PG_locked,		"locked"	},
> @@ -46,6 +47,10 @@ static const struct trace_print_flags pageflag_names[] = {
>  #endif
>  };
>  
> +static const struct trace_print_flags gfpflag_names[] = {
> +	__def_gfpflag_names
> +};
> +
>  static void dump_flag_names(unsigned long flags,
>  			const struct trace_print_flags *names, int count)
>  {
> @@ -73,6 +78,11 @@ static void dump_flag_names(unsigned long flags,
>  	pr_cont(")\n");
>  }
>  
> +void dump_gfpflag_names(unsigned long gfp_flags)
> +{
> +	dump_flag_names(gfp_flags, gfpflag_names, ARRAY_SIZE(gfpflag_names));
> +}
> +
>  void dump_page_badflags(struct page *page, const char *reason,
>  		unsigned long badflags)
>  {
> -- 
> 2.6.3
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277164 — Re: [PATCH v2 6/9] mm, debug: introduce dump_gfpflag_names() for symbolic printing of gfp_flags

On 11/25/2015 09:16 AM, Joonsoo Kim wrote:
> On Tue, Nov 24, 2015 at 01:36:18PM +0100, Vlastimil Babka wrote:
>> --- a/include/trace/events/gfpflags.h
>> +++ b/include/trace/events/gfpflags.h
>> @@ -8,8 +8,8 @@
>>   *
>>   * Thus most bits set go first.
>>   */
>> -#define show_gfp_flags(flags)						\
>> -	(flags) ? __print_flags(flags, "|",				\
>> +
>> +#define __def_gfpflag_names						\
>>  	{(unsigned long)GFP_TRANSHUGE,		"GFP_TRANSHUGE"},	\
>>  	{(unsigned long)GFP_HIGHUSER_MOVABLE,	"GFP_HIGHUSER_MOVABLE"}, \
>>  	{(unsigned long)GFP_HIGHUSER,		"GFP_HIGHUSER"},	\
>> @@ -19,9 +19,13 @@
>>  	{(unsigned long)GFP_NOFS,		"GFP_NOFS"},		\
>>  	{(unsigned long)GFP_ATOMIC,		"GFP_ATOMIC"},		\
>>  	{(unsigned long)GFP_NOIO,		"GFP_NOIO"},		\
>> +	{(unsigned long)GFP_NOWAIT,		"GFP_NOWAIT"},		\
>> +	{(unsigned long)__GFP_DMA,		"GFP_DMA"},		\
>> +	{(unsigned long)__GFP_DMA32,		"GFP_DMA32"},		\
>>  	{(unsigned long)__GFP_HIGH,		"GFP_HIGH"},		\
>>  	{(unsigned long)__GFP_ATOMIC,		"GFP_ATOMIC"},		\
>>  	{(unsigned long)__GFP_IO,		"GFP_IO"},		\
>> +	{(unsigned long)__GFP_FS,		"GFP_FS"},		\
>>  	{(unsigned long)__GFP_COLD,		"GFP_COLD"},		\
>>  	{(unsigned long)__GFP_NOWARN,		"GFP_NOWARN"},		\
>>  	{(unsigned long)__GFP_REPEAT,		"GFP_REPEAT"},		\
>> @@ -36,8 +40,12 @@
>>  	{(unsigned long)__GFP_RECLAIMABLE,	"GFP_RECLAIMABLE"},	\
>>  	{(unsigned long)__GFP_MOVABLE,		"GFP_MOVABLE"},		\
>>  	{(unsigned long)__GFP_NOTRACK,		"GFP_NOTRACK"},		\
>> +	{(unsigned long)__GFP_WRITE,		"GFP_WRITE"},		\
>>  	{(unsigned long)__GFP_DIRECT_RECLAIM,	"GFP_DIRECT_RECLAIM"},	\
>>  	{(unsigned long)__GFP_KSWAPD_RECLAIM,	"GFP_KSWAPD_RECLAIM"},	\
>>  	{(unsigned long)__GFP_OTHER_NODE,	"GFP_OTHER_NODE"}	\
>> -	) : "GFP_NOWAIT"
>>  
>> +#define show_gfp_flags(flags)						\
>> +	(flags) ? __print_flags(flags, "|",				\
>> +	__def_gfpflag_names						\
>> +	) : "none"
> 
> How about moving this to gfp.h or something?
> Now, we use it in out of tracepoints so there is no need to keep it
> in include/trace/events/xxx.

Hm I didn't want to pollute such widely included header with such defines. And
show_gfp_flags shouldn't be there definitely as it depends on __print_flags.
What do others think?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1278516 — Re: [PATCH v2 6/9] mm, debug: introduce dump_gfpflag_names() for symbolic printing of gfp_flags

> On Nov 25, 2015, at 18:28, Vlastimil Babka <vbabka@suse.cz> wrote:
> 
> On 11/25/2015 09:16 AM, Joonsoo Kim wrote:
>> On Tue, Nov 24, 2015 at 01:36:18PM +0100, Vlastimil Babka wrote:
>>> --- a/include/trace/events/gfpflags.h
>>> +++ b/include/trace/events/gfpflags.h
>>> @@ -8,8 +8,8 @@
>>>  *
>>>  * Thus most bits set go first.
>>>  */
>>> -#define show_gfp_flags(flags)						\
>>> -	(flags) ? __print_flags(flags, "|",				\
>>> +
>>> +#define __def_gfpflag_names						\
>>> 	{(unsigned long)GFP_TRANSHUGE,		"GFP_TRANSHUGE"},	\
>>> 	{(unsigned long)GFP_HIGHUSER_MOVABLE,	"GFP_HIGHUSER_MOVABLE"}, \
>>> 	{(unsigned long)GFP_HIGHUSER,		"GFP_HIGHUSER"},	\
>>> @@ -19,9 +19,13 @@
>>> 	{(unsigned long)GFP_NOFS,		"GFP_NOFS"},		\
>>> 	{(unsigned long)GFP_ATOMIC,		"GFP_ATOMIC"},		\
>>> 	{(unsigned long)GFP_NOIO,		"GFP_NOIO"},		\
>>> +	{(unsigned long)GFP_NOWAIT,		"GFP_NOWAIT"},		\
>>> +	{(unsigned long)__GFP_DMA,		"GFP_DMA"},		\
>>> +	{(unsigned long)__GFP_DMA32,		"GFP_DMA32"},		\
>>> 	{(unsigned long)__GFP_HIGH,		"GFP_HIGH"},		\
>>> 	{(unsigned long)__GFP_ATOMIC,		"GFP_ATOMIC"},		\
>>> 	{(unsigned long)__GFP_IO,		"GFP_IO"},		\
>>> +	{(unsigned long)__GFP_FS,		"GFP_FS"},		\
>>> 	{(unsigned long)__GFP_COLD,		"GFP_COLD"},		\
>>> 	{(unsigned long)__GFP_NOWARN,		"GFP_NOWARN"},		\
>>> 	{(unsigned long)__GFP_REPEAT,		"GFP_REPEAT"},		\
>>> @@ -36,8 +40,12 @@
>>> 	{(unsigned long)__GFP_RECLAIMABLE,	"GFP_RECLAIMABLE"},	\
>>> 	{(unsigned long)__GFP_MOVABLE,		"GFP_MOVABLE"},		\
>>> 	{(unsigned long)__GFP_NOTRACK,		"GFP_NOTRACK"},		\
>>> +	{(unsigned long)__GFP_WRITE,		"GFP_WRITE"},		\
>>> 	{(unsigned long)__GFP_DIRECT_RECLAIM,	"GFP_DIRECT_RECLAIM"},	\
>>> 	{(unsigned long)__GFP_KSWAPD_RECLAIM,	"GFP_KSWAPD_RECLAIM"},	\
>>> 	{(unsigned long)__GFP_OTHER_NODE,	"GFP_OTHER_NODE"}	\
>>> -	) : "GFP_NOWAIT"
>>> 
>>> +#define show_gfp_flags(flags)						\
>>> +	(flags) ? __print_flags(flags, "|",				\
>>> +	__def_gfpflag_names						\
>>> +	) : "none"
>> 
>> How about moving this to gfp.h or something?
>> Now, we use it in out of tracepoints so there is no need to keep it
>> in include/trace/events/xxx.
> 
> Hm I didn't want to pollute such widely included header with such defines. And
> show_gfp_flags shouldn't be there definitely as it depends on __print_flags.
> What do others think?
how about add this into standard printk()  format ?
like cpu mask print in printk use %*pb[l]  ,
it define a macro cpumask_pr_args  to print cpumask .

we can also define a new format like %pG  means print flag ,
then it will be useful for other code to use , like dump vma /  mm  flags ..

Thanks





--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277436

On Tue 24-11-15 13:36:12, Vlastimil Babka wrote:
[...]
> For the last point, Kirill requested a human readable printing of gfp_mask and
> migratetype after v1. At that point it probably makes a lot of sense to do the
> same for page alloc failure and OOM warnings. The flags have been undergoing
> revisions recently, and we might be getting reports from various kernel
> versions that differ. The ./scripts/gfp-translate tool needs to be pointed at
> the corresponding sources to be accurate.  The downside is potentially breaking
> scripts that grep these warnings, but it's not a first change done there over
> the years.

Yes this is very helpful! Thanks for doing this.
 
> Note I'm not entirely happy about the dump_gfpflag_names() implementation, due
> to usage of pr_cont() unreliable on SMP (and I've seen spurious newlines in
> dmesg output, while being correct on serial console or /var/log/messages).
> It also doesn't allow plugging the gfp_mask translation into
> /sys/kernel/debug/page_owner where it also could make sense. Maybe a new
> *printf formatting flag?

I wouldn't object. gfp_mask has its own "type" so having a specific
formatter sounds like a good idea to me.

-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web