Re: Race between "mount" uevent and /proc/mounts?

[Al Viro]

On Wed, Nov 02, 2005 at 04:01:18PM +0300, Sergey Vlasov wrote:
> @@ -120,6 +122,10 @@ static void detach_mnt(struct vfsmount *
>  	list_del_init(&mnt->mnt_child);
>  	list_del_init(&mnt->mnt_hash);
>  	old_nd->dentry->d_mounted--;
> +	if (current->namespace) {
> +		current->namespace->event++;
> +		wake_up_interruptible(&mounts_wait);
> +	}
>  }
>  
>  static void attach_mnt(struct vfsmount *mnt, struct nameidata *nd)

Ugh...  So umount -l gives one hell of a spew for no good reason.

> @@ -129,6 +135,8 @@ static void attach_mnt(struct vfsmount *
>  	list_add(&mnt->mnt_hash, mount_hashtable+hash(nd->mnt, nd->dentry));
>  	list_add_tail(&mnt->mnt_child, &nd->mnt->mnt_mounts);
>  	nd->dentry->d_mounted++;
> +	current->namespace->event++;
> +	wake_up_interruptible(&mounts_wait);
>  }

Bad idea - copy_tree() will spew *and* we get bogus events on CLONE_NEWNS
(i.e. current->namespace is not even the namespace being modified).

> @@ -1093,6 +1104,7 @@ int copy_namespace(int flags, struct tas
>  	atomic_set(&new_ns->count, 1);
>  	init_rwsem(&new_ns->sem);
>  	INIT_LIST_HEAD(&new_ns->list);
> +	new_ns->event = 0;

BTW, I'd rather make that queue per-namespace...

> +	down_read(&namespace->sem);
> +	if (private->last_event != namespace->event) {
> +		private->last_event = namespace->event;
> +		ret = POLLIN | POLLRDNORM;

Umm...  I'd rather use POLLERR, since POLLIN doesn't apply here - it's not
a stream of data that gives blocking read() when reached the end.


IMO the right approach is to have global event counter and do the following:
	if (namespace->event != event) {
		namespace->event = event;
		wake...
	}
in tree modifications and bump the event counter as soon as we decide
to do something.  I'll see how well that works with shared-subtree stuff
and post the patch if it turns out to be usable...
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/