Re: CVS commit: src/sys/ufs/ufs
On Tue, Sep 22, 2009 at 10:42:59PM +0300, Antti Kantee wrote: Blah, I didn't even want to think about this migrane-inducer now. Maybe people who have recently worked on vnode reclaiming could instead be the ones to comment? It's becoming clear that this is something I'm going to need to wade into. Much as I've been trying to avoid it. :-/ There's a (perfectly natural) tendency to try to fix synchronization problems by adding states -- extra flags, more locks, moving things to the background, etc. -- but in general the way to fix synchronization problems so they *stay* fixed is to remove states. For example, from what I've seen so far I'm pretty sure XLOCK ought to go away. -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/ufs/ufs
On Sep,Tuesday 22 2009, at 9:42 PM, Antti Kantee wrote: On Tue Sep 22 2009 at 21:04:14 +0200, Manuel Bouyer wrote: that's not an issue with the reference count. It's an issue with vclean() calling VOP_RECLAIM() even if the refcount is greater than 1, and vrelel() calling vclean() even if the refcount is greater than 1, when the file has been unlink(2)ed. There's a comment about this in vrelel(), look for variable recycle. Ah, ic, probably would've been easier if I read the PR first ;) So basically someone can vget the vnode (via fhtovp, since it's gone from the directory namespace) between VOP_INACTIVE and clean. Your fix doesn't really fix this problem, since depending on timings the inode might still be recycled after you check it's valid. Hmm, ok, so things which might happen: 1: VOP_REMOVE, vnode is locked, vrele is called 2: fhtovp before inode is reclaimed, blocks on vn_lock 1: VOP_INACTIVE releases vnlock, returns signal to recycle vnode 2: gets lock, does check that it's still the same inode 1: reclaims vnode 2: boom Since vget takes the interlock, a dirty cheap trick might be to check that the reference count is still one before trying to clean the vnode in vrelel(), otherwise punting and letting the next release-to-0 reclaim it? Blah, I didn't even want to think about this migrane-inducer now. Maybe people who have recently worked on vnode reclaiming could instead be the ones to comment? Sorry I haven't read this thread until today. I think that it would be good to test my vreclaim patched kernel it might help to resolve your problem. But I don't think it is a solution see above. Regards Adam.
Re: CVS commit: src/sys/ufs/ufs
On Wed, Sep 23, 2009 at 06:09:00PM +0200, Adam Hamsik wrote: Sorry I haven't read this thread until today. I think that it would be good to test my vreclaim patched kernel it might help to resolve your problem. But I don't think it is a solution see above. Your patch won't help. It only changes the behavior or getnewvnode(), and in this problem getnewvnode() isn't involved at all. Also I can't see how your patch would help, a vnode for an inode which has been deleted still needs to be invalidated at unlink time, and can't be deffered to another thread. -- Manuel Bouyer bou...@antioche.eu.org NetBSD: 26 ans d'experience feront toujours la difference --
Re: CVS commit: src/sys/ufs/ufs
On Wed, Sep 23, 2009 at 03:08:14PM +, David Holland wrote: It's becoming clear that this is something I'm going to need to wade into. Much as I've been trying to avoid it. :-/ There's a (perfectly natural) tendency to try to fix synchronization problems by adding states -- extra flags, more locks, moving things to the background, etc. -- but in general the way to fix synchronization problems so they *stay* fixed is to remove states. For example, from what I've seen so far I'm pretty sure XLOCK ought to go away. That's possible, at last in its current form as it fails to protect vget(). -- Manuel Bouyer bou...@antioche.eu.org NetBSD: 26 ans d'experience feront toujours la difference --
