On Wed, Mar 26, 2008 at 10:11:34PM +0100, Miklos Szeredi wrote:
> +static int mnt_alloc_id(struct vfsmount *mnt)
> +{
> + int res;
> +
> + retry:
> + spin_lock(&vfsmount_lock);
> + res = ida_get_new(&mnt_id_ida, &mnt->mnt_id);
> + spin_unlock(&vfsmount_lock);
> + if (res == -EAGAIN) {
> + if (ida_pre_get(&mnt_id_ida, GFP_KERNEL))
> + goto retry;
> + res = -ENOMEM;
> + }
> + return res;
*Ugh*
Why bother with vfsmount_lock here? All allocations are done under
namespace_sem. Moreover, I'd rather replace that 'goto retry' with
a single call of ida_get_new(), since we are serialized anyway.
> @@ -353,6 +386,7 @@ EXPORT_SYMBOL(simple_set_mnt);
> void free_vfsmnt(struct vfsmount *mnt)
> {
> kfree(mnt->mnt_devname);
> + mnt_free_id(mnt);
> kmem_cache_free(mnt_cache, mnt);
> }
... and I'd rather do that earlier, e.g. in umount_tree(). At that point
we (a) have namespace_sem and (b) irrevocably kick the sucker out of
any namespace.
I'd rather minimize banging vfsmount_lock like that...
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
