Re: [PATCH] NLM: hold BKL when clearing global lockd task and serv vars

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 8 Apr 2008 12:28:21 -0400
"J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote:

> On Tue, Apr 08, 2008 at 09:21:02AM -0400, Jeff Layton wrote:
> > On Mon, 7 Apr 2008 16:50:27 -0400
> > "J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote:
> > 
> > > On Mon, Apr 07, 2008 at 04:22:41PM -0400, Jeff Layton wrote:
> > > > On Mon, 7 Apr 2008 13:56:15 -0400
> > > > "J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote:
> > > > 
> > > > > On Mon, Apr 07, 2008 at 12:45:01PM -0400, Christoph Hellwig wrote:
> > > > > > On Mon, Apr 07, 2008 at 09:38:34AM -0400, Jeff Layton wrote:
> > > > > > > The global task and serv pointers for lockd are normally protected by
> > > > > > > the nlmsvc_mutex. The exception is when the lockd exits abnormally. When
> > > > > > > this occurs, these variables are cleared without any locking.
> > > > > > 
> > > > > > Shouldn't we get rid of the case where it exits abnormally instead?
> > > > > 
> > > > > I tried to figure out when this could actually occur (when can
> > > > > svc_recv() return an error other than -EINTR or -EAGAIN?), and got lost
> > > > > in sock_recvmsg():
> > > > > 
> > > > > 	- svc_recv() itself returns only -EAGAIN or the return from
> > > > > 	  ->xpo_recvfrom().
> > > > > 	- the only xpo_recvfrom() that's interesting is
> > > > > 	  svc_tcp_recvfrom(), which can return the error it gets from
> > > > > 	  svc_recvfrom(), which can return the error from
> > > > > 	  kernel_recvmsg(), which gets its return from sock_recvmsg().
> > > > > 
> > > > > Since __sock_recvmsg() has a security hook, it looks like we can end up
> > > > > with an -EACCES from selinux?
> > > > > 
> > > > > So one case would be selinux deciding we weren't allowed to receive
> > > > > packets from this socket.  Huh.
> > > > 
> > > > I got lost there too, but I would suspect that there are other errors
> > > > that can bubble up from the lower networking layers as well. Even if
> > > > there aren't currently, it's probably still prudent to assume that it's
> > > > a possibility and code for it.
> > > > 
> > > > I tend to think the safest thing is probably to do a long sleep (1s or
> > > > so and retry when we get an error (maybe also a ratelimited printk?).
> > > 
> > > Yeah, I guess I can't think of anything better.
> > > 
> > 
> > Ok, I went ahead and did patches for this and gave them a quick test
> > this morning. Obviously, these are hard to fully unit test since this
> > seems to be a very uncommon occurrence.
> 
> I suppose this could probably be reproduced with some selinux magic.
> 

Good idea. I'll chat with our SELinux guys and see if they can point me
in the right direction on this.

> > Any thoughts?
> 
> If anyone does ever hit this and it doesn't go away, the printk (even
> with the ratelimiting) could be pretty annoying, so it might be worth
> arranging to print this just once.  But perhaps we can wait and see if
> that actually happens.
> 

I tend to think that this is most likely to happen when there's a
significant problem in lower level networking (or maybe a VM problem?).
SELinux is also a possibility, as you pointed out, but I'm not sure how
much kernel threads are affected by SELinux...

In any case, if this happens, then we're probably going to have bigger
problems than a printk every second. My suggestion would be that we keep
it like this for now, and reconsider it if it turns out to be a
problem...

> Given what appears to be a very unusual crash, and what I'm assuming is
> an impending release, I suppose we should wait for the merge window (but
> possibly also submit to 2.6.25.x).
> 

Yes. I was thinking that all of this should probably be 2.6.26
material (along with all of the kthread conversion patches). Aside from
this possibly happening in Michael's crash, I've never seen lockd exit
abnormally like this.

-- 
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux