* Re: [nfsd:nfsd-next 5/23] net/sunrpc/svc_xprt.c:794:20: error: 'current_task' undeclared [not found] <53f0db34.mtq2J6NaXWKpR15G%fengguang.wu@intel.com> @ 2014-08-18 16:18 ` J. Bruce Fields 2014-08-20 22:33 ` Trond Myklebust 0 siblings, 1 reply; 3+ messages in thread From: J. Bruce Fields @ 2014-08-18 16:18 UTC (permalink / raw) To: kbuild test robot; +Cc: Trond Myklebust, J. Bruce Fields, kbuild-all, linux-nfs 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? --b. > 795 > 796 err = svc_alloc_arg(rqstp); > 797 if (err) > > --- > 0-DAY kernel build testing backend Open Source Technology Center > http://lists.01.org/mailman/listinfo/kbuild Intel Corporation ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [nfsd:nfsd-next 5/23] net/sunrpc/svc_xprt.c:794:20: error: 'current_task' undeclared 2014-08-18 16:18 ` [nfsd:nfsd-next 5/23] net/sunrpc/svc_xprt.c:794:20: error: 'current_task' undeclared J. Bruce Fields @ 2014-08-20 22:33 ` Trond Myklebust 2014-08-28 19:52 ` J. Bruce Fields 0 siblings, 1 reply; 3+ messages in thread From: Trond Myklebust @ 2014-08-20 22:33 UTC (permalink / raw) To: J. Bruce Fields Cc: kbuild test robot, J. Bruce Fields, kbuild-all, Linux NFS Mailing List 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. 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. -- Trond Myklebust Linux NFS client maintainer, PrimaryData trond.myklebust@primarydata.com ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [nfsd:nfsd-next 5/23] net/sunrpc/svc_xprt.c:794:20: error: 'current_task' undeclared 2014-08-20 22:33 ` Trond Myklebust @ 2014-08-28 19:52 ` J. Bruce Fields 0 siblings, 0 replies; 3+ messages in thread From: J. Bruce Fields @ 2014-08-28 19:52 UTC (permalink / raw) To: Trond Myklebust Cc: kbuild test robot, J. Bruce Fields, kbuild-all, Linux NFS Mailing List 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. ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-08-28 19:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <53f0db34.mtq2J6NaXWKpR15G%fengguang.wu@intel.com>
2014-08-18 16:18 ` [nfsd:nfsd-next 5/23] net/sunrpc/svc_xprt.c:794:20: error: 'current_task' undeclared J. Bruce Fields
2014-08-20 22:33 ` Trond Myklebust
2014-08-28 19:52 ` J. Bruce Fields
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).