2021-04-28 14:55:43

by Justin He

[permalink] [raw]
Subject: [PATCH 1/4] iwlwifi: mvm: Explicitly use %pd1 in debugfs entry

'%pd'(no digit following) will mean to print last 4 components of file
dentry. Hence explicitly use %pd1 instead.

Signed-off-by: Jia He <[email protected]>
---
drivers/net/wireless/intel/iwlwifi/mvm/debugfs-vif.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs-vif.c b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs-vif.c
index 38d0bfb649cc..41ccbb4286c2 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs-vif.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs-vif.c
@@ -752,7 +752,7 @@ void iwl_mvm_vif_dbgfs_register(struct iwl_mvm *mvm, struct ieee80211_vif *vif)
* find
* netdev:wlan0 -> ../../../ieee80211/phy0/netdev:wlan0/iwlmvm/
*/
- snprintf(buf, 100, "../../../%pd3/%pd",
+ snprintf(buf, 100, "../../../%pd3/%pd1",
dbgfs_dir,
mvmvif->dbgfs_dir);

--
2.17.1


2021-04-28 14:55:47

by Justin He

[permalink] [raw]
Subject: [PATCH 3/4] s390/hmcdrv: Remove the redundant directory path in debug message

It would be better to use full file path with '%pD' instead of hard coding.

Signed-off-by: Jia He <[email protected]>
---
drivers/s390/char/hmcdrv_dev.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/s390/char/hmcdrv_dev.c b/drivers/s390/char/hmcdrv_dev.c
index 20e9cd542e03..cdde75508c8a 100644
--- a/drivers/s390/char/hmcdrv_dev.c
+++ b/drivers/s390/char/hmcdrv_dev.c
@@ -137,7 +137,7 @@ static int hmcdrv_dev_open(struct inode *inode, struct file *fp)
if (rc)
module_put(THIS_MODULE);

- pr_debug("open file '/dev/%pD' with return code %d\n", fp, rc);
+ pr_debug("open file '%pD' with return code %d\n", fp, rc);
return rc;
}

@@ -146,7 +146,7 @@ static int hmcdrv_dev_open(struct inode *inode, struct file *fp)
*/
static int hmcdrv_dev_release(struct inode *inode, struct file *fp)
{
- pr_debug("closing file '/dev/%pD'\n", fp);
+ pr_debug("closing file '%pD'\n", fp);
kfree(fp->private_data);
fp->private_data = NULL;
hmcdrv_ftp_shutdown();
@@ -231,7 +231,7 @@ static ssize_t hmcdrv_dev_read(struct file *fp, char __user *ubuf,
retlen = hmcdrv_dev_transfer((char *) fp->private_data,
*pos, ubuf, len);

- pr_debug("read from file '/dev/%pD' at %lld returns %zd/%zu\n",
+ pr_debug("read from file '%pD' at %lld returns %zd/%zu\n",
fp, (long long) *pos, retlen, len);

if (retlen > 0)
@@ -248,7 +248,7 @@ static ssize_t hmcdrv_dev_write(struct file *fp, const char __user *ubuf,
{
ssize_t retlen;

- pr_debug("writing file '/dev/%pD' at pos. %lld with length %zd\n",
+ pr_debug("writing file '%pD' at pos. %lld with length %zd\n",
fp, (long long) *pos, len);

if (!fp->private_data) { /* first expect a cmd write */
@@ -272,7 +272,7 @@ static ssize_t hmcdrv_dev_write(struct file *fp, const char __user *ubuf,
if (retlen > 0)
*pos += retlen;

- pr_debug("write to file '/dev/%pD' returned %zd\n", fp, retlen);
+ pr_debug("write to file '%pD' returned %zd\n", fp, retlen);

return retlen;
}
--
2.17.1

2021-04-28 14:55:51

by Justin He

[permalink] [raw]
Subject: [PATCH 4/4] lib/test_printf: Explicitly add components number to %pD and %pd

After changing the default components number from 1 to 4 for %pD
and %pd, it would be better to explicitly add the number in test_printf
cases.

Add a test case of %pd5 to verify if it can be capped by 4 components.

Signed-off-by: Jia He <[email protected]>
---
lib/test_printf.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 27b964ec723d..899cd55d1c90 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -478,18 +478,20 @@ static struct dentry test_dentry[4] __initdata = {
static void __init
dentry(void)
{
- test("foo", "%pd", &test_dentry[0]);
+ test("foo", "%pd1", &test_dentry[0]);
test("foo", "%pd2", &test_dentry[0]);

- test("(null)", "%pd", NULL);
- test("(efault)", "%pd", PTR_INVALID);
- test("(null)", "%pD", NULL);
- test("(efault)", "%pD", PTR_INVALID);
+ test("(null)", "%pd1", NULL);
+ test("(efault)", "%pd1", PTR_INVALID);
+ test("(null)", "%pD1", NULL);
+ test("(efault)", "%pD1", PTR_INVALID);

- test("romeo", "%pd", &test_dentry[3]);
+ test("romeo", "%pd1", &test_dentry[3]);
test("alfa/romeo", "%pd2", &test_dentry[3]);
test("bravo/alfa/romeo", "%pd3", &test_dentry[3]);
test("/bravo/alfa/romeo", "%pd4", &test_dentry[3]);
+ test("/bravo/alfa/romeo", "%pd", &test_dentry[3]);
+ test("/bravo/alfa/romeo", "%pd5", &test_dentry[3]);
test("/bravo/alfa", "%pd4", &test_dentry[2]);

test("bravo/alfa |bravo/alfa ", "%-12pd2|%*pd2", &test_dentry[2], -12, &test_dentry[2]);
--
2.17.1

2021-04-28 14:57:48

by Justin He

[permalink] [raw]
Subject: [PATCH 2/4] lib/vsprintf.c: Make %p{D,d} mean as much components as possible

From: Linus Torvalds <[email protected]>

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

%pD4 should be more than good enough, but we should make plain "%pD" mean
"as much of the path that is reasonable" rather than "as few components as
possible" (ie 1).

Signed-off-by: Linus Torvalds <[email protected]>
Signed-off-by: Jia He <[email protected]>
---
Documentation/core-api/printk-formats.rst | 3 ++-
lib/vsprintf.c | 4 ++--
2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 9be6de402cb9..aa76cbec0dae 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -413,7 +413,8 @@ dentry names
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 does the same thing for struct file. By default, %p{D,d}
+is equal to %p{D,d}4.

Passed by reference.

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 6c56c62fd9a5..5b563953f970 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -880,11 +880,11 @@ char *dentry_name(char *buf, char *end, const struct dentry *d, struct printf_sp
int i, n;

switch (fmt[1]) {
- case '2': case '3': case '4':
+ case '1': case '2': case '3': case '4':
depth = fmt[1] - '0';
break;
default:
- depth = 1;
+ depth = 4;
}

rcu_read_lock();
--
2.17.1

2021-04-28 15:11:38

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 2/4] lib/vsprintf.c: Make %p{D,d} mean as much components as possible

On Wed, Apr 28, 2021 at 5:56 PM Jia He <[email protected]> wrote:
>
> From: Linus Torvalds <[email protected]>

Hmm... Okay.

> We have '%pD'(no digit following) for printing a filename. It may not be
> perfect (by default it only prints one component.
>
> %pD4 should be more than good enough, but we should make plain "%pD" mean
> "as much of the path that is reasonable" rather than "as few components as
> possible" (ie 1).

Sorry, but from above I didn't get why.

The commit message tells only about %pD, but patch changes behaviour
of the ~100 or so users of "%pd" without any explanation.

Besides that the patch is prepended only by one change (which is also
not related to %pD), while we have ~30 users which behaviour got
changed.


--
With Best Regards,
Andy Shevchenko

2021-04-29 08:55:30

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 2/4] lib/vsprintf.c: Make %p{D,d} mean as much components as possible

On Thu, Apr 29, 2021 at 11:47 AM Petr Mladek <[email protected]> wrote:
>
> On Wed 2021-04-28 21:59:27, Jia He wrote:
> > From: Linus Torvalds <[email protected]>
> >
> > We have '%pD'(no digit following) for printing a filename. It may not be
> > perfect (by default it only prints one component.
> >
> > %pD4 should be more than good enough, but we should make plain "%pD" mean
> > "as much of the path that is reasonable" rather than "as few components as
> > possible" (ie 1).
>
> Could you please provide link to the discussion where this idea was
> came from?

https://lore.kernel.org/lkml/20210427025805.GD3122264@magnolia/

--
With Best Regards,
Andy Shevchenko

2021-04-29 09:25:34

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH 2/4] lib/vsprintf.c: Make %p{D,d} mean as much components as possible

On Thu 2021-04-29 11:52:49, Andy Shevchenko wrote:
> On Thu, Apr 29, 2021 at 11:47 AM Petr Mladek <[email protected]> wrote:
> >
> > On Wed 2021-04-28 21:59:27, Jia He wrote:
> > > From: Linus Torvalds <[email protected]>
> > >
> > > We have '%pD'(no digit following) for printing a filename. It may not be
> > > perfect (by default it only prints one component.
> > >
> > > %pD4 should be more than good enough, but we should make plain "%pD" mean
> > > "as much of the path that is reasonable" rather than "as few components as
> > > possible" (ie 1).
> >
> > Could you please provide link to the discussion where this idea was
> > came from?
>
> https://lore.kernel.org/lkml/20210427025805.GD3122264@magnolia/

Thanks for the link. I see that it was not clear whether the patch
was good for %pd behavior.

Linus actually suggests to keep %pd behavior as it was before, see
https://lore.kernel.org/lkml/CAHk-=wimsMqGdzik187YWLb-ru+iktb4MYbMQG1rnZ81dXYFVg@mail.gmail.com/

Well, I think that this is up to the file system developers to decide.
I am not sure if the path would do more harm than good,
or vice versa, for dentry names.

Best Regards,
Petr

2021-04-30 01:36:51

by Justin He

[permalink] [raw]
Subject: RE: [PATCH 2/4] lib/vsprintf.c: Make %p{D,d} mean as much components as possible

Hi

> -----Original Message-----
> From: Petr Mladek <[email protected]>
> Sent: Thursday, April 29, 2021 5:25 PM
> To: Andy Shevchenko <[email protected]>
> Cc: Justin He <[email protected]>; Linus Torvalds <torvalds@linux-
> foundation.org>; Steven Rostedt <[email protected]>; Sergey Senozhatsky
> <[email protected]>; Andy Shevchenko
> <[email protected]>; Rasmus Villemoes
> <[email protected]>; Jonathan Corbet <[email protected]>; Luca Coelho
> <[email protected]>; Kalle Valo <[email protected]>; David S.
> Miller <[email protected]>; Jakub Kicinski <[email protected]>; Heiko
> Carstens <[email protected]>; Vasily Gorbik <[email protected]>; Christian
> Borntraeger <[email protected]>; Johannes Berg
> <[email protected]>; Linux Documentation List <linux-
> [email protected]>; Linux Kernel Mailing List <linux-
> [email protected]>; open list:TI WILINK WIRELES... <linux-
> [email protected]>; netdev <[email protected]>; linux-
> [email protected]
> Subject: Re: [PATCH 2/4] lib/vsprintf.c: Make %p{D,d} mean as much
> components as possible
>
> On Thu 2021-04-29 11:52:49, Andy Shevchenko wrote:
> > On Thu, Apr 29, 2021 at 11:47 AM Petr Mladek <[email protected]> wrote:
> > >
> > > On Wed 2021-04-28 21:59:27, Jia He wrote:
> > > > From: Linus Torvalds <[email protected]>
> > > >
> > > > We have '%pD'(no digit following) for printing a filename. It may not
> be
> > > > perfect (by default it only prints one component.
> > > >
> > > > %pD4 should be more than good enough, but we should make plain "%pD"
> mean
> > > > "as much of the path that is reasonable" rather than "as few
> components as
> > > > possible" (ie 1).
> > >
> > > Could you please provide link to the discussion where this idea was
> > > came from?
> >
> > https://lore.kernel.org/lkml/20210427025805.GD3122264@magnolia/
>
> Thanks for the link. I see that it was not clear whether the patch
> was good for %pd behavior.
>
> Linus actually suggests to keep %pd behavior as it was before, see
> https://lore.kernel.org/lkml/CAHk-=wimsMqGdzik187YWLb-
> [email protected]/

Okay, let me keep the default %pd behavior as before('%pd' is '%pd1') and
change the behavior of %pD ('%pD' is '%pD4')

--
Cheers,
Justin (Jia He)
>
> Well, I think that this is up to the file system developers to decide.
> I am not sure if the path would do more harm than good,
> or vice versa, for dentry names.
>
> Best Regards,
> Petr
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.