Re: [PATCH RFC 1/4] nfsd: fix a warning in __cld_pipe_upcall()

J. Bruce Fields
2018-11-27T21:19:50Z

This looks fine, applying now for 4.21.--b.

On Tue, Nov 06, 2018 at 01:35:08PM -0500, Scott Mayhew wrote:
> __cld_pipe_upcall() emits a "do not call blocking ops when
> !TASK_RUNNING" warning due to the dput() call in rpc_queue_upcall().
> Fix it by using a completion instead of hand coding the wait.
>
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> ---
> fs/nfsd/nfs4recover.c | 17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> index 9c247fa1e959..5188f9f70c78 100644
> --- a/fs/nfsd/nfs4recover.c
> +++ b/fs/nfsd/nfs4recover.c
> @@ -662,7 +662,7 @@ struct cld_net {
> struct cld_upcall {
> struct list_head cu_list;
> struct cld_net *cu_net;
> - struct task_struct *cu_task;
> + struct completion cu_done;
> struct cld_msg cu_msg;
> };
>
> @@ -671,23 +671,18 @@ __cld_pipe_upcall(struct rpc_pipe *pipe, struct cld_msg *cmsg)
> {
> int ret;
> struct rpc_pipe_msg msg;
> + struct cld_upcall *cup = container_of(cmsg, struct cld_upcall, cu_msg);
>
> memset(&msg, 0, sizeof(msg));
> msg.data = cmsg;
> msg.len = sizeof(*cmsg);
>
> - /*
> - * Set task state before we queue the upcall. That prevents
> - * wake_up_process in the downcall from racing with schedule.
> - */
> - set_current_state(TASK_UNINTERRUPTIBLE);
> ret = rpc_queue_upcall(pipe, &msg);
> if (ret < 0) {
> - set_current_state(TASK_RUNNING);
> goto out;
> }
>
> - schedule();
> + wait_for_completion(&cup->cu_done);
>
> if (msg.errno < 0)
> ret = msg.errno;
> @@ -754,7 +749,7 @@ cld_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
> if (copy_from_user(&cup->cu_msg, src, mlen) != 0)
> return -EFAULT;
>
> - wake_up_process(cup->cu_task);
> + complete(&cup->cu_done);
> return mlen;
> }
>
> @@ -769,7 +764,7 @@ cld_pipe_destroy_msg(struct rpc_pipe_msg *msg)
> if (msg->errno >= 0)
> return;
>
> - wake_up_process(cup->cu_task);
> + complete(&cup->cu_done);
> }
>
> static const struct rpc_pipe_ops cld_upcall_ops = {
> @@ -900,7 +895,7 @@ alloc_cld_upcall(struct cld_net *cn)
> goto restart_search;
> }
> }
> - new->cu_task = current;
> + init_completion(&new->cu_done);
> new->cu_msg.cm_vers = CLD_UPCALL_VERSION;
> put_unaligned(cn->cn_xid++, &new->cu_msg.cm_xid);
> new->cu_net = cn;
> --
> 2.17.1