* [PATCH 1/2] devpts: remove unneeded inode_lock in mknod_ptmx
@ 2019-01-28 17:11 Christian Brauner
2019-01-28 17:11 ` [PATCH 2/2] devpts: simplify devpts_fill_super() Christian Brauner
2019-02-19 9:24 ` [PATCH 1/2] devpts: remove unneeded inode_lock in mknod_ptmx Christian Brauner
0 siblings, 2 replies; 3+ messages in thread
From: Christian Brauner @ 2019-01-28 17:11 UTC (permalink / raw)
To: linux-kernel, viro, linux-fsdevel
Cc: gregkh, ebiederm, ebiggers, willy, dhowells, Christian Brauner
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(-)
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/2] devpts: simplify devpts_fill_super()
2019-01-28 17:11 [PATCH 1/2] devpts: remove unneeded inode_lock in mknod_ptmx Christian Brauner
@ 2019-01-28 17:11 ` Christian Brauner
2019-02-19 9:24 ` [PATCH 1/2] devpts: remove unneeded inode_lock in mknod_ptmx Christian Brauner
1 sibling, 0 replies; 3+ messages in thread
From: Christian Brauner @ 2019-01-28 17:11 UTC (permalink / raw)
To: linux-kernel, viro, linux-fsdevel
Cc: gregkh, ebiederm, ebiggers, willy, dhowells, Christian Brauner
When devpts_fill_super() fails deactivate_locked_super() will cleanup
dentries for us so there's no need to do that work in devpts_fill_super()
itself. This allows us to simplify that function quite a bit.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christian Brauner <christian@brauner.io>
---
fs/devpts/inode.c | 22 ++++++----------------
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index 8fa1492f9712..59b7625a2110 100644
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -450,19 +450,18 @@ devpts_fill_super(struct super_block *s, void *data, int silent)
s->s_op = &devpts_sops;
s->s_time_gran = 1;
- error = -ENOMEM;
s->s_fs_info = new_pts_fs_info(s);
if (!s->s_fs_info)
- goto fail;
+ return -ENOMEM;
error = parse_mount_options(data, PARSE_MOUNT, &DEVPTS_SB(s)->mount_opts);
if (error)
- goto fail;
+ return error;
- error = -ENOMEM;
inode = new_inode(s);
if (!inode)
- goto fail;
+ return -ENOMEM;
+
inode->i_ino = 1;
inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
inode->i_mode = S_IFDIR | S_IRUGO | S_IXUGO | S_IWUSR;
@@ -473,19 +472,10 @@ devpts_fill_super(struct super_block *s, void *data, int silent)
s->s_root = d_make_root(inode);
if (!s->s_root) {
pr_err("get root dentry failed\n");
- goto fail;
+ return -ENOMEM;
}
- error = mknod_ptmx(s);
- if (error)
- goto fail_dput;
-
- return 0;
-fail_dput:
- dput(s->s_root);
- s->s_root = NULL;
-fail:
- return error;
+ return mknod_ptmx(s);
}
/*
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/2] devpts: remove unneeded inode_lock in mknod_ptmx
2019-01-28 17:11 [PATCH 1/2] devpts: remove unneeded inode_lock in mknod_ptmx Christian Brauner
2019-01-28 17:11 ` [PATCH 2/2] devpts: simplify devpts_fill_super() Christian Brauner
@ 2019-02-19 9:24 ` Christian Brauner
1 sibling, 0 replies; 3+ messages in thread
From: Christian Brauner @ 2019-02-19 9:24 UTC (permalink / raw)
To: linux-kernel, viro, linux-fsdevel
Cc: gregkh, ebiederm, ebiggers, willy, dhowells
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-02-19 9:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-28 17:11 [PATCH 1/2] devpts: remove unneeded inode_lock in mknod_ptmx Christian Brauner
2019-01-28 17:11 ` [PATCH 2/2] devpts: simplify devpts_fill_super() Christian Brauner
2019-02-19 9:24 ` [PATCH 1/2] devpts: remove unneeded inode_lock in mknod_ptmx Christian Brauner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).