| Message ID | 20190128171133.539-1-christian@brauner.io (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] devpts: remove unneeded inode_lock in mknod_ptmx | expand |
On Mon, Jan 28, 2019 at 06:11:32PM +0100, Christian Brauner wrote: > Afaict, the mknod_ptmx() call is a no-op on subsequent calls and the first > call is done before we unlock the suberblock. If I'm not mistaken this is > exactly parallel to what Al pointed out for binderfs 29ef1c8e16a > ("binderfs: drop lock in binderfs_binder_ctl_create"). In both filesystems > it should not be necessary to take inode_lock() in there. Let's remove it > and remove the goto. > > Cc: Al Viro <viro@zeniv.linux.org.uk> > Signed-off-by: Christian Brauner <christian@brauner.io> Since the merge window is coming up: are you fine with this patch, Al? Christian > --- > fs/devpts/inode.c | 19 ++++++------------- > 1 file changed, 6 insertions(+), 13 deletions(-) > > diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c > index c53814539070..8fa1492f9712 100644 > --- a/fs/devpts/inode.c > +++ b/fs/devpts/inode.c > @@ -325,7 +325,6 @@ static int parse_mount_options(char *data, int op, struct pts_mount_opts *opts) > static int mknod_ptmx(struct super_block *sb) > { > int mode; > - int rc = -ENOMEM; > struct dentry *dentry; > struct inode *inode; > struct dentry *root = sb->s_root; > @@ -334,18 +333,14 @@ static int mknod_ptmx(struct super_block *sb) > kuid_t ptmx_uid = current_fsuid(); > kgid_t ptmx_gid = current_fsgid(); > > - inode_lock(d_inode(root)); > - > /* If we have already created ptmx node, return */ > - if (fsi->ptmx_dentry) { > - rc = 0; > - goto out; > - } > + if (fsi->ptmx_dentry) > + return 0; > > dentry = d_alloc_name(root, "ptmx"); > if (!dentry) { > pr_err("Unable to alloc dentry for ptmx node\n"); > - goto out; > + return -ENOMEM; > } > > /* > @@ -355,7 +350,7 @@ static int mknod_ptmx(struct super_block *sb) > if (!inode) { > pr_err("Unable to alloc inode for ptmx node\n"); > dput(dentry); > - goto out; > + return -ENOMEM; > } > > inode->i_ino = 2; > @@ -369,10 +364,8 @@ static int mknod_ptmx(struct super_block *sb) > d_add(dentry, inode); > > fsi->ptmx_dentry = dentry; > - rc = 0; > -out: > - inode_unlock(d_inode(root)); > - return rc; > + > + return 0; > } > > static void update_ptmx_mode(struct pts_fs_info *fsi) > -- > 2.20.1 >
diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c index c53814539070..8fa1492f9712 100644 --- a/fs/devpts/inode.c +++ b/fs/devpts/inode.c @@ -325,7 +325,6 @@ static int parse_mount_options(char *data, int op, struct pts_mount_opts *opts) static int mknod_ptmx(struct super_block *sb) { int mode; - int rc = -ENOMEM; struct dentry *dentry; struct inode *inode; struct dentry *root = sb->s_root; @@ -334,18 +333,14 @@ static int mknod_ptmx(struct super_block *sb) kuid_t ptmx_uid = current_fsuid(); kgid_t ptmx_gid = current_fsgid(); - inode_lock(d_inode(root)); - /* If we have already created ptmx node, return */ - if (fsi->ptmx_dentry) { - rc = 0; - goto out; - } + if (fsi->ptmx_dentry) + return 0; dentry = d_alloc_name(root, "ptmx"); if (!dentry) { pr_err("Unable to alloc dentry for ptmx node\n"); - goto out; + return -ENOMEM; } /* @@ -355,7 +350,7 @@ static int mknod_ptmx(struct super_block *sb) if (!inode) { pr_err("Unable to alloc inode for ptmx node\n"); dput(dentry); - goto out; + return -ENOMEM; } inode->i_ino = 2; @@ -369,10 +364,8 @@ static int mknod_ptmx(struct super_block *sb) d_add(dentry, inode); fsi->ptmx_dentry = dentry; - rc = 0; -out: - inode_unlock(d_inode(root)); - return rc; + + return 0; } static void update_ptmx_mode(struct pts_fs_info *fsi)
Afaict, the mknod_ptmx() call is a no-op on subsequent calls and the first call is done before we unlock the suberblock. If I'm not mistaken this is exactly parallel to what Al pointed out for binderfs 29ef1c8e16a ("binderfs: drop lock in binderfs_binder_ctl_create"). In both filesystems it should not be necessary to take inode_lock() in there. Let's remove it and remove the goto. Cc: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: Christian Brauner <christian@brauner.io> --- fs/devpts/inode.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-)