Messages in this thread | | | Date | Sun, 22 Jan 2006 21:22:43 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH] shrink_dcache_parent() races against shrink_dcache_memory() |
| |
Jan Blunck <jblunck@suse.de> wrote: > > Kirill Korotaev <dev@sw.ru> discovered a race between shrink_dcache_parent() > and shrink_dcache_memory(). That one is based on dput() is calling > dentry_iput() too early and therefore is giving up the dcache_lock. This leads > to the situation that the parent dentry might be still referenced although all > childs are already dead. This parent is ignore by a concurrent select_parent() > call which might be the reason for busy inode after umount failures. > > This is from Kirill's original patch: > > CPU 1 CPU 2 > ~~~~~ ~~~~~ > umount /dev/sda1 > generic_shutdown_super shrink_dcache_memory > shrink_dcache_parent prune_one_dentry > select_parent dput <<<< child is dead, locks are released, > but parent is still referenced!!! >>>> > skip dentry->parent, > since it's d_count > 0 > > message: BUSY inodes after umount... > <<< parent is left on dentry_unused list, > referencing freed super block >>> > > This patch is introducing dput_locked() which is doing all the dput work > except of freeing up the dentry's inode and memory itself. Therefore, when the > dcache_lock is given up, all the reference counts of the parents are correct. > prune_one_dentry() must also use the dput_locked version and free up the > inodes and the memory of the parents later. Otherwise we have an incorrect > reference count on the parents of the dentry to prune. > > ...
> -void dput(struct dentry *dentry) > +static void dput_locked(struct dentry *dentry, struct list_head *list) > { > if (!dentry) > return; > > -repeat: > - if (atomic_read(&dentry->d_count) == 1) > - might_sleep(); > - if (!atomic_dec_and_lock(&dentry->d_count, &dcache_lock)) > + if (!atomic_dec_and_test(&dentry->d_count)) > return; > > + > > ... > > +void dput(struct dentry *dentry) > +{ > + LIST_HEAD(free_list); > + > + if (!dentry) > + return; > + > + if (atomic_add_unless(&dentry->d_count, -1, 1)) > + return; > + > + spin_lock(&dcache_lock); > + dput_locked(dentry, &free_list); > + spin_unlock(&dcache_lock);
This seems to be an open-coded copy of atomic_dec_and_lock()?
- 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/
| |