Commit 8ead9dd5 authored by Linus Torvalds's avatar Linus Torvalds

devpts: more pty driver interface cleanups

This is more prep-work for the upcoming pty changes.  Still just code
cleanup with no actual semantic changes.

This removes a bunch pointless complexity by just having the slave pty
side remember the dentry associated with the devpts slave rather than
the inode.  That allows us to remove all the "look up the dentry" code
for when we want to remove it again.

Together with moving the tty pointer from "inode->i_private" to
"dentry->d_fsdata" and getting rid of pointless inode locking, this
removes about 30 lines of code.  Not only is the end result smaller,
it's simpler and easier to understand.

The old code, for example, depended on the d_find_alias() to not just
find the dentry, but also to check that it is still hashed, which in
turn validated the tty pointer in the inode.

That is a _very_ roundabout way to say "invalidate the cached tty
pointer when the dentry is removed".

The new code just does

	dentry->d_fsdata = NULL;

in devpts_pty_kill() instead, invalidating the tty pointer rather more
directly and obviously.  Don't do something complex and subtle when the
obvious straightforward approach will do.

The rest of the patch (ie apart from code deletion and the above tty
pointer clearing) is just switching the calling convention to pass the
dentry or file pointer around instead of the inode.

Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Peter Anvin <hpa@zytor.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Peter Hurley <peter@hurleysoftware.com>
Cc: Serge Hallyn <serge.hallyn@ubuntu.com>
Cc: Willy Tarreau <w@1wt.eu>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Alan Cox <gnomes@lxorguk.ukuu.org.uk>
Cc: Jann Horn <jann@thejh.net>
Cc: Greg KH <greg@kroah.com>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Florian Weimer <fw@deneb.enyo.de>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent bcc981e9
...@@ -626,7 +626,7 @@ static int pty_unix98_ioctl(struct tty_struct *tty, ...@@ -626,7 +626,7 @@ static int pty_unix98_ioctl(struct tty_struct *tty,
*/ */
static struct tty_struct *ptm_unix98_lookup(struct tty_driver *driver, static struct tty_struct *ptm_unix98_lookup(struct tty_driver *driver,
struct inode *ptm_inode, int idx) struct file *file, int idx)
{ {
/* Master must be open via /dev/ptmx */ /* Master must be open via /dev/ptmx */
return ERR_PTR(-EIO); return ERR_PTR(-EIO);
...@@ -642,12 +642,12 @@ static struct tty_struct *ptm_unix98_lookup(struct tty_driver *driver, ...@@ -642,12 +642,12 @@ static struct tty_struct *ptm_unix98_lookup(struct tty_driver *driver,
*/ */
static struct tty_struct *pts_unix98_lookup(struct tty_driver *driver, static struct tty_struct *pts_unix98_lookup(struct tty_driver *driver,
struct inode *pts_inode, int idx) struct file *file, int idx)
{ {
struct tty_struct *tty; struct tty_struct *tty;
mutex_lock(&devpts_mutex); mutex_lock(&devpts_mutex);
tty = devpts_get_priv(pts_inode); tty = devpts_get_priv(file->f_path.dentry);
mutex_unlock(&devpts_mutex); mutex_unlock(&devpts_mutex);
/* Master must be open before slave */ /* Master must be open before slave */
if (!tty) if (!tty)
...@@ -722,7 +722,7 @@ static int ptmx_open(struct inode *inode, struct file *filp) ...@@ -722,7 +722,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
{ {
struct pts_fs_info *fsi; struct pts_fs_info *fsi;
struct tty_struct *tty; struct tty_struct *tty;
struct inode *slave_inode; struct dentry *dentry;
int retval; int retval;
int index; int index;
...@@ -769,14 +769,12 @@ static int ptmx_open(struct inode *inode, struct file *filp) ...@@ -769,14 +769,12 @@ static int ptmx_open(struct inode *inode, struct file *filp)
tty_add_file(tty, filp); tty_add_file(tty, filp);
slave_inode = devpts_pty_new(fsi, dentry = devpts_pty_new(fsi, index, tty->link);
MKDEV(UNIX98_PTY_SLAVE_MAJOR, index), index, if (IS_ERR(dentry)) {
tty->link); retval = PTR_ERR(dentry);
if (IS_ERR(slave_inode)) {
retval = PTR_ERR(slave_inode);
goto err_release; goto err_release;
} }
tty->link->driver_data = slave_inode; tty->link->driver_data = dentry;
retval = ptm_driver->ops->open(tty, filp); retval = ptm_driver->ops->open(tty, filp);
if (retval) if (retval)
......
...@@ -1367,12 +1367,12 @@ static ssize_t tty_line_name(struct tty_driver *driver, int index, char *p) ...@@ -1367,12 +1367,12 @@ static ssize_t tty_line_name(struct tty_driver *driver, int index, char *p)
* Locking: tty_mutex must be held. If the tty is found, bump the tty kref. * Locking: tty_mutex must be held. If the tty is found, bump the tty kref.
*/ */
static struct tty_struct *tty_driver_lookup_tty(struct tty_driver *driver, static struct tty_struct *tty_driver_lookup_tty(struct tty_driver *driver,
struct inode *inode, int idx) struct file *file, int idx)
{ {
struct tty_struct *tty; struct tty_struct *tty;
if (driver->ops->lookup) if (driver->ops->lookup)
tty = driver->ops->lookup(driver, inode, idx); tty = driver->ops->lookup(driver, file, idx);
else else
tty = driver->ttys[idx]; tty = driver->ttys[idx];
...@@ -2040,7 +2040,7 @@ static struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode, ...@@ -2040,7 +2040,7 @@ static struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
} }
/* check whether we're reopening an existing tty */ /* check whether we're reopening an existing tty */
tty = tty_driver_lookup_tty(driver, inode, index); tty = tty_driver_lookup_tty(driver, filp, index);
if (IS_ERR(tty)) { if (IS_ERR(tty)) {
mutex_unlock(&tty_mutex); mutex_unlock(&tty_mutex);
goto out; goto out;
......
...@@ -604,8 +604,7 @@ void devpts_put_ref(struct pts_fs_info *fsi) ...@@ -604,8 +604,7 @@ void devpts_put_ref(struct pts_fs_info *fsi)
* *
* The created inode is returned. Remove it from /dev/pts/ by devpts_pty_kill. * The created inode is returned. Remove it from /dev/pts/ by devpts_pty_kill.
*/ */
struct inode *devpts_pty_new(struct pts_fs_info *fsi, dev_t device, int index, struct dentry *devpts_pty_new(struct pts_fs_info *fsi, int index, void *priv)
void *priv)
{ {
struct dentry *dentry; struct dentry *dentry;
struct super_block *sb; struct super_block *sb;
...@@ -629,25 +628,21 @@ struct inode *devpts_pty_new(struct pts_fs_info *fsi, dev_t device, int index, ...@@ -629,25 +628,21 @@ struct inode *devpts_pty_new(struct pts_fs_info *fsi, dev_t device, int index,
inode->i_uid = opts->setuid ? opts->uid : current_fsuid(); inode->i_uid = opts->setuid ? opts->uid : current_fsuid();
inode->i_gid = opts->setgid ? opts->gid : current_fsgid(); inode->i_gid = opts->setgid ? opts->gid : current_fsgid();
inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME; inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
init_special_inode(inode, S_IFCHR|opts->mode, device); init_special_inode(inode, S_IFCHR|opts->mode, MKDEV(UNIX98_PTY_SLAVE_MAJOR, index));
inode->i_private = priv;
sprintf(s, "%d", index); sprintf(s, "%d", index);
inode_lock(d_inode(root));
dentry = d_alloc_name(root, s); dentry = d_alloc_name(root, s);
if (dentry) { if (dentry) {
dentry->d_fsdata = priv;
d_add(dentry, inode); d_add(dentry, inode);
fsnotify_create(d_inode(root), dentry); fsnotify_create(d_inode(root), dentry);
} else { } else {
iput(inode); iput(inode);
inode = ERR_PTR(-ENOMEM); dentry = ERR_PTR(-ENOMEM);
} }
inode_unlock(d_inode(root)); return dentry;
return inode;
} }
/** /**
...@@ -656,24 +651,10 @@ struct inode *devpts_pty_new(struct pts_fs_info *fsi, dev_t device, int index, ...@@ -656,24 +651,10 @@ struct inode *devpts_pty_new(struct pts_fs_info *fsi, dev_t device, int index,
* *
* Returns whatever was passed as priv in devpts_pty_new for a given inode. * Returns whatever was passed as priv in devpts_pty_new for a given inode.
*/ */
void *devpts_get_priv(struct inode *pts_inode) void *devpts_get_priv(struct dentry *dentry)
{ {
struct dentry *dentry; WARN_ON_ONCE(dentry->d_sb->s_magic != DEVPTS_SUPER_MAGIC);
void *priv = NULL; return dentry->d_fsdata;
BUG_ON(pts_inode->i_rdev == MKDEV(TTYAUX_MAJOR, PTMX_MINOR));
/* Ensure dentry has not been deleted by devpts_pty_kill() */
dentry = d_find_alias(pts_inode);
if (!dentry)
return NULL;
if (pts_inode->i_sb->s_magic == DEVPTS_SUPER_MAGIC)
priv = pts_inode->i_private;
dput(dentry);
return priv;
} }
/** /**
...@@ -682,24 +663,14 @@ void *devpts_get_priv(struct inode *pts_inode) ...@@ -682,24 +663,14 @@ void *devpts_get_priv(struct inode *pts_inode)
* *
* This is an inverse operation of devpts_pty_new. * This is an inverse operation of devpts_pty_new.
*/ */
void devpts_pty_kill(struct inode *inode) void devpts_pty_kill(struct dentry *dentry)
{ {
struct super_block *sb = pts_sb_from_inode(inode); WARN_ON_ONCE(dentry->d_sb->s_magic != DEVPTS_SUPER_MAGIC);
struct dentry *root = sb->s_root;
struct dentry *dentry;
BUG_ON(inode->i_rdev == MKDEV(TTYAUX_MAJOR, PTMX_MINOR));
inode_lock(d_inode(root)); dentry->d_fsdata = NULL;
drop_nlink(dentry->d_inode);
dentry = d_find_alias(inode);
drop_nlink(inode);
d_delete(dentry); d_delete(dentry);
dput(dentry); /* d_alloc_name() in devpts_pty_new() */ dput(dentry); /* d_alloc_name() in devpts_pty_new() */
dput(dentry); /* d_find_alias above */
inode_unlock(d_inode(root));
} }
static int __init init_devpts_fs(void) static int __init init_devpts_fs(void)
......
...@@ -27,11 +27,11 @@ int devpts_new_index(struct pts_fs_info *); ...@@ -27,11 +27,11 @@ int devpts_new_index(struct pts_fs_info *);
void devpts_kill_index(struct pts_fs_info *, int); void devpts_kill_index(struct pts_fs_info *, int);
/* mknod in devpts */ /* mknod in devpts */
struct inode *devpts_pty_new(struct pts_fs_info *, dev_t, int, void *); struct dentry *devpts_pty_new(struct pts_fs_info *, int, void *);
/* get private structure */ /* get private structure */
void *devpts_get_priv(struct inode *pts_inode); void *devpts_get_priv(struct dentry *);
/* unlink */ /* unlink */
void devpts_pty_kill(struct inode *inode); void devpts_pty_kill(struct dentry *);
#endif #endif
......
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
* defined; unless noted otherwise, they are optional, and can be * defined; unless noted otherwise, they are optional, and can be
* filled in with a null pointer. * filled in with a null pointer.
* *
* struct tty_struct * (*lookup)(struct tty_driver *self, int idx) * struct tty_struct * (*lookup)(struct tty_driver *self, struct file *, int idx)
* *
* Return the tty device corresponding to idx, NULL if there is not * Return the tty device corresponding to idx, NULL if there is not
* one currently in use and an ERR_PTR value on error. Called under * one currently in use and an ERR_PTR value on error. Called under
...@@ -250,7 +250,7 @@ struct serial_icounter_struct; ...@@ -250,7 +250,7 @@ struct serial_icounter_struct;
struct tty_operations { struct tty_operations {
struct tty_struct * (*lookup)(struct tty_driver *driver, struct tty_struct * (*lookup)(struct tty_driver *driver,
struct inode *inode, int idx); struct file *filp, int idx);
int (*install)(struct tty_driver *driver, struct tty_struct *tty); int (*install)(struct tty_driver *driver, struct tty_struct *tty);
void (*remove)(struct tty_driver *driver, struct tty_struct *tty); void (*remove)(struct tty_driver *driver, struct tty_struct *tty);
int (*open)(struct tty_struct * tty, struct file * filp); int (*open)(struct tty_struct * tty, struct file * filp);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment