Messages in this thread | | | Date | Fri, 3 Mar 2006 12:42:31 +0100 | From | Jan Blunck <> | Subject | Re: [PATCH] Busy inodes after unmount, be more verbose in generic_shutdown_super |
| |
On Thu, Mar 02, Neil Brown wrote:
> The core problem is that: > prune_one_dentry can hold a reference to a dentry without any > lock being held, and without any other reference to the > filesystem (if it is being called from shrink_dcache_memory). > It holds this reference while calling iput on an inode. This can > take an arbitrarily long time to complete, especially if NFS > needs to wait for some RPCs to complete or timeout. > > shrink_dcache_parent skips over dentries which have a reference, > such as the one held by prune_one_dentry. > > Thus umount can find that an inode is still in use (by it's dentry > which was skipped) and will complain. Worse, when the nfs request > on some inode finally completes, it might find the superblock > doesn't exist any more and... oops. > > My proposed solution to the problem is never to expose the reference > held by prune_one_dentry. i.e. keep the spin_lock held.
This morning I wondered, why I was using a list to drop the dentry's inodes after the dput() has visited all parents.
It is not enough to fix prune_one_dentry() to hold the lock until the parent is dereferenced since prune_one_dentry() calls __dput_locked(). And in __dput_locked() we have the same problem again:
from __dput_locked():
list_del(&dentry->d_u.d_child); dentry_stat.nr_dentry--; /* For d_free, below */ /*drops the locks, at that point nobody can reach this dentry*/ -> dentry_iput(dentry); parent = dentry->d_parent; d_free(dentry); if (dentry == parent) return; dentry = parent; + + if (atomic_read(&dentry->d_count) == 1) + might_sleep(); + if (!atomic_dec_and_lock(&dentry->d_count, &dcache_lock)) + return; + + spin_lock(&dentry->d_lock); goto repeat; } }
Between -> and the atomic_dec_and_lock() the reference count on the parent is wrong and no lock is held. I fixed that by using d_lru to keep track of all dentry's which inodes still have to be dereferenced. This should happen after all parents have been dereferenced.
Regards, Jan
-- Jan Blunck jblunck@suse.de SuSE LINUX AG - A Novell company Maxfeldstr. 5 +49-911-74053-608 D-90409 Nürnberg http://www.suse.de - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |