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

Groups > linux.kernel > #1280045 > unrolled thread

[PATCH 1/2] mm, printk: introduce new format string for flags

Started byVlastimil Babka <vbabka@suse.cz>
First post2015-11-30 17:20 +0100
Last post2015-12-10 05:10 +0100
Articles 8 on this page of 28 — 7 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/2] mm, printk: introduce new format string for flags Vlastimil Babka <vbabka@suse.cz> - 2015-11-30 17:20 +0100
    [PATCH 2/2] mm, page_owner: provide symbolic page flags and gfp_flags Vlastimil Babka <vbabka@suse.cz> - 2015-11-30 17:20 +0100
    Re: [PATCH 1/2] mm, printk: introduce new format string for flags Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-02 12:10 +0100
      Re: [PATCH 1/2] mm, printk: introduce new format string for flags Vlastimil Babka <vbabka@suse.cz> - 2015-12-02 21:40 +0100
        Re: [PATCH 1/2] mm, printk: introduce new format string for flags Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-03 13:40 +0100
          Re: [PATCH 1/2] mm, printk: introduce new format string for flags Vlastimil Babka <vbabka@suse.cz> - 2015-12-03 14:50 +0100
      [PATCH v2 3/3] mm, debug: move bad flags printing to bad_page() Vlastimil Babka <vbabka@suse.cz> - 2015-12-04 16:20 +0100
      [PATCH v2 2/3] mm, page_owner: provide symbolic page flags and gfp_flags Vlastimil Babka <vbabka@suse.cz> - 2015-12-04 16:20 +0100
      [PATCH v2 1/3] mm, printk: introduce new format string for flags Vlastimil Babka <vbabka@suse.cz> - 2015-12-04 16:20 +0100
        Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-05 21:10 +0100
        Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags Arnd Bergmann <arnd@arndb.de> - 2015-12-09 12:40 +0100
          Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags Vlastimil Babka <vbabka@suse.cz> - 2015-12-09 21:50 +0100
          Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags James Hogan <james.hogan@imgtec.com> - 2015-12-10 13:30 +0100
        Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2015-12-10 04:00 +0100
          Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags Steven Rostedt <rostedt@goodmis.org> - 2015-12-10 05:10 +0100
            Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2015-12-10 05:50 +0100
            Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-10 09:50 +0100
            Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags Vlastimil Babka <vbabka@suse.cz> - 2015-12-10 11:10 +0100
        Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags Steven Rostedt <rostedt@goodmis.org> - 2015-12-10 05:00 +0100
          Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags Vlastimil Babka <vbabka@suse.cz> - 2015-12-10 11:00 +0100
    Re: [PATCH 1/2] mm, printk: introduce new format string for flags yalin wang <yalin.wang2010@gmail.com> - 2015-12-02 18:50 +0100
      Re: [PATCH 1/2] mm, printk: introduce new format string for flags Vlastimil Babka <vbabka@suse.cz> - 2015-12-02 22:20 +0100
        Re: [PATCH 1/2] mm, printk: introduce new format string for flags yalin wang <yalin.wang2010@gmail.com> - 2015-12-03 01:20 +0100
          Re: [PATCH 1/2] mm, printk: introduce new format string for flags Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-03 09:10 +0100
            Re: [PATCH 1/2] mm, printk: introduce new format string for flags yalin wang <yalin.wang2010@gmail.com> - 2015-12-03 19:40 +0100
              Re: [PATCH 1/2] mm, printk: introduce new format string for flags yalin wang <yalin.wang2010@gmail.com> - 2015-12-04 02:10 +0100
              Re: [PATCH 1/2] mm, printk: introduce new format string for flags Vlastimil Babka <vbabka@suse.cz> - 2015-12-04 15:20 +0100
                Re: [PATCH 1/2] mm, printk: introduce new format string for flags Steven Rostedt <rostedt@goodmis.org> - 2015-12-10 05:10 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1282179

Fromyalin wang <yalin.wang2010@gmail.com>
Date2015-12-02 18:50 +0100
Message-ID<qBjHu-1WC-45@gated-at.bofh.it>
In reply to#1280045
> On Nov 30, 2015, at 08:10, Vlastimil Babka <vbabka@suse.cz> wrote:
> 
> In mm we use several kinds of flags bitfields that are sometimes printed for
> debugging purposes, or exported to userspace via sysfs. To make them easier to
> interpret independently on kernel version and config, we want to dump also the
> symbolic flag names. So far this has been done with repeated calls to
> pr_cont(), which is unreliable on SMP, and not usable for e.g. sysfs export.
> 
> To get a more reliable and universal solution, this patch extends printk()
> format string for pointers to handle the page flags (%pgp), gfp_flags (%pgg)
> and vma flags (%pgv). Existing users of dump_flag_names() are converted and
> simplified.
> 
> It would be possible to pass flags by value instead of pointer, but the %p
> format string for pointers already has extensions for various kernel
> structures, so it's a good fit, and the extra indirection in a non-critical
> path is negligible.
> 
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
> I'm sending it on top of the page_owner series, as it's already in mmotm.
> But to reduce churn (in case this approach is accepted), I can later
> incorporate it and resend it whole.
> 
> Documentation/printk-formats.txt |  14 ++++
> include/linux/mmdebug.h          |   5 +-
> lib/vsprintf.c                   |  31 ++++++++
> mm/debug.c                       | 150 ++++++++++++++++++++++-----------------
> mm/oom_kill.c                    |   5 +-
> mm/page_alloc.c                  |   5 +-
> mm/page_owner.c                  |   5 +-
> 7 files changed, 140 insertions(+), 75 deletions(-)
> 
> diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
> index b784c270105f..4b5156e74b09 100644
> --- a/Documentation/printk-formats.txt
> +++ b/Documentation/printk-formats.txt
> @@ -292,6 +292,20 @@ Raw pointer value SHOULD be printed with %p. The kernel supports
> 
> 	Passed by reference.
> 
> +Flags bitfields such as page flags, gfp_flags:
> +
> +	%pgp	0x1fffff8000086c(referenced|uptodate|lru|active|private)
> +	%pgg	0x24202c4(GFP_USER|GFP_DMA32|GFP_NOWARN)
> +	%pgv	0x875(read|exec|mayread|maywrite|mayexec|denywrite)
> +
> +	For printing raw values of flags bitfields together with symbolic
> +	strings that would construct the value. The type of flags is given by
> +	the third character. Currently supported are [p]age flags, [g]fp_flags
> +	and [v]ma_flags. The flag names and print order depends on the
> +	particular type.
> +
> +	Passed by reference.
> +
> Network device features:
> 
> 	%pNF	0x000000000000c000
> diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h
> index 3b77fab7ad28..e6518df259ca 100644
> --- a/include/linux/mmdebug.h
> +++ b/include/linux/mmdebug.h
> @@ -2,6 +2,7 @@
> #define LINUX_MM_DEBUG_H 1
> 
> #include <linux/stringify.h>
> +#include <linux/types.h>
> 
> struct page;
> struct vm_area_struct;
> @@ -10,7 +11,9 @@ 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);
> +extern char *format_page_flags(unsigned long flags, char *buf, char *end);
> +extern char *format_vma_flags(unsigned long flags, char *buf, char *end);
> +extern char *format_gfp_flags(gfp_t gfp_flags, char *buf, char*end);
> void dump_vma(const struct vm_area_struct *vma);
> void dump_mm(const struct mm_struct *mm);
> 
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index f9cee8e1233c..41cd122bd307 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -31,6 +31,7 @@
> #include <linux/dcache.h>
> #include <linux/cred.h>
> #include <net/addrconf.h>
> +#include <linux/mmdebug.h>
> 
> #include <asm/page.h>		/* for PAGE_SIZE */
> #include <asm/sections.h>	/* for dereference_function_descriptor() */
> @@ -1361,6 +1362,29 @@ char *clock(char *buf, char *end, struct clk *clk, struct printf_spec spec,
> 	}
> }
> 
> +static noinline_for_stack
> +char *flags_string(char *buf, char *end, void *flags_ptr,
> +			struct printf_spec spec, const char *fmt)
> +{
> +	unsigned long flags;
> +	gfp_t gfp_flags;
> +
> +	switch (fmt[1]) {
> +	case 'p':
> +		flags = *(unsigned long *)flags_ptr;
> +		return format_page_flags(flags, buf, end);
> +	case 'v':
> +		flags = *(unsigned long *)flags_ptr;
> +		return format_vma_flags(flags, buf, end);
> +	case 'g':
> +		gfp_flags = *(gfp_t *)flags_ptr;
> +		return format_gfp_flags(gfp_flags, buf, end);
> +	default:
> +		WARN_ONCE(1, "Unsupported flags modifier: %c\n", fmt[1]);
> +		return 0;
> +	}
> +}
> +
> int kptr_restrict __read_mostly;
> 
> /*
> @@ -1448,6 +1472,11 @@ int kptr_restrict __read_mostly;
>  * - 'Cn' For a clock, it prints the name (Common Clock Framework) or address
>  *        (legacy clock framework) of the clock
>  * - 'Cr' For a clock, it prints the current rate of the clock
> + * - 'g' For flags to be printed as a collection of symbolic strings that would
> + *       construct the specific value. Supported flags given by option:
> + *       p page flags (see struct page) given as pointer to unsigned long
> + *       g gfp flags (GFP_* and __GFP_*) given as pointer to gfp_t
> + *       v vma flags (VM_*) given as pointer to unsigned long
>  *
>  * ** Please update also Documentation/printk-formats.txt when making changes **
>  *
> @@ -1600,6 +1629,8 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
> 		return dentry_name(buf, end,
> 				   ((const struct file *)ptr)->f_path.dentry,
> 				   spec, fmt);
> +	case 'g':
> +		return flags_string(buf, end, ptr, spec, fmt);
> 	}
> 	spec.flags |= SMALL;
> 	if (spec.field_width == -1) {
> diff --git a/mm/debug.c b/mm/debug.c
> index 2fdf0999e6f9..a092111920e7 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -59,40 +59,109 @@ static const struct trace_print_flags pageflag_names[] = {
> #endif
> };
> 
> +static const struct trace_print_flags vmaflags_names[] = {
> +	{VM_READ,			"read"		},
> +	{VM_WRITE,			"write"		},
> +	{VM_EXEC,			"exec"		},
> +	{VM_SHARED,			"shared"	},
> +	{VM_MAYREAD,			"mayread"	},
> +	{VM_MAYWRITE,			"maywrite"	},
> +	{VM_MAYEXEC,			"mayexec"	},
> +	{VM_MAYSHARE,			"mayshare"	},
> +	{VM_GROWSDOWN,			"growsdown"	},
> +	{VM_PFNMAP,			"pfnmap"	},
> +	{VM_DENYWRITE,			"denywrite"	},
> +	{VM_LOCKONFAULT,		"lockonfault"	},
> +	{VM_LOCKED,			"locked"	},
> +	{VM_IO,				"io"		},
> +	{VM_SEQ_READ,			"seqread"	},
> +	{VM_RAND_READ,			"randread"	},
> +	{VM_DONTCOPY,			"dontcopy"	},
> +	{VM_DONTEXPAND,			"dontexpand"	},
> +	{VM_ACCOUNT,			"account"	},
> +	{VM_NORESERVE,			"noreserve"	},
> +	{VM_HUGETLB,			"hugetlb"	},
> +#if defined(CONFIG_X86)
> +	{VM_PAT,			"pat"		},
> +#elif defined(CONFIG_PPC)
> +	{VM_SAO,			"sao"		},
> +#elif defined(CONFIG_PARISC) || defined(CONFIG_METAG) || defined(CONFIG_IA64)
> +	{VM_GROWSUP,			"growsup"	},
> +#elif !defined(CONFIG_MMU)
> +	{VM_MAPPED_COPY,		"mappedcopy"	},
> +#else
> +	{VM_ARCH_1,			"arch_1"	},
> +#endif
> +	{VM_DONTDUMP,			"dontdump"	},
> +#ifdef CONFIG_MEM_SOFT_DIRTY
> +	{VM_SOFTDIRTY,			"softdirty"	},
> +#endif
> +	{VM_MIXEDMAP,			"mixedmap"	},
> +	{VM_HUGEPAGE,			"hugepage"	},
> +	{VM_NOHUGEPAGE,			"nohugepage"	},
> +	{VM_MERGEABLE,			"mergeable"	},
> +};
> +
> 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)
> +static char *format_flag_names(unsigned long flags, unsigned long mask_out,
> +		const struct trace_print_flags *names, int count,
> +		char *buf, char *end)
> {
> 	const char *delim = "";
> 	unsigned long mask;
> 	int i;
> 
> -	pr_cont("(");
> +	buf += snprintf(buf, end - buf, "%#lx(", flags);
> +
> +	flags &= ~mask_out;
> 
> 	for (i = 0; i < count && flags; i++) {
> +		if (buf >= end)
> +			break;
> 
> 		mask = names[i].mask;
> 		if ((flags & mask) != mask)
> 			continue;
> 
> 		flags &= ~mask;
> -		pr_cont("%s%s", delim, names[i].name);
> +		buf += snprintf(buf, end - buf, "%s%s", delim, names[i].name);
> 		delim = "|";
> 	}
> 
> 	/* check for left over flags */
> -	if (flags)
> -		pr_cont("%s%#lx", delim, flags);
> +	if (flags && (buf < end))
> +		buf += snprintf(buf, end - buf, "%s%#lx", delim, flags);
> +
> +	if (buf < end) {
> +		*buf = ')';
> +		buf++;
> +	}
> 
> -	pr_cont(")\n");
> +	return buf;
> }
> 
> -void dump_gfpflag_names(unsigned long gfp_flags)
> +char *format_page_flags(unsigned long flags, char *buf, char *end)
> {
> -	dump_flag_names(gfp_flags, gfpflag_names, ARRAY_SIZE(gfpflag_names));
> +	/* remove zone id */
> +	unsigned long mask = (1UL << NR_PAGEFLAGS) - 1;
> +
> +	return format_flag_names(flags, ~mask, pageflag_names,
> +					ARRAY_SIZE(pageflag_names), buf, end);
> +}
> +
> +char *format_vma_flags(unsigned long flags, char *buf, char *end)
> +{
> +	return format_flag_names(flags, 0, vmaflags_names,
> +					ARRAY_SIZE(vmaflags_names), buf, end);
> +}
> +
> +char *format_gfp_flags(gfp_t gfp_flags, char *buf, char *end)
> +{
> +	return format_flag_names(gfp_flags, 0, gfpflag_names,
> +					ARRAY_SIZE(gfpflag_names), buf, end);
> }
> 
> void dump_page_badflags(struct page *page, const char *reason,
> @@ -108,18 +177,15 @@ void dump_page_badflags(struct page *page, const char *reason,
> 	pr_cont("\n");
> 	BUILD_BUG_ON(ARRAY_SIZE(pageflag_names) != __NR_PAGEFLAGS);
> 
> -	pr_emerg("flags: %#lx", printflags);
> +	pr_emerg("flags: %pgp\n", &printflags);
> 	/* remove zone id */
> 	printflags &= (1UL << NR_PAGEFLAGS) - 1;
> -	dump_flag_names(printflags, pageflag_names, ARRAY_SIZE(pageflag_names));
> 
> 	if (reason)
> 		pr_alert("page dumped because: %s\n", reason);
> 	if (page->flags & badflags) {
> 		printflags = page->flags & badflags;
> -		pr_alert("bad because of flags: %#lx:", printflags);
> -		dump_flag_names(printflags, pageflag_names,
> -						ARRAY_SIZE(pageflag_names));
> +		pr_alert("bad because of flags: %pgp\n", &printflags);
> 	}
> #ifdef CONFIG_MEMCG
> 	if (page->mem_cgroup)
> @@ -136,63 +202,19 @@ EXPORT_SYMBOL(dump_page);
> 
> #ifdef CONFIG_DEBUG_VM
> 
> -static const struct trace_print_flags vmaflags_names[] = {
> -	{VM_READ,			"read"		},
> -	{VM_WRITE,			"write"		},
> -	{VM_EXEC,			"exec"		},
> -	{VM_SHARED,			"shared"	},
> -	{VM_MAYREAD,			"mayread"	},
> -	{VM_MAYWRITE,			"maywrite"	},
> -	{VM_MAYEXEC,			"mayexec"	},
> -	{VM_MAYSHARE,			"mayshare"	},
> -	{VM_GROWSDOWN,			"growsdown"	},
> -	{VM_PFNMAP,			"pfnmap"	},
> -	{VM_DENYWRITE,			"denywrite"	},
> -	{VM_LOCKONFAULT,		"lockonfault"	},
> -	{VM_LOCKED,			"locked"	},
> -	{VM_IO,				"io"		},
> -	{VM_SEQ_READ,			"seqread"	},
> -	{VM_RAND_READ,			"randread"	},
> -	{VM_DONTCOPY,			"dontcopy"	},
> -	{VM_DONTEXPAND,			"dontexpand"	},
> -	{VM_ACCOUNT,			"account"	},
> -	{VM_NORESERVE,			"noreserve"	},
> -	{VM_HUGETLB,			"hugetlb"	},
> -#if defined(CONFIG_X86)
> -	{VM_PAT,			"pat"		},
> -#elif defined(CONFIG_PPC)
> -	{VM_SAO,			"sao"		},
> -#elif defined(CONFIG_PARISC) || defined(CONFIG_METAG) || defined(CONFIG_IA64)
> -	{VM_GROWSUP,			"growsup"	},
> -#elif !defined(CONFIG_MMU)
> -	{VM_MAPPED_COPY,		"mappedcopy"	},
> -#else
> -	{VM_ARCH_1,			"arch_1"	},
> -#endif
> -	{VM_DONTDUMP,			"dontdump"	},
> -#ifdef CONFIG_MEM_SOFT_DIRTY
> -	{VM_SOFTDIRTY,			"softdirty"	},
> -#endif
> -	{VM_MIXEDMAP,			"mixedmap"	},
> -	{VM_HUGEPAGE,			"hugepage"	},
> -	{VM_NOHUGEPAGE,			"nohugepage"	},
> -	{VM_MERGEABLE,			"mergeable"	},
> -};
> -
> void dump_vma(const struct vm_area_struct *vma)
> {
> 	pr_emerg("vma %p start %p end %p\n"
> 		"next %p prev %p mm %p\n"
> 		"prot %lx anon_vma %p vm_ops %p\n"
> -		"pgoff %lx file %p private_data %p\n",
> +		"pgoff %lx file %p private_data %p\n"
> +		"flags: %pgv\n",
> 		vma, (void *)vma->vm_start, (void *)vma->vm_end, vma->vm_next,
> 		vma->vm_prev, vma->vm_mm,
> 		(unsigned long)pgprot_val(vma->vm_page_prot),
> 		vma->anon_vma, vma->vm_ops, vma->vm_pgoff,
> -		vma->vm_file, vma->vm_private_data);
> -	pr_emerg("flags: %#lx", vma->vm_flags);
> -	dump_flag_names(vma->vm_flags, vmaflags_names,
> -						ARRAY_SIZE(vmaflags_names));
> +		vma->vm_file, vma->vm_private_data,
> +		&vma->vm_flags);
> }
> EXPORT_SYMBOL(dump_vma);
> 
> @@ -263,9 +285,7 @@ void dump_mm(const struct mm_struct *mm)
> 		""		/* This is here to not have a comma! */
> 		);
> 
> -	pr_emerg("def_flags: %#lx", mm->def_flags);
> -	dump_flag_names(mm->def_flags, vmaflags_names,
> -					ARRAY_SIZE(vmaflags_names));
> +	pr_emerg("def_flags: %pgv\n", &mm->def_flags);
> }
> 
> #endif		/* CONFIG_DEBUG_VM */
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 542d56c93209..63a68b62ee68 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -387,10 +387,9 @@ static void dump_header(struct oom_control *oc, struct task_struct *p,
> 			struct mem_cgroup *memcg)
> {
> 	pr_warning("%s invoked oom-killer: order=%d, oom_score_adj=%hd, "
> -			"gfp_mask=0x%x",
> +			"gfp_mask=%pgg\n",
> 		current->comm, oc->order, current->signal->oom_score_adj,
> -		oc->gfp_mask);
> -	dump_gfpflag_names(oc->gfp_mask);
> +		&oc->gfp_mask);
> 
> 	cpuset_print_current_mems_allowed();
> 	dump_stack();
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 80349acd8c17..77d2c75f80e4 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2711,9 +2711,8 @@ 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",
> -		current->comm, order, gfp_mask);
> -	dump_gfpflag_names(gfp_mask);
> +	pr_warn("%s: page allocation failure: order:%u, mode:%pgg\n",
> +		current->comm, order, &gfp_mask);
> 	dump_stack();
> 	if (!should_suppress_show_mem())
> 		show_mem(filter);
> diff --git a/mm/page_owner.c b/mm/page_owner.c
> index f4acd2452c35..ff862b6d12da 100644
> --- a/mm/page_owner.c
> +++ b/mm/page_owner.c
> @@ -208,9 +208,8 @@ void __dump_page_owner(struct page *page)
> 		return;
> 	}
> 
> -	pr_alert("page allocated via order %u, migratetype %s, gfp_mask 0x%x",
> -			page_ext->order, migratetype_names[mt], gfp_mask);
> -	dump_gfpflag_names(gfp_mask);
> +	pr_alert("page allocated via order %u, migratetype %s, gfp_mask %pgg\n",
> +			page_ext->order, migratetype_names[mt], &gfp_mask);
> 	print_stack_trace(&trace, 0);
> 
> 	if (page_ext->last_migrate_reason != -1)
> -- 
> 2.6.3
> 
i am thinking why not make %pg* to be more generic ?
not restricted to only GFP / vma flags / page flags .
so could we change format like this ?
define a flag spec struct to include flag and trace_print_flags and some other option :
typedef struct { 
unsigned long flag;
struct trace_print_flags *flags;
unsigned long option; } flag_sec;
flag_sec my_flag;
in printk we only pass like this :
printk(“%pg\n”, &my_flag) ;
then it can print any flags defined by user .
more useful for other drivers to use .

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]

#1282421

FromVlastimil Babka <vbabka@suse.cz>
Date2015-12-02 22:20 +0100
Message-ID<qBmYG-4b2-3@gated-at.bofh.it>
In reply to#1282179
On 12/02/2015 06:40 PM, yalin wang wrote:

(please trim your reply next time, no need to quote whole patch here)

> i am thinking why not make %pg* to be more generic ?
> not restricted to only GFP / vma flags / page flags .
> so could we change format like this ?
> define a flag spec struct to include flag and trace_print_flags and some other option :
> typedef struct { 
> unsigned long flag;
> struct trace_print_flags *flags;
> unsigned long option; } flag_sec;
> flag_sec my_flag;
> in printk we only pass like this :
> printk(“%pg\n”, &my_flag) ;
> then it can print any flags defined by user .
> more useful for other drivers to use .

I don't know, it sounds quite complicated given that we had no flags printing
for years and now there's just three kinds of them. The extra struct flag_sec is
IMHO nuissance. No other printk format needs such thing AFAIK? For example, if I
were to print page flags from several places, each would have to define the
struct flag_sec instance, or some header would have to provide it?

I could maybe accept passing a flag value and trace_print_flags * as two
separate parameters, but I guess that breaks an ancient invariant of one
parameter per format string...

> 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]

#1282623

Fromyalin wang <yalin.wang2010@gmail.com>
Date2015-12-03 01:20 +0100
Message-ID<qBpMS-5Y5-23@gated-at.bofh.it>
In reply to#1282421
> On Dec 2, 2015, at 13:04, Vlastimil Babka <vbabka@suse.cz> wrote:
> 
> On 12/02/2015 06:40 PM, yalin wang wrote:
> 
> (please trim your reply next time, no need to quote whole patch here)
> 
>> i am thinking why not make %pg* to be more generic ?
>> not restricted to only GFP / vma flags / page flags .
>> so could we change format like this ?
>> define a flag spec struct to include flag and trace_print_flags and some other option :
>> typedef struct { 
>> unsigned long flag;
>> structtrace_print_flags *flags;
>> unsigned long option; } flag_sec;
>> flag_sec my_flag;
>> in printk we only pass like this :
>> printk(“%pg\n”, &my_flag) ;
>> then it can print any flags defined by user .
>> more useful for other drivers to use .
> 
> I don't know, it sounds quite complicated given that we had no flags printing
> for years and now there's just three kinds of them. The extra struct flag_sec is
> IMHO nuissance. No other printk format needs such thing AFAIK? For example, if I
> were to print page flags from several places, each would have to define the
> struct flag_sec instance, or some header would have to provide it?
this can be avoided by provide a macro in header file .
we can add a new struct to declare trace_print_flags :
for example:
#define DECLARE_FLAG_PRINTK_FMT(name, flags_array)   flag_spec name = { .flags = flags_array};
#define FLAG_PRINTK_FMT(name, flag) ({  name.flag = flag;  &name})

in source code :
DECLARE_FLAG_PRINTK_FMT(my_flag, vmaflags_names);
printk(“%pg\n”, FLAG_PRINTK_FMT(my_flag, vma->flag));

i am not if DECLARE_FLAG_PRINTK_FMT and FLAG_PRINTK_FMT macro 
can be defined into one macro ?
maybe need some trick here .

is it possible ?


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]

#1282768

FromRasmus Villemoes <linux@rasmusvillemoes.dk>
Date2015-12-03 09:10 +0100
Message-ID<qBx7I-2kH-15@gated-at.bofh.it>
In reply to#1282623
On Thu, Dec 03 2015, yalin wang <yalin.wang2010@gmail.com> wrote:

>> On Dec 2, 2015, at 13:04, Vlastimil Babka <vbabka@suse.cz> wrote:
>> 
>> On 12/02/2015 06:40 PM, yalin wang wrote:
>> 
>> (please trim your reply next time, no need to quote whole patch here)
>> 
>>> i am thinking why not make %pg* to be more generic ?
>>> not restricted to only GFP / vma flags / page flags .
>>> so could we change format like this ?
>>> define a flag spec struct to include flag and trace_print_flags and some other option :
>>> typedef struct { 
>>> unsigned long flag;
>>> structtrace_print_flags *flags;
>>> unsigned long option; } flag_sec;
>>> flag_sec my_flag;
>>> in printk we only pass like this :
>>> printk(“%pg\n”, &my_flag) ;
>>> then it can print any flags defined by user .
>>> more useful for other drivers to use .
>> 
>> I don't know, it sounds quite complicated

Agreed, I think this would be premature generalization. There's also
some value in having the individual %pgX specifiers, as that allows
individual tweaks such as the mask_out for page flags.

 given that we had no flags printing
>> for years and now there's just three kinds of them. The extra struct flag_sec is
>> IMHO nuissance. No other printk format needs such thing AFAIK? For example, if I
>> were to print page flags from several places, each would have to define the
>> struct flag_sec instance, or some header would have to provide it?
> this can be avoided by provide a macro in header file .
> we can add a new struct to declare trace_print_flags :
> for example:
> #define DECLARE_FLAG_PRINTK_FMT(name, flags_array)   flag_spec name = { .flags = flags_array};
> #define FLAG_PRINTK_FMT(name, flag) ({  name.flag = flag;  &name})
>
> in source code :
> DECLARE_FLAG_PRINTK_FMT(my_flag, vmaflags_names);
> printk(“%pg\n”, FLAG_PRINTK_FMT(my_flag, vma->flag));
>

Compared to printk("%pgv\n", &vma->flag), I know which I'd prefer to read.

> i am not if DECLARE_FLAG_PRINTK_FMT and FLAG_PRINTK_FMT macro 
> can be defined into one macro ?
> maybe need some trick here .
>
> is it possible ?

Technically, I think the answer is yes, at least in C99 (and I suppose
gcc would accept it in gnu89 mode as well).

printk("%pg\n", &(struct flag_printer){.flags = my_flags, .names = vmaflags_names});

Not tested, and I still don't think it would be particularly readable
even when macroized

printk("%pg\n", PRINTF_VMAFLAGS(my_flags));

Rasmus
--
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]

#1283251

Fromyalin wang <yalin.wang2010@gmail.com>
Date2015-12-03 19:40 +0100
Message-ID<qBGXo-j5-25@gated-at.bofh.it>
In reply to#1282768
> On Dec 3, 2015, at 00:03, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
> 
> On Thu, Dec 03 2015, yalin wang <yalin.wang2010@gmail.com> wrote:
> 
>>> On Dec 2, 2015, at 13:04, Vlastimil Babka <vbabka@suse.cz> wrote:
>>> 
>>> On 12/02/2015 06:40 PM, yalin wang wrote:
>>> 
>>> (please trim your reply next time, no need to quote whole patch here)
>>> 
>>>> i am thinking why not make %pg* to be more generic ?
>>>> not restricted to only GFP / vma flags / page flags .
>>>> so could we change format like this ?
>>>> define a flag spec struct to include flag and trace_print_flags and some other option :
>>>> typedef struct { 
>>>> unsigned long flag;
>>>> structtrace_print_flags *flags;
>>>> unsigned long option; } flag_sec;
>>>> flag_sec my_flag;
>>>> in printk we only pass like this :
>>>> printk(“%pg\n”, &my_flag) ;
>>>> then it can print any flags defined by user .
>>>> more useful for other drivers to use .
>>> 
>>> I don't know, it sounds quite complicated
> 
> Agreed, I think this would be premature generalization. There's also
> some value in having the individual %pgX specifiers, as that allows
> individual tweaks such as the mask_out for page flags.
> 
> given that we had no flags printing
>> 
if we use this generic method, %pgX where X can be used to specify some flag to
mask out some thing .  it will be great .

> 
> Compared to printk("%pgv\n", &vma->flag), I know which I'd prefer to read.
> 
>> i am not if DECLARE_FLAG_PRINTK_FMT and FLAG_PRINTK_FMT macro 
>> can be defined into one macro ?
>> maybe need some trick here .
>> 
>> is it possible ?
> 
> Technically, I think the answer is yes, at least in C99 (and I suppose
> gcc would accept it in gnu89 mode as well).
> 
> printk("%pg\n", &(struct flag_printer){.flags = my_flags, .names = vmaflags_names});
> 
> Not tested, and I still don't think it would be particularly readable
> even when macroized
> 
> printk("%pg\n", PRINTF_VMAFLAGS(my_flags));
i test on gcc 4.9.3, it can work for this method,
so the final solution like this:
printk.h:
struct flag_fmt_spec {
	unsigned long flag;
	struct trace_print_flags *flags;
	int array_size;
	char delimiter; }

#define FLAG_FORMAT(flag, flag_array, delimiter) (&(struct flag_ft_spec){ .flag = flag, .flags = flag_array, .array_size = ARRAY_SIZE(flag_array), .delimiter = delimiter})
#define VMA_FLAG_FORMAT(flag)  FLAG_FORMAT(flag, vmaflags_names, ‘|')

source code:
printk("%pg\n", VMA_FLAG_FORMAT(my_flags)); 

that’s all, see cpumask_pr_args(masks) macro,
it also use macro and  %*pb  to print cpu mask .
i think this method is not very complex to use .

search source code ,
there is lots of printk to print flag into hex number :
$ grep -n  -r 'printk.*flag.*%x’  .
it will be great if this flag string print is generic.

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]

#1283491

Fromyalin wang <yalin.wang2010@gmail.com>
Date2015-12-04 02:10 +0100
Message-ID<qBN2R-4ko-63@gated-at.bofh.it>
In reply to#1283251
>> Technically, I think the answer is yes, at least in C99 (and I suppose
>> gcc would accept it in gnu89 mode as well).
>> 
>> printk("%pg\n", &(struct flag_printer){.flags = my_flags, .names = vmaflags_names});
>> 
>> Not tested, and I still don't think it would be particularly readable
>> even when macroized
>> 
>> printk("%pg\n", PRINTF_VMAFLAGS(my_flags));
> i test on gcc 4.9.3, it can work for this method,
> so the final solution like this:
> printk.h:
> struct flag_fmt_spec {
> 	unsigned long flag;
> 	struct trace_print_flags *flags;
> 	int array_size;
> 	char delimiter; }
> 
> #define FLAG_FORMAT(flag, flag_array, delimiter) (&(struct flag_ft_spec){ .flag = flag, .flags = flag_array, .array_size = ARRAY_SIZE(flag_array), .delimiter = delimiter})
> #define VMA_FLAG_FORMAT(flag)  FLAG_FORMAT(flag, vmaflags_names, ‘|’)
a little change:
	#define VMA_FLAG_FORMAT(vma)  FLAG_FORMAT(vma->vm_flags, vmaflags_names, ‘|’)


> source code:
> printk("%pg\n", VMA_FLAG_FORMAT(my_flags)); 
a little change:
	printk("%pg\n", VMA_FLAG_FORMAT(vma)); 

> 
> that’s all, see cpumask_pr_args(masks) macro,
> it also use macro and  %*pb  to print cpu mask .
> i think this method is not very complex to use .
> 
> search source code ,
> there is lots of printk to print flag into hex number :
> $ grep -n  -r 'printk.*flag.*%x’  .
> it will be great if this flag string print is generic.
> 
> 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]

#1283846

FromVlastimil Babka <vbabka@suse.cz>
Date2015-12-04 15:20 +0100
Message-ID<qBZnm-3Km-31@gated-at.bofh.it>
In reply to#1283251
On 12/03/2015 07:38 PM, yalin wang wrote:
> that’s all, see cpumask_pr_args(masks) macro,
> it also use macro and  %*pb  to print cpu mask .
> i think this method is not very complex to use .

Well, one also has to write the appropriate translation tables.

> search source code ,
> there is lots of printk to print flag into hex number :
> $ grep -n  -r 'printk.*flag.*%x’  .
> it will be great if this flag string print is generic.

I think it can always be done later, this is an internal API. For now we 
just have 3 quite generic flags, so let's not over-engineer things right 
now.

> 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]

#1288178

FromSteven Rostedt <rostedt@goodmis.org>
Date2015-12-10 05:10 +0100
Message-ID<qE0Ih-11h-1@gated-at.bofh.it>
In reply to#1283846
On Fri, Dec 04, 2015 at 03:15:45PM +0100, Vlastimil Babka wrote:
> On 12/03/2015 07:38 PM, yalin wang wrote:
> >that’s all, see cpumask_pr_args(masks) macro,
> >it also use macro and  %*pb  to print cpu mask .
> >i think this method is not very complex to use .
> 
> Well, one also has to write the appropriate translation tables.
> 
> >search source code ,
> >there is lots of printk to print flag into hex number :
> >$ grep -n  -r 'printk.*flag.*%x’  .
> >it will be great if this flag string print is generic.
> 
> I think it can always be done later, this is an internal API. For now we
> just have 3 quite generic flags, so let's not over-engineer things right
> now.
> 

As long as it is never used in the TP_printk() part of a tracepoint. As soon
as it is, trace-cmd and perf will update parse-events to handle that
parameter, and as soon as that is done, it becomes a userspace ABI.

Just be warned.

-- Steve

--
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]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web