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

Al Viro
2005-11-03T08:07:18Z

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...