All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jia He <justin.he@arm.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	Petr Mladek <pmladek@suse.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Jonathan Corbet <corbet@lwn.net>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Luca Coelho <luciano.coelho@intel.com>,
	Kalle Valo <kvalo@codeaurora.org>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Heiko Carstens <hca@linux.ibm.com>,
	Vasily Gorbik <gor@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>,
	Johannes Berg <johannes.berg@intel.com>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-s390@vger.kernel.org, Jia He <justin.he@arm.com>
Subject: [PATCH RFCv2 2/3] lib/vsprintf.c: make %pD print full path for file
Date: Fri, 28 May 2021 19:39:50 +0800	[thread overview]
Message-ID: <20210528113951.6225-3-justin.he@arm.com> (raw)
In-Reply-To: <20210528113951.6225-1-justin.he@arm.com>

We have '%pD' for printing a filename. It may not be perfect (by
default it only prints one component.)

As suggested by Linus at [1]:
A dentry has a parent, but at the same time, a dentry really does
inherently have "one name" (and given just the dentry pointers, you
can't show mount-related parenthood, so in many ways the "show just
one name" makes sense for "%pd" in ways it doesn't necessarily for
"%pD"). But while a dentry arguably has that "one primary component",
a _file_ is certainly not exclusively about that last component.

Hence "file_dentry_name()" simply shouldn't use "dentry_name()" at all.
Despite that shared code origin, and despite that similar letter
choice (lower-vs-upper case), a dentry and a file really are very
different from a name standpoint.

Here stack space is preferred for file_d_path_name() because it is
much safer. The stack size 256 is a compromise between stack overflow
and too short full path.

[1] https://lore.kernel.org/lkml/CAHk-=wimsMqGdzik187YWLb-ru+iktb4MYbMQG1rnZ81dXYFVg@mail.gmail.com/

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Jia He <justin.he@arm.com>
---
 Documentation/core-api/printk-formats.rst |  5 +++--
 lib/vsprintf.c                            | 21 +++++++++++++++++----
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index f063a384c7c8..95ba14dc529b 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -408,12 +408,13 @@ dentry names
 ::
 
 	%pd{,2,3,4}
-	%pD{,2,3,4}
+	%pD
 
 For printing dentry name; if we race with :c:func:`d_move`, the name might
 be a mix of old and new ones, but it won't oops.  %pd dentry is a safer
 equivalent of %s dentry->d_name.name we used to use, %pd<n> prints ``n``
-last components.  %pD does the same thing for struct file.
+last components.  %pD prints full file path together with mount-related
+parenthood.
 
 Passed by reference.
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index f0c35d9b65bf..2e5387b08d67 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -27,6 +27,7 @@
 #include <linux/string.h>
 #include <linux/ctype.h>
 #include <linux/kernel.h>
+#include <linux/dcache.h>
 #include <linux/kallsyms.h>
 #include <linux/math64.h>
 #include <linux/uaccess.h>
@@ -920,13 +921,25 @@ char *dentry_name(char *buf, char *end, const struct dentry *d, struct printf_sp
 }
 
 static noinline_for_stack
-char *file_dentry_name(char *buf, char *end, const struct file *f,
+char *file_d_path_name(char *buf, char *end, const struct file *f,
 			struct printf_spec spec, const char *fmt)
 {
+	const struct path *path;
+	char *p;
+	char full_path[256];
+
 	if (check_pointer(&buf, end, f, spec))
 		return buf;
 
-	return dentry_name(buf, end, f->f_path.dentry, spec, fmt);
+	path = &f->f_path;
+	if (check_pointer(&buf, end, path, spec))
+		return buf;
+
+	p = d_path_fast(path, full_path, sizeof(full_path));
+	if (IS_ERR(p))
+		return err_ptr(buf, end, p, spec);
+
+	return string_nocheck(buf, end, p, spec);
 }
 #ifdef CONFIG_BLOCK
 static noinline_for_stack
@@ -2296,7 +2309,7 @@ early_param("no_hash_pointers", no_hash_pointers_enable);
  * - 'a[pd]' For address types [p] phys_addr_t, [d] dma_addr_t and derivatives
  *           (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
+ * - 'D' For full path name of a struct file
  * - 'g' For block_device name (gendisk + partition number)
  * - 't[RT][dt][r]' For time and date as represented by:
  *      R    struct rtc_time
@@ -2395,7 +2408,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 	case 'C':
 		return clock(buf, end, ptr, spec, fmt);
 	case 'D':
-		return file_dentry_name(buf, end, ptr, spec, fmt);
+		return file_d_path_name(buf, end, ptr, spec, fmt);
 #ifdef CONFIG_BLOCK
 	case 'g':
 		return bdev_name(buf, end, ptr, spec, fmt);
-- 
2.17.1


  parent reply	other threads:[~2021-05-28 11:40 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-28 11:39 [PATCH RFCv2 0/3] make '%pD' print full path for file Jia He
2021-05-28 11:39 ` [PATCH RFCv2 1/3] fs: introduce helper d_path_fast() Jia He
2021-05-28 12:37   ` kernel test robot
2021-05-28 12:44   ` Al Viro
2021-05-28 12:51   ` Matthew Wilcox
2021-05-28 14:23     ` Justin He
2021-05-28 16:02   ` kernel test robot
2021-05-28 11:39 ` Jia He [this message]
2021-05-28 12:59   ` [PATCH RFCv2 2/3] lib/vsprintf.c: make %pD print full path for file Matthew Wilcox
2021-05-28 14:22     ` Justin He
2021-05-28 14:52       ` Matthew Wilcox
2021-05-28 15:09         ` Justin He
2021-05-28 15:22           ` Matthew Wilcox
2021-05-31  0:39             ` Justin He
2021-06-01 14:42             ` Justin He
2021-06-01 15:30               ` Matthew Wilcox
2021-06-01 15:36                 ` Andy Shevchenko
2021-06-01 15:44                   ` Matthew Wilcox
2021-06-01 15:53                     ` Andy Shevchenko
2021-06-01 16:10                       ` Andy Shevchenko
2021-06-01 17:05                         ` Matthew Wilcox
2021-06-01 19:01                           ` Rasmus Villemoes
2021-06-02  5:47                             ` Justin He
2021-05-28 20:06       ` Rasmus Villemoes
2021-05-30 15:18         ` Matthew Wilcox
2021-05-31  9:40           ` Petr Mladek
2021-05-30 20:51   ` kernel test robot
2021-05-28 11:39 ` [PATCH RFCv2 3/3] s390/hmcdrv: remove the redundant directory path in debug message Jia He
2021-05-29  0:26 [PATCH RFCv2 2/3] lib/vsprintf.c: make %pD print full path for file kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210528113951.6225-3-justin.he@arm.com \
    --to=justin.he@arm.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=borntraeger@de.ibm.com \
    --cc=corbet@lwn.net \
    --cc=davem@davemloft.net \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=johannes.berg@intel.com \
    --cc=kuba@kernel.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=luciano.coelho@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.