Hi all,
This patch series fixes W=1 compiler warnings in sunrpc, related to
variables that are used only when debugging is enabled.
Changes compared to v2:
- New patch "sunrpc: Wrap read accesses to rpc_task.tk_pid",
- Add Acked-by.
Changes compared to v1:
- s/uncontionally/unconditionally/,
- Drop CONFIG_SUNRPC_DEBUG check in fs/lockd/svclock.c to fix build
failure.
Thanks for your comments!
Geert Uytterhoeven (2):
sunrpc: Wrap read accesses to rpc_task.tk_pid
sunrpc: Use no_printk() in dfprintk*() dummies
fs/lockd/svclock.c | 2 --
fs/nfs/filelayout/filelayout.c | 12 ++++++------
fs/nfs/flexfilelayout/flexfilelayout.c | 9 +++------
include/linux/sunrpc/debug.h | 6 +++---
include/linux/sunrpc/sched.h | 10 ++++++++++
net/sunrpc/clnt.c | 2 +-
net/sunrpc/debugfs.c | 2 +-
7 files changed, 24 insertions(+), 19 deletions(-)
--
2.34.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
The tk_pid member in the rpc_task structure exists conditionally on
debug or tracing being enabled.
Introduce and use a wapper to read the value of this member, so users
outside tracing no longer have to care about these conditions.
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v3:
- New.
---
fs/nfs/filelayout/filelayout.c | 12 ++++++------
fs/nfs/flexfilelayout/flexfilelayout.c | 9 +++------
include/linux/sunrpc/sched.h | 10 ++++++++++
net/sunrpc/clnt.c | 2 +-
net/sunrpc/debugfs.c | 2 +-
5 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/fs/nfs/filelayout/filelayout.c b/fs/nfs/filelayout/filelayout.c
index ce8f8934bca517c0..5af545f49c54db4f 100644
--- a/fs/nfs/filelayout/filelayout.c
+++ b/fs/nfs/filelayout/filelayout.c
@@ -93,8 +93,7 @@ static void filelayout_reset_write(struct nfs_pgio_header *hdr)
if (!test_and_set_bit(NFS_IOHDR_REDO, &hdr->flags)) {
dprintk("%s Reset task %5u for i/o through MDS "
"(req %s/%llu, %u bytes @ offset %llu)\n", __func__,
- hdr->task.tk_pid,
- hdr->inode->i_sb->s_id,
+ rpc_tk_pid(task), hdr->inode->i_sb->s_id,
(unsigned long long)NFS_FILEID(hdr->inode),
hdr->args.count,
(unsigned long long)hdr->args.offset);
@@ -110,8 +109,7 @@ static void filelayout_reset_read(struct nfs_pgio_header *hdr)
if (!test_and_set_bit(NFS_IOHDR_REDO, &hdr->flags)) {
dprintk("%s Reset task %5u for i/o through MDS "
"(req %s/%llu, %u bytes @ offset %llu)\n", __func__,
- hdr->task.tk_pid,
- hdr->inode->i_sb->s_id,
+ rpc_tk_pid(task), hdr->inode->i_sb->s_id,
(unsigned long long)NFS_FILEID(hdr->inode),
hdr->args.count,
(unsigned long long)hdr->args.offset);
@@ -274,7 +272,8 @@ static void filelayout_read_prepare(struct rpc_task *task, void *data)
return;
}
if (filelayout_reset_to_mds(hdr->lseg)) {
- dprintk("%s task %u reset io to MDS\n", __func__, task->tk_pid);
+ dprintk("%s task %u reset io to MDS\n", __func__,
+ rpc_tk_pid(task));
filelayout_reset_read(hdr);
rpc_exit(task, 0);
return;
@@ -372,7 +371,8 @@ static void filelayout_write_prepare(struct rpc_task *task, void *data)
return;
}
if (filelayout_reset_to_mds(hdr->lseg)) {
- dprintk("%s task %u reset io to MDS\n", __func__, task->tk_pid);
+ dprintk("%s task %u reset io to MDS\n", __func__,
+ rpc_tk_pid(task));
filelayout_reset_write(hdr);
rpc_exit(task, 0);
return;
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index a1dc338649062de3..3dd17f675d433f4d 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -1017,8 +1017,7 @@ static void ff_layout_reset_write(struct nfs_pgio_header *hdr, bool retry_pnfs)
if (retry_pnfs) {
dprintk("%s Reset task %5u for i/o through pNFS "
"(req %s/%llu, %u bytes @ offset %llu)\n", __func__,
- hdr->task.tk_pid,
- hdr->inode->i_sb->s_id,
+ rpc_tk_pid(task), hdr->inode->i_sb->s_id,
(unsigned long long)NFS_FILEID(hdr->inode),
hdr->args.count,
(unsigned long long)hdr->args.offset);
@@ -1030,8 +1029,7 @@ static void ff_layout_reset_write(struct nfs_pgio_header *hdr, bool retry_pnfs)
if (!test_and_set_bit(NFS_IOHDR_REDO, &hdr->flags)) {
dprintk("%s Reset task %5u for i/o through MDS "
"(req %s/%llu, %u bytes @ offset %llu)\n", __func__,
- hdr->task.tk_pid,
- hdr->inode->i_sb->s_id,
+ rpc_tk_pid(task), hdr->inode->i_sb->s_id,
(unsigned long long)NFS_FILEID(hdr->inode),
hdr->args.count,
(unsigned long long)hdr->args.offset);
@@ -1066,8 +1064,7 @@ static void ff_layout_reset_read(struct nfs_pgio_header *hdr)
if (!test_and_set_bit(NFS_IOHDR_REDO, &hdr->flags)) {
dprintk("%s Reset task %5u for i/o through MDS "
"(req %s/%llu, %u bytes @ offset %llu)\n", __func__,
- hdr->task.tk_pid,
- hdr->inode->i_sb->s_id,
+ rpc_tk_pid(task), hdr->inode->i_sb->s_id,
(unsigned long long)NFS_FILEID(hdr->inode),
hdr->args.count,
(unsigned long long)hdr->args.offset);
diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h
index 8ada7dc802d30507..0f39e60d28ed0132 100644
--- a/include/linux/sunrpc/sched.h
+++ b/include/linux/sunrpc/sched.h
@@ -270,6 +270,11 @@ void rpc_prepare_task(struct rpc_task *task);
gfp_t rpc_task_gfp_mask(void);
#if IS_ENABLED(CONFIG_SUNRPC_DEBUG) || IS_ENABLED(CONFIG_TRACEPOINTS)
+static inline unsigned short rpc_tk_pid(const struct rpc_task *task)
+{
+ return task->tk_pid;
+}
+
static inline const char * rpc_qname(const struct rpc_wait_queue *q)
{
return ((q && q->name) ? q->name : "unknown");
@@ -281,6 +286,11 @@ static inline void rpc_assign_waitqueue_name(struct rpc_wait_queue *q,
q->name = name;
}
#else
+static inline unsigned short rpc_tk_pid(const struct rpc_task *task)
+{
+ return 0;
+}
+
static inline void rpc_assign_waitqueue_name(struct rpc_wait_queue *q,
const char *name)
{
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 9c210273d06b7f51..2f37e4143789b0cc 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -3320,7 +3320,7 @@ static void rpc_show_task(const struct rpc_clnt *clnt,
rpc_waitq = rpc_qname(task->tk_waitqueue);
printk(KERN_INFO "%5u %04x %6d %8p %8p %8ld %8p %sv%u %s a:%ps q:%s\n",
- task->tk_pid, task->tk_flags, task->tk_status,
+ rpc_tk_pid(task), task->tk_flags, task->tk_status,
clnt, task->tk_rqstp, rpc_task_timeout(task), task->tk_ops,
clnt->cl_program->name, clnt->cl_vers, rpc_proc_name(task),
task->tk_action, rpc_waitq);
diff --git a/net/sunrpc/debugfs.c b/net/sunrpc/debugfs.c
index a176d5a0b0ee9a2c..8896518dd6f3ce0e 100644
--- a/net/sunrpc/debugfs.c
+++ b/net/sunrpc/debugfs.c
@@ -31,7 +31,7 @@ tasks_show(struct seq_file *f, void *v)
xid = be32_to_cpu(task->tk_rqstp->rq_xid);
seq_printf(f, "%5u %04x %6d 0x%x 0x%x %8ld %ps %sv%u %s a:%ps q:%s\n",
- task->tk_pid, task->tk_flags, task->tk_status,
+ rpc_tk_pid(task), task->tk_flags, task->tk_status,
clnt->cl_clid, xid, rpc_task_timeout(task), task->tk_ops,
clnt->cl_program->name, clnt->cl_vers, rpc_proc_name(task),
task->tk_action, rpc_waitq);
--
2.34.1
On 16 Oct 2023, at 9:09, Geert Uytterhoeven wrote:
> The tk_pid member in the rpc_task structure exists conditionally on
> debug or tracing being enabled.
>
> Introduce and use a wapper to read the value of this member, so users
> outside tracing no longer have to care about these conditions.
>
> Reported-by: kernel test robot <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> Signed-off-by: Geert Uytterhoeven <[email protected]>
I never work on kernels that don't have tk_pid, but I can say its so useful
that for 2 out of the 224 bytes that rpc_task uses (on aarch64), I'd be
inclined to just include it all the time. That way its around for folks to
reference with realtime tools (like bpftrace, stap).
Does anyone know if there is a good reason not to include it for all
configurations?
Ben
..also:
Reviewed-by: Benjamin Coddington <[email protected]>
Hi Ben.
On Mon, Oct 16, 2023 at 11:15 PM Benjamin Coddington
<[email protected]> wrote:
> On 16 Oct 2023, at 9:09, Geert Uytterhoeven wrote:
> > The tk_pid member in the rpc_task structure exists conditionally on
> > debug or tracing being enabled.
> >
> > Introduce and use a wapper to read the value of this member, so users
> > outside tracing no longer have to care about these conditions.
> >
> > Reported-by: kernel test robot <[email protected]>
> > Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
>
> I never work on kernels that don't have tk_pid, but I can say its so useful
> that for 2 out of the 224 bytes that rpc_task uses (on aarch64), I'd be
> inclined to just include it all the time. That way its around for folks to
> reference with realtime tools (like bpftrace, stap).
That would work, too.
In fact always including it should not increase the size of struct rpc_task,
as there's still unused spaced in the gap at the end.
> Does anyone know if there is a good reason not to include it for all
> configurations?
>
> Ben
>
> ..also:
> Reviewed-by: Benjamin Coddington <[email protected]>
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds