[RFC][BUG] ns_mkdir_op() locking is FUBAR

[Al Viro <viro@zeniv.linux.org.uk>]

	AFAICS, you are trying to put your locks outside of the
->i_rwsem *and* take them from inside your ->mkdir() instance.
This
        /* we have to unlock and then relock to get locking order right
         * for pin_fs
         */
        inode_unlock(dir);
        error = simple_pin_fs(&aafs_ops, &aafs_mnt, &aafs_count);
        mutex_lock_nested(&parent->lock, parent->level);
        inode_lock_nested(dir, I_MUTEX_PARENT);
        if (error)
                goto out;

        error = __aafs_setup_d_inode(dir, dentry, mode | S_IFDIR,  NULL,
                                     NULL, NULL, NULL);
        if (error)
                goto out_pin;

        ns = __aa_find_or_create_ns(parent, READ_ONCE(dentry->d_name.name),
                                    dentry);

is completely broken.

Think what happens if two threads call mkdir() on the same thing.
OK, the first one got through vfs_mkdir() to the point where it
calls ->mkdir().  Parent is locked (->i_rwsem, exclusive), dentry
happens to be a hashed negative (e.g. from a slightly overlapping
earlier stat(2), whatever).

Your ->mkdir() drops the lock on parent, which is the only thing
holding the second thread back.  Now, the second thread gets in
and it happens to grab parent->lock first.  It makes dentry hashed
positive, drops the lock and buggers off.  Your first thread gets
parent->lock... and calls d_instantiate() on an already positive
dentry.  At which point the data structures are FUBAR.

Better yet, have mkdir A/B race with rmdir A.  After dropping
the locks in ->mkdir() you get the second thread getting through
the entire rmdir(2).  You get to __aa_find_or_create_ns(), which
calls __aa_create_ns(), which gets to
        error = __aafs_ns_mkdir(ns, ns_subns_dir(parent), name, dir);
Unfortunately, ns_subns_dir(parent) is already NULL, so you hit
        AA_BUG(!parent);
in __aafd_ns_mkdir().

AFAICS, rmdir/rmdir races are also there and also rather unpleasant.

Could you explain what exclusion are you trying to get there?
The mechanism is currently broken, but what is it trying to achieve?