Re: [RFC patch] vsprintf: Add %pav extension for print_vma_addr

[Joe Perches]

On Wed, 2015-05-27 at 14:19 -0700, Andrew Morton wrote:
> On Wed, 27 May 2015 14:09:31 -0700 Joe Perches <joe@xxxxxxxxxxx> wrote:
> 
> > > Problems when CONFIG_PREEMPT=n.
> > > 
> > > > +	down_read(&mm->mmap_sem);
> > > > +	vma = find_vma(mm, ip);
> > > > +	if (vma && vma->vm_file) {
> > > > +		struct file *f = vma->vm_file;
> > > > +		char *gfp_buf = (char *)__get_free_page(GFP_KERNEL);
> > > 
> > > We shouldn't assume we can use GFP_KERNEL here.  Even if the
> > > preempt_count() worked, we might be in a context which requires
> > > GFP_NOFS or GFP_NOIO.
> > 
> > This code is basically a copy of the existing print_vma_addr()
> > so is that true for all the existing uses too?
> 
> Yeah, the current code is pretty junky.  But normally print_vma_addr()
> should never be called so nobody noticed...
> 
> In e8bff74a Ingo did a fiddle to preempt_conditional_sti() which looks
> like it will address the CONFIG_PREEMPT=n issue, but only on x86.

Maybe this? (using GFP_ATOMIC and __GFP_NOWARN)

 Documentation/printk-formats.txt |  5 +++
 lib/vsprintf.c                   | 66 ++++++++++++++++++++++++++++++++++++----
 2 files changed, 65 insertions(+), 6 deletions(-)

diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
index 2ec6d84..962f82c 100644
--- a/Documentation/printk-formats.txt
+++ b/Documentation/printk-formats.txt
@@ -86,6 +86,11 @@ DMA addresses types dma_addr_t:
 	For printing a dma_addr_t type which can vary based on build options,
 	regardless of the width of the CPU data path. Passed by reference.
 
+VMA addresses: where the content of an unsigned long * is used in find_vma()
+	%pav	"%s[%lx+%lx]",
+	        kbasename(d_path(vma->vm_file)),
+		vma->vm_start, vma->vm_end - vma->vm_start)
+
 Raw buffer as an escaped string:
 
 	%*pE[achnops]
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 8243e2f..bb4fa63 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1313,23 +1313,74 @@ char *netdev_feature_string(char *buf, char *end, const u8 *addr,
 }
 
 static noinline_for_stack
+char *vma_addr(char *buf, char *end, const void *addr,
+	       struct printf_spec spec, const char *fmt)
+{
+	struct mm_struct *mm = current->mm;
+	const char *errmsg;
+	struct vm_area_struct *vma;
+	char *page;
+	char *path;
+
+	/* if we are in atomic contexts (in exception stacks, etc.) */
+	if (preempt_count()) {
+		errmsg = "(atomic context)";
+		goto err_string;
+	}
+
+	down_read(&mm->mmap_sem);
+	vma = find_vma(mm, *(unsigned long *)addr);
+
+	if (!vma || !vma->vm_file) {
+		errmsg = "(find_vma failed)";
+		goto err_up_read;
+	}
+
+	page = (char *)__get_free_page(GFP_ATOMIC | __GFP_NOWARN);
+	if (!page) {
+		errmsg = "(__get_free_page failed)";
+		goto err_up_read;
+	}
+
+	path = d_path(&vma->vm_file->f_path, page, PAGE_SIZE);
+	if (IS_ERR(path))
+		path = "?";
+
+	buf += snprintf(buf, end > buf ? end - buf : 0, "%s[%lx+%lx]",
+			kbasename(path),
+			vma->vm_start, vma->vm_end - vma->vm_start);
+
+	free_page((unsigned long)page);
+	up_read(&mm->mmap_sem);
+	return buf;
+
+err_up_read:
+	up_read(&mm->mmap_sem);
+err_string:
+	return string(buf, end, errmsg, spec);
+}
+
+static noinline_for_stack
 char *address_val(char *buf, char *end, const void *addr,
 		  struct printf_spec spec, const char *fmt)
 {
 	unsigned long long num;
 
-	spec.flags |= SPECIAL | SMALL | ZEROPAD;
-	spec.base = 16;
-
 	switch (fmt[1]) {
+	case 'v':
+		return vma_addr(buf, end, addr, spec, fmt);
 	case 'd':
-		num = *(const dma_addr_t *)addr;
+		spec.flags |= SPECIAL | SMALL | ZEROPAD;
+		spec.base = 16;
 		spec.field_width = sizeof(dma_addr_t) * 2 + 2;
+		num = *(const dma_addr_t *)addr;
 		break;
 	case 'p':
 	default:
-		num = *(const phys_addr_t *)addr;
+		spec.flags |= SPECIAL | SMALL | ZEROPAD;
+		spec.base = 16;
 		spec.field_width = sizeof(phys_addr_t) * 2 + 2;
+		num = *(const phys_addr_t *)addr;
 		break;
 	}
 
@@ -1453,7 +1504,10 @@ int kptr_restrict __read_mostly;
  *              N no separator
  *            The maximum supported length is 64 bytes of the input. Consider
  *            to use print_hex_dump() for the larger input.
- * - 'a[pd]' For address types [p] phys_addr_t, [d] dma_addr_t and derivatives
+ * - 'a[pdv]' For address types:
+ *            [p] phys_addr_t and derivatives (resource_size_t)
+ *            [d] dma_addr_t
+ *            [v] vma_addr
  *           (default assumed to be phys_addr_t, passed by reference)
  * - 'd[234]' For a dentry name (optionally 2-4 last components)
  * - 'D[234]' Same as 'd' but for a struct file


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