Re: [nfsd:nfsd-next 5/23] net/sunrpc/svc_xprt.c:794:20: error: 'current_task' undeclared

["J. Bruce Fields" <bfields@fieldses.org>]

On Wed, Aug 20, 2014 at 06:33:06PM -0400, Trond Myklebust wrote:
> On Mon, Aug 18, 2014 at 12:18 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> > On Mon, Aug 18, 2014 at 12:41:24AM +0800, kbuild test robot wrote:
> >> tree:   git://linux-nfs.org/~bfields/linux.git nfsd-next
> >> head:   da8b8f7602ed3b8105c6247a2844455fec72caaa
> >> commit: 983c684466e02b21f83c025ea539deee6c0aeac0 [5/23] SUNRPC: get rid of the request wait queue
> >> config: make ARCH=xtensa allyesconfig
> >>
> >> All error/warnings:
> >>
> >>    net/sunrpc/svc_xprt.c: In function 'svc_recv':
> >> >> net/sunrpc/svc_xprt.c:794:20: error: 'current_task' undeclared (first use in this function)
> >>       rqstp->rq_task = current_task;
> >>                        ^
> >>    net/sunrpc/svc_xprt.c:794:20: note: each undeclared identifier is reported only once for each function it appears in
> >>
> >> vim +/current_task +794 net/sunrpc/svc_xprt.c
> >>
> >>    788                        printk(KERN_ERR
> >>    789                                "svc_recv: service %p, transport not NULL!\n",
> >>    790                                 rqstp);
> >>    791
> >>    792                /* Make sure the task pointer is set! */
> >>    793                if (WARN_ON_ONCE(!rqstp->rq_task))
> >>  > 794                        rqstp->rq_task = current_task;
> >
> > I appreciate the effort to fail gracefully here, but maybe this would be
> > a case for simple BUG_ON()?
> >
> > I don't think there are normally any important locks held here, and I
> > can't see any other reason why killing a server thread would bring a box
> > down.
> >
> > And the first developer to forget to set rq_task on a newly created
> > server will get an immediate bug, so this should be obvious enough.
> > Unless there's fear of a race between starting the thread and setting
> > rq_task?
> 
> No, there is no fear of a race. I'm fine with removing the above check
> altogether and just letting code Oops if it turns out that we add new
> cases that don't set rq_task.

OK, I'll remove it entirely.

> Ultimately, though, I think we need a better interface for starting
> RPC server threads; something that combines svc_prepare_thread() and
> kthread_run() into a single function call and that can thus set
> rqstp->rq_task correctly.

Sounds good.

--b.