2021-07-15 03:43:33

by Justin He

[permalink] [raw]
Subject: [PATCH RFC 00/13] Simplify the print format string with new '%pD'

This is the followup work after changing the behavior of '%pD' to
print the full path of file.

Background
==========
Linus suggested printing the full path of file instead of printing
the components as '%pd'.

This series is based on my patch series of making '%pD' print the full
path of file [1], which is stable now.

[1]: https://lkml.org/lkml/2021/7/14/1519

Test
====
I only tested it with basic compilation and booting. All the changed
codes are compiled and built successfully on Arm64. Therefore set it as
RFC.

Patch details
=============
Patches 01 to 06 with minor changes are easy for review. They are to
remove the hard coding and the postfix number of previous '%pD'. This
should be removed after the '%pD' behavior is changed.

Patches 07 to 13 are changed to simplify the printing helpers.

Jia He (13):
s390/hmcdrv: remove the redundant directory path in format string
afs: Remove the number postfix of '%pD' in format string
fs: Remove the number postfix of '%pD' in format string
NFS: Remove the number postfix of '%pD' in format string
NFSD: Remove the number postfix of '%pD' in format string
ovl: remove the number postfix of '%pD' in format string
iomap: simplify iomap_swapfile_fail() with '%pD' specifier
fs/coredump: simplify the printing with '%pD' and '%pd' specifier
mm/filemap: simplify the printing with '%pD' specifier
usb: gadget: simplify the printing with '%pD' specifier
md/bitmap: simplify the printing with '%pD' specifier
mm: simplify the printing with '%pd' specifier
ext4: simplify the printing with '%pD' specifier

drivers/md/md-bitmap.c | 13 ++-------
drivers/s390/char/hmcdrv_dev.c | 10 +++----
drivers/usb/gadget/function/f_mass_storage.c | 28 ++++++++------------
fs/afs/mntpt.c | 2 +-
fs/coredump.c | 26 +++---------------
fs/exec.c | 2 +-
fs/ext4/super.c | 12 +++------
fs/ioctl.c | 2 +-
fs/iomap/direct-io.c | 2 +-
fs/iomap/swapfile.c | 8 +-----
fs/nfs/dir.c | 12 ++++-----
fs/nfs/direct.c | 4 +--
fs/nfs/file.c | 26 +++++++++---------
fs/nfs/nfs4file.c | 2 +-
fs/nfs/write.c | 2 +-
fs/nfsd/nfs4state.c | 2 +-
fs/overlayfs/file.c | 2 +-
fs/read_write.c | 2 +-
fs/splice.c | 2 +-
mm/filemap.c | 7 +----
mm/memory.c | 16 +++++------
21 files changed, 65 insertions(+), 117 deletions(-)

--
2.17.1


2021-07-15 04:27:43

by Justin He

[permalink] [raw]
Subject: [PATCH RFC 01/13] s390/hmcdrv: remove the redundant directory path in format string

After the behavior of '%pD' is changed to print the full path of file,
it would be better to use '%pD' instead of hard coding the parent
mountpoint.

Cc: Heiko Carstens <[email protected]>
Cc: Vasily Gorbik <[email protected]>
Cc: Christian Borntraeger <[email protected]>
Cc: [email protected]
Cc: [email protected]
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-07-15 04:27:45

by Justin He

[permalink] [raw]
Subject: [PATCH RFC 03/13] fs: Remove the number postfix of '%pD' in format string

After the behavior of '%pD' is changed to print the full path of file,
the previous number postfix of '%pD' is pointless.

Cc: Alexander Viro <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Jia He <[email protected]>
---
fs/exec.c | 2 +-
fs/ioctl.c | 2 +-
fs/read_write.c | 2 +-
fs/splice.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 38f63451b928..a9f9de7da8ff 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -811,7 +811,7 @@ int setup_arg_pages(struct linux_binprm *bprm,
BUG_ON(prev != vma);

if (unlikely(vm_flags & VM_EXEC)) {
- pr_warn_once("process '%pD4' started with executable stack\n",
+ pr_warn_once("process '%pD' started with executable stack\n",
bprm->file);
}

diff --git a/fs/ioctl.c b/fs/ioctl.c
index 1e2204fa9963..80c9d3d00c8f 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -78,7 +78,7 @@ static int ioctl_fibmap(struct file *filp, int __user *p)

if (block > INT_MAX) {
error = -ERANGE;
- pr_warn_ratelimited("[%s/%d] FS: %s File: %pD4 would truncate fibmap result\n",
+ pr_warn_ratelimited("[%s/%d] FS: %s File: %pD would truncate fibmap result\n",
current->comm, task_pid_nr(current),
sb->s_id, filp);
}
diff --git a/fs/read_write.c b/fs/read_write.c
index 9db7adf160d2..3fdb17e4b712 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -422,7 +422,7 @@ static ssize_t new_sync_read(struct file *filp, char __user *buf, size_t len, lo
static int warn_unsupported(struct file *file, const char *op)
{
pr_warn_ratelimited(
- "kernel %s not supported for file %pD4 (pid: %d comm: %.20s)\n",
+ "kernel %s not supported for file %pD (pid: %d comm: %.20s)\n",
op, file, current->pid, current->comm);
return -EINVAL;
}
diff --git a/fs/splice.c b/fs/splice.c
index 5dbce4dcc1a7..4b0b9029b5ca 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -751,7 +751,7 @@ EXPORT_SYMBOL(generic_splice_sendpage);
static int warn_unsupported(struct file *file, const char *op)
{
pr_debug_ratelimited(
- "splice %s not supported for file %pD4 (pid: %d comm: %.20s)\n",
+ "splice %s not supported for file %pD (pid: %d comm: %.20s)\n",
op, file, current->pid, current->comm);
return -EINVAL;
}
--
2.17.1

2021-07-15 04:28:17

by Justin He

[permalink] [raw]
Subject: [PATCH RFC 10/13] usb: gadget: simplify the printing with '%pD' specifier

After the behavior of '%pD' is changed to print the full path of file,
the log printing in fsg_common_create_lun() can be simplified.

Given the space with proper length would be allocated in vprintk_store(),
it is worthy of dropping kmalloc()/kfree() to avoid additional space
allocation. The error case is well handled in d_path_unsafe(), the error
string would be copied in '%pD' buffer, no need to additionally handle
IS_ERR().

Cc: Felipe Balbi <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: "Gustavo A. R. Silva" <[email protected]>
Cc: Chen Lin <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Jia He <[email protected]>
---
drivers/usb/gadget/function/f_mass_storage.c | 28 ++++++++------------
1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index 4a4703634a2a..04d4e8a0f6fd 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -2738,7 +2738,6 @@ int fsg_common_create_lun(struct fsg_common *common, struct fsg_lun_config *cfg,
const char **name_pfx)
{
struct fsg_lun *lun;
- char *pathbuf, *p;
int rc = -ENOMEM;

if (id >= ARRAY_SIZE(common->luns))
@@ -2790,22 +2789,17 @@ int fsg_common_create_lun(struct fsg_common *common, struct fsg_lun_config *cfg,
goto error_lun;
}

- pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
- p = "(no medium)";
- if (fsg_lun_is_open(lun)) {
- p = "(error)";
- if (pathbuf) {
- p = file_path(lun->filp, pathbuf, PATH_MAX);
- if (IS_ERR(p))
- p = "(error)";
- }
- }
- pr_info("LUN: %s%s%sfile: %s\n",
- lun->removable ? "removable " : "",
- lun->ro ? "read only " : "",
- lun->cdrom ? "CD-ROM " : "",
- p);
- kfree(pathbuf);
+ if (fsg_lun_is_open(lun))
+ pr_info("LUN: %s%s%sfile: %pD\n",
+ lun->removable ? "removable " : "",
+ lun->ro ? "read only " : "",
+ lun->cdrom ? "CD-ROM " : "",
+ lun->filp);
+ else
+ pr_info("LUN: %s%s%sfile: (no medium)\n",
+ lun->removable ? "removable " : "",
+ lun->ro ? "read only " : "",
+ lun->cdrom ? "CD-ROM " : "");

return 0;

--
2.17.1

2021-07-15 09:00:14

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH RFC 10/13] usb: gadget: simplify the printing with '%pD' specifier

On Thu, Jul 15, 2021 at 11:15:30AM +0800, Jia He wrote:
> After the behavior of '%pD' is changed to print the full path of file,
> the log printing in fsg_common_create_lun() can be simplified.
>
> Given the space with proper length would be allocated in vprintk_store(),
> it is worthy of dropping kmalloc()/kfree() to avoid additional space
> allocation. The error case is well handled in d_path_unsafe(), the error
> string would be copied in '%pD' buffer, no need to additionally handle
> IS_ERR().
>
> Cc: Felipe Balbi <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: "Gustavo A. R. Silva" <[email protected]>
> Cc: Chen Lin <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Jia He <[email protected]>
> ---
> drivers/usb/gadget/function/f_mass_storage.c | 28 ++++++++------------
> 1 file changed, 11 insertions(+), 17 deletions(-)

Reviewed-by: Greg Kroah-Hartman <[email protected]>

2021-07-15 10:32:23

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH RFC 10/13] usb: gadget: simplify the printing with '%pD' specifier

Jia He <[email protected]> writes:

> After the behavior of '%pD' is changed to print the full path of file,
> the log printing in fsg_common_create_lun() can be simplified.
>
> Given the space with proper length would be allocated in vprintk_store(),
> it is worthy of dropping kmalloc()/kfree() to avoid additional space
> allocation. The error case is well handled in d_path_unsafe(), the error
> string would be copied in '%pD' buffer, no need to additionally handle
> IS_ERR().
>
> Cc: Felipe Balbi <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: "Gustavo A. R. Silva" <[email protected]>
> Cc: Chen Lin <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Jia He <[email protected]>

Acked-by: Felipe Balbi <[email protected]>

--
balbi


Attachments:
signature.asc (521.00 B)