Skip to content

Commit

Permalink
Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel…
Browse files Browse the repository at this point in the history
…/git/viro/vfs

Pull vfs fixes from Al Viro:
 "Followups to the parallel lookup work:

   - update docs

   - restore killability of the places that used to take ->i_mutex
     killably now that we have down_write_killable() merged

   - Additionally, it turns out that I missed a prerequisite for
     security_d_instantiate() stuff - ->getxattr() wasn't the only thing
     that could be called before dentry is attached to inode; with smack
     we needed the same treatment applied to ->setxattr() as well"

* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs:
  switch ->setxattr() to passing dentry and inode separately
  switch xattr_handler->set() to passing dentry and inode separately
  restore killability of old mutex_lock_killable(&inode->i_mutex) users
  add down_write_killable_nested()
  update D/f/directory-locking
  • Loading branch information
torvalds committed May 28, 2016
2 parents 0121a32 + 3767e25 commit d102a56
Show file tree
Hide file tree
Showing 58 changed files with 265 additions and 209 deletions.
32 changes: 20 additions & 12 deletions Documentation/filesystems/directory-locking
Original file line number Diff line number Diff line change
@@ -1,30 +1,37 @@
Locking scheme used for directory operations is based on two
kinds of locks - per-inode (->i_mutex) and per-filesystem
kinds of locks - per-inode (->i_rwsem) and per-filesystem
(->s_vfs_rename_mutex).

When taking the i_mutex on multiple non-directory objects, we
When taking the i_rwsem on multiple non-directory objects, we
always acquire the locks in order by increasing address. We'll call
that "inode pointer" order in the following.

For our purposes all operations fall in 5 classes:

1) read access. Locking rules: caller locks directory we are accessing.
The lock is taken shared.

2) object creation. Locking rules: same as above.
2) object creation. Locking rules: same as above, but the lock is taken
exclusive.

3) object removal. Locking rules: caller locks parent, finds victim,
locks victim and calls the method.
locks victim and calls the method. Locks are exclusive.

4) rename() that is _not_ cross-directory. Locking rules: caller locks
the parent and finds source and target. If target already exists, lock
it. If source is a non-directory, lock it. If that means we need to
lock both, lock them in inode pointer order.
the parent and finds source and target. In case of exchange (with
RENAME_EXCHANGE in rename2() flags argument) lock both. In any case,
if the target already exists, lock it. If the source is a non-directory,
lock it. If we need to lock both, lock them in inode pointer order.
Then call the method. All locks are exclusive.
NB: we might get away with locking the the source (and target in exchange
case) shared.

5) link creation. Locking rules:
* lock parent
* check that source is not a directory
* lock source
* call the method.
All locks are exclusive.

6) cross-directory rename. The trickiest in the whole bunch. Locking
rules:
Expand All @@ -35,11 +42,12 @@ rules:
fail with -ENOTEMPTY
* if new parent is equal to or is a descendent of source
fail with -ELOOP
* If target exists, lock it. If source is a non-directory, lock
it. In case that means we need to lock both source and target,
do so in inode pointer order.
* If it's an exchange, lock both the source and the target.
* If the target exists, lock it. If the source is a non-directory,
lock it. If we need to lock both, do so in inode pointer order.
* call the method.

All ->i_rwsem are taken exclusive. Again, we might get away with locking
the the source (and target in exchange case) shared.

The rules above obviously guarantee that all directories that are going to be
read, modified or removed by method will be locked by caller.
Expand Down Expand Up @@ -73,7 +81,7 @@ objects - A < B iff A is an ancestor of B.
attempt to acquire some lock and already holds at least one lock. Let's
consider the set of contended locks. First of all, filesystem lock is
not contended, since any process blocked on it is not holding any locks.
Thus all processes are blocked on ->i_mutex.
Thus all processes are blocked on ->i_rwsem.

By (3), any process holding a non-directory lock can only be
waiting on another non-directory lock with a larger address. Therefore
Expand Down
7 changes: 7 additions & 0 deletions Documentation/filesystems/porting
Original file line number Diff line number Diff line change
Expand Up @@ -578,3 +578,10 @@ in your dentry operations instead.
--
[mandatory]
->atomic_open() calls without O_CREAT may happen in parallel.
--
[mandatory]
->setxattr() and xattr_handler.set() get dentry and inode passed separately.
dentry might be yet to be attached to inode, so do _not_ use its ->d_inode
in the instances. Rationale: !@#!@# security_d_instantiate() needs to be
called before we attach dentry to inode and !@#!@##!@$!$#!@#$!@$!@$ smack
->d_instantiate() uses not just ->getxattr() but ->setxattr() as well.
4 changes: 2 additions & 2 deletions drivers/staging/lustre/lustre/llite/llite_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -976,8 +976,8 @@ static inline __u64 ll_file_maxbytes(struct inode *inode)
}

/* llite/xattr.c */
int ll_setxattr(struct dentry *dentry, const char *name,
const void *value, size_t size, int flags);
int ll_setxattr(struct dentry *dentry, struct inode *inode,
const char *name, const void *value, size_t size, int flags);
ssize_t ll_getxattr(struct dentry *dentry, struct inode *inode,
const char *name, void *buffer, size_t size);
ssize_t ll_listxattr(struct dentry *dentry, char *buffer, size_t size);
Expand Down
6 changes: 2 additions & 4 deletions drivers/staging/lustre/lustre/llite/xattr.c
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,9 @@ int ll_setxattr_common(struct inode *inode, const char *name,
return 0;
}

int ll_setxattr(struct dentry *dentry, const char *name,
const void *value, size_t size, int flags)
int ll_setxattr(struct dentry *dentry, struct inode *inode,
const char *name, const void *value, size_t size, int flags)
{
struct inode *inode = d_inode(dentry);

LASSERT(inode);
LASSERT(name);

Expand Down
6 changes: 3 additions & 3 deletions fs/9p/acl.c
Original file line number Diff line number Diff line change
Expand Up @@ -239,13 +239,13 @@ static int v9fs_xattr_get_acl(const struct xattr_handler *handler,
}

static int v9fs_xattr_set_acl(const struct xattr_handler *handler,
struct dentry *dentry, const char *name,
const void *value, size_t size, int flags)
struct dentry *dentry, struct inode *inode,
const char *name, const void *value,
size_t size, int flags)
{
int retval;
struct posix_acl *acl;
struct v9fs_session_info *v9ses;
struct inode *inode = d_inode(dentry);

v9ses = v9fs_dentry2v9ses(dentry);
/*
Expand Down
5 changes: 3 additions & 2 deletions fs/9p/xattr.c
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,9 @@ static int v9fs_xattr_handler_get(const struct xattr_handler *handler,
}

static int v9fs_xattr_handler_set(const struct xattr_handler *handler,
struct dentry *dentry, const char *name,
const void *value, size_t size, int flags)
struct dentry *dentry, struct inode *inode,
const char *name, const void *value,
size_t size, int flags)
{
const char *full_name = xattr_full_name(handler, name);

Expand Down
4 changes: 2 additions & 2 deletions fs/bad_inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ static int bad_inode_setattr(struct dentry *direntry, struct iattr *attrs)
return -EIO;
}

static int bad_inode_setxattr(struct dentry *dentry, const char *name,
const void *value, size_t size, int flags)
static int bad_inode_setxattr(struct dentry *dentry, struct inode *inode,
const char *name, const void *value, size_t size, int flags)
{
return -EIO;
}
Expand Down
18 changes: 7 additions & 11 deletions fs/btrfs/ioctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -846,11 +846,9 @@ static noinline int btrfs_mksubvol(struct path *parent,
struct dentry *dentry;
int error;

inode_lock_nested(dir, I_MUTEX_PARENT);
// XXX: should've been
// mutex_lock_killable_nested(&dir->i_mutex, I_MUTEX_PARENT);
// if (error == -EINTR)
// return error;
error = down_write_killable_nested(&dir->i_rwsem, I_MUTEX_PARENT);
if (error == -EINTR)
return error;

dentry = lookup_one_len(name, parent->dentry, namelen);
error = PTR_ERR(dentry);
Expand Down Expand Up @@ -2377,11 +2375,9 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
goto out;


inode_lock_nested(dir, I_MUTEX_PARENT);
// XXX: should've been
// err = mutex_lock_killable_nested(&dir->i_mutex, I_MUTEX_PARENT);
// if (err == -EINTR)
// goto out_drop_write;
err = down_write_killable_nested(&dir->i_rwsem, I_MUTEX_PARENT);
if (err == -EINTR)
goto out_drop_write;
dentry = lookup_one_len(vol_args->name, parent, namelen);
if (IS_ERR(dentry)) {
err = PTR_ERR(dentry);
Expand Down Expand Up @@ -2571,7 +2567,7 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
dput(dentry);
out_unlock_dir:
inode_unlock(dir);
//out_drop_write:
out_drop_write:
mnt_drop_write_file(file);
out:
kfree(vol_args);
Expand Down
12 changes: 5 additions & 7 deletions fs/btrfs/xattr.c
Original file line number Diff line number Diff line change
Expand Up @@ -380,23 +380,21 @@ static int btrfs_xattr_handler_get(const struct xattr_handler *handler,
}

static int btrfs_xattr_handler_set(const struct xattr_handler *handler,
struct dentry *dentry, const char *name,
const void *buffer, size_t size,
int flags)
struct dentry *unused, struct inode *inode,
const char *name, const void *buffer,
size_t size, int flags)
{
struct inode *inode = d_inode(dentry);

name = xattr_full_name(handler, name);
return __btrfs_setxattr(NULL, inode, name, buffer, size, flags);
}

static int btrfs_xattr_handler_set_prop(const struct xattr_handler *handler,
struct dentry *dentry,
struct dentry *unused, struct inode *inode,
const char *name, const void *value,
size_t size, int flags)
{
name = xattr_full_name(handler, name);
return btrfs_set_prop(d_inode(dentry), name, value, size, flags);
return btrfs_set_prop(inode, name, value, size, flags);
}

static const struct xattr_handler btrfs_security_xattr_handler = {
Expand Down
7 changes: 4 additions & 3 deletions fs/ceph/xattr.c
Original file line number Diff line number Diff line change
Expand Up @@ -1056,12 +1056,13 @@ static int ceph_get_xattr_handler(const struct xattr_handler *handler,
}

static int ceph_set_xattr_handler(const struct xattr_handler *handler,
struct dentry *dentry, const char *name,
const void *value, size_t size, int flags)
struct dentry *unused, struct inode *inode,
const char *name, const void *value,
size_t size, int flags)
{
if (!ceph_is_valid_xattr(name))
return -EOPNOTSUPP;
return __ceph_setxattr(d_inode(dentry), name, value, size, flags);
return __ceph_setxattr(inode, name, value, size, flags);
}

const struct xattr_handler ceph_other_xattr_handler = {
Expand Down
9 changes: 5 additions & 4 deletions fs/cifs/xattr.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@
enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT };

static int cifs_xattr_set(const struct xattr_handler *handler,
struct dentry *dentry, const char *name,
const void *value, size_t size, int flags)
struct dentry *dentry, struct inode *inode,
const char *name, const void *value,
size_t size, int flags)
{
int rc = -EOPNOTSUPP;
unsigned int xid;
Expand Down Expand Up @@ -99,12 +100,12 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
if (value &&
pTcon->ses->server->ops->set_acl)
rc = pTcon->ses->server->ops->set_acl(pacl,
size, d_inode(dentry),
size, inode,
full_path, CIFS_ACL_DACL);
else
rc = -EOPNOTSUPP;
if (rc == 0) /* force revalidate of the inode */
CIFS_I(d_inode(dentry))->time = 0;
CIFS_I(inode)->time = 0;
kfree(pacl);
}
#endif /* CONFIG_CIFS_ACL */
Expand Down
9 changes: 5 additions & 4 deletions fs/ecryptfs/crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -1141,12 +1141,13 @@ ecryptfs_write_metadata_to_contents(struct inode *ecryptfs_inode,

static int
ecryptfs_write_metadata_to_xattr(struct dentry *ecryptfs_dentry,
struct inode *ecryptfs_inode,
char *page_virt, size_t size)
{
int rc;

rc = ecryptfs_setxattr(ecryptfs_dentry, ECRYPTFS_XATTR_NAME, page_virt,
size, 0);
rc = ecryptfs_setxattr(ecryptfs_dentry, ecryptfs_inode,
ECRYPTFS_XATTR_NAME, page_virt, size, 0);
return rc;
}

Expand Down Expand Up @@ -1215,8 +1216,8 @@ int ecryptfs_write_metadata(struct dentry *ecryptfs_dentry,
goto out_free;
}
if (crypt_stat->flags & ECRYPTFS_METADATA_IN_XATTR)
rc = ecryptfs_write_metadata_to_xattr(ecryptfs_dentry, virt,
size);
rc = ecryptfs_write_metadata_to_xattr(ecryptfs_dentry, ecryptfs_inode,
virt, size);
else
rc = ecryptfs_write_metadata_to_contents(ecryptfs_inode, virt,
virt_len);
Expand Down
4 changes: 2 additions & 2 deletions fs/ecryptfs/ecryptfs_kernel.h
Original file line number Diff line number Diff line change
Expand Up @@ -609,8 +609,8 @@ ssize_t
ecryptfs_getxattr_lower(struct dentry *lower_dentry, struct inode *lower_inode,
const char *name, void *value, size_t size);
int
ecryptfs_setxattr(struct dentry *dentry, const char *name, const void *value,
size_t size, int flags);
ecryptfs_setxattr(struct dentry *dentry, struct inode *inode, const char *name,
const void *value, size_t size, int flags);
int ecryptfs_read_xattr_region(char *page_virt, struct inode *ecryptfs_inode);
#ifdef CONFIG_ECRYPT_FS_MESSAGING
int ecryptfs_process_response(struct ecryptfs_daemon *daemon,
Expand Down
7 changes: 4 additions & 3 deletions fs/ecryptfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -1001,7 +1001,8 @@ static int ecryptfs_getattr(struct vfsmount *mnt, struct dentry *dentry,
}

int
ecryptfs_setxattr(struct dentry *dentry, const char *name, const void *value,
ecryptfs_setxattr(struct dentry *dentry, struct inode *inode,
const char *name, const void *value,
size_t size, int flags)
{
int rc = 0;
Expand All @@ -1014,8 +1015,8 @@ ecryptfs_setxattr(struct dentry *dentry, const char *name, const void *value,
}

rc = vfs_setxattr(lower_dentry, name, value, size, flags);
if (!rc && d_really_is_positive(dentry))
fsstack_copy_attr_all(d_inode(dentry), d_inode(lower_dentry));
if (!rc && inode)
fsstack_copy_attr_all(inode, d_inode(lower_dentry));
out:
return rc;
}
Expand Down
3 changes: 2 additions & 1 deletion fs/ecryptfs/mmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,8 @@ static int ecryptfs_write_inode_size_to_xattr(struct inode *ecryptfs_inode)
if (size < 0)
size = 8;
put_unaligned_be64(i_size_read(ecryptfs_inode), xattr_virt);
rc = lower_inode->i_op->setxattr(lower_dentry, ECRYPTFS_XATTR_NAME,
rc = lower_inode->i_op->setxattr(lower_dentry, lower_inode,
ECRYPTFS_XATTR_NAME,
xattr_virt, size, 0);
inode_unlock(lower_inode);
if (rc)
Expand Down
7 changes: 4 additions & 3 deletions fs/ext2/xattr_security.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ ext2_xattr_security_get(const struct xattr_handler *handler,

static int
ext2_xattr_security_set(const struct xattr_handler *handler,
struct dentry *dentry, const char *name,
const void *value, size_t size, int flags)
struct dentry *unused, struct inode *inode,
const char *name, const void *value,
size_t size, int flags)
{
return ext2_xattr_set(d_inode(dentry), EXT2_XATTR_INDEX_SECURITY, name,
return ext2_xattr_set(inode, EXT2_XATTR_INDEX_SECURITY, name,
value, size, flags);
}

Expand Down
7 changes: 4 additions & 3 deletions fs/ext2/xattr_trusted.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@ ext2_xattr_trusted_get(const struct xattr_handler *handler,

static int
ext2_xattr_trusted_set(const struct xattr_handler *handler,
struct dentry *dentry, const char *name,
const void *value, size_t size, int flags)
struct dentry *unused, struct inode *inode,
const char *name, const void *value,
size_t size, int flags)
{
return ext2_xattr_set(d_inode(dentry), EXT2_XATTR_INDEX_TRUSTED, name,
return ext2_xattr_set(inode, EXT2_XATTR_INDEX_TRUSTED, name,
value, size, flags);
}

Expand Down
9 changes: 5 additions & 4 deletions fs/ext2/xattr_user.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@ ext2_xattr_user_get(const struct xattr_handler *handler,

static int
ext2_xattr_user_set(const struct xattr_handler *handler,
struct dentry *dentry, const char *name,
const void *value, size_t size, int flags)
struct dentry *unused, struct inode *inode,
const char *name, const void *value,
size_t size, int flags)
{
if (!test_opt(dentry->d_sb, XATTR_USER))
if (!test_opt(inode->i_sb, XATTR_USER))
return -EOPNOTSUPP;

return ext2_xattr_set(d_inode(dentry), EXT2_XATTR_INDEX_USER,
return ext2_xattr_set(inode, EXT2_XATTR_INDEX_USER,
name, value, size, flags);
}

Expand Down
7 changes: 4 additions & 3 deletions fs/ext4/xattr_security.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ ext4_xattr_security_get(const struct xattr_handler *handler,

static int
ext4_xattr_security_set(const struct xattr_handler *handler,
struct dentry *dentry, const char *name,
const void *value, size_t size, int flags)
struct dentry *unused, struct inode *inode,
const char *name, const void *value,
size_t size, int flags)
{
return ext4_xattr_set(d_inode(dentry), EXT4_XATTR_INDEX_SECURITY,
return ext4_xattr_set(inode, EXT4_XATTR_INDEX_SECURITY,
name, value, size, flags);
}

Expand Down
Loading

0 comments on commit d102a56

Please sign in to comment.