Messages in this thread |  | | | Date | Thu, 7 May 2009 16:35:49 -0400 | | Subject | Re: [v3][PATCH 5/5] Merge code for single and multiple-instance mounts | | From | Eric Paris <> |
| |
On Sat, Mar 7, 2009 at 2:12 PM, Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> wrote: > > From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> > Subject: [v3][PATCH 5/5] Merge code for single and multiple-instance mounts
I just tried to load the linux-next kernel on F11 and ran into a problem. X started, I could log in, I could start programs like firefox and evolution, but not gnome-terminal. It would just flash and disappear. Running xterm resulted in a window, that I could type in, but it wasn't a shell. It didn't do anything.
I switched to vt2 set the display to my X session and tried to run xterm. It said something about a permission being denied, so I decided to strace it. I saw EACCESS returning from calls dealing with /dev/pts/0. This lead me to git bisect start fs/devpts from the latest in linux-next as bad and 2.6.29 as good. Couple interations later and I find that this commit (1bd7903560f1f7) breaks gnome-terminal xterm!
So I have no idea why and haven't tried to figure it out (I'll try to poke it more tonight), but this is the commit that git bisect blames!
> new_pts_mount() (including the get_sb_nodev()), shares a lot of code > with init_pts_mount(). The only difference between them is the 'test-super' > function passed into sget(). > > Move all common code into devpts_get_sb() and remove the new_pts_mount() and > init_pts_mount() functions, > > Changelog[v3]: > [Serge Hallyn]: Remove unnecessary printk()s > Changelog[v2]: > (Christoph Hellwig): Merge code in 'do_pts_mount()' into devpts_get_sb() > > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> > Acked-by: Serge Hallyn <serue@us.ibm.com> > Tested-by: Serge Hallyn <serue@us.ibm.com> > --- > fs/devpts/inode.c | 115 ++++++++++++++++++---------------------------------- > 1 files changed, 40 insertions(+), 75 deletions(-) > > diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c > index c821c9a..c25fb34 100644 > --- a/fs/devpts/inode.c > +++ b/fs/devpts/inode.c > @@ -325,87 +325,38 @@ static int compare_init_pts_sb(struct super_block *s, void *p) > } > > /* > - * Mount a new (private) instance of devpts. PTYs created in this > - * instance are independent of the PTYs in other devpts instances. > - */ > -static int new_pts_mount(struct file_system_type *fs_type, int flags, > - void *data, struct pts_mount_opts *opts, struct vfsmount *mnt) > -{ > - int err; > - struct pts_fs_info *fsi; > - > - printk(KERN_NOTICE "devpts: newinstance mount\n"); > - > - err = get_sb_nodev(fs_type, flags, data, devpts_fill_super, mnt); > - if (err) > - return err; > - > - fsi = DEVPTS_SB(mnt->mnt_sb); > - memcpy(&fsi->mount_opts, opts, sizeof(opts)); > - > - return 0; > -} > - > -/* > - * init_pts_mount() > + * devpts_get_sb() > + * > + * If the '-o newinstance' mount option was specified, mount a new > + * (private) instance of devpts. PTYs created in this instance are > + * independent of the PTYs in other devpts instances. > * > - * Mount or remount the initial kernel mount of devpts. This type of > - * mount maintains the legacy, single-instance semantics, while the > - * kernel still allows multiple-instances. > + * If the '-o newinstance' option was not specified, mount/remount the > + * initial kernel mount of devpts. This type of mount gives the > + * legacy, single-instance semantics. > * > - * This interface is needed to support multiple namespace semantics in > - * devpts while preserving backward compatibility of the current 'single- > - * namespace' semantics. i.e all mounts of devpts without the 'newinstance' > - * mount option should bind to the initial kernel mount, like > - * get_sb_single(). > + * The 'newinstance' option is needed to support multiple namespace > + * semantics in devpts while preserving backward compatibility of the > + * current 'single-namespace' semantics. i.e all mounts of devpts > + * without the 'newinstance' mount option should bind to the initial > + * kernel mount, like get_sb_single(). > * > - * Mounts with 'newinstance' option create a new private namespace. > + * Mounts with 'newinstance' option create a new, private namespace. > * > - * But for single-mount semantics, devpts cannot use get_sb_single(), > + * NOTE: > + * > + * For single-mount semantics, devpts cannot use get_sb_single(), > * because get_sb_single()/sget() find and use the super-block from > * the most recent mount of devpts. But that recent mount may be a > * 'newinstance' mount and get_sb_single() would pick the newinstance > * super-block instead of the initial super-block. > - * > - * This interface is identical to get_sb_single() except that it > - * consistently selects the 'single-namespace' superblock even in the > - * presence of the private namespace (i.e 'newinstance') super-blocks. > */ > -static int init_pts_mount(struct file_system_type *fs_type, int flags, > - void *data, struct pts_mount_opts *opts, struct vfsmount *mnt) > -{ > - struct super_block *s; > - struct pts_fs_info *fsi; > - int error; > - > - s = sget(fs_type, compare_init_pts_sb, set_anon_super, NULL); > - if (IS_ERR(s)) > - return PTR_ERR(s); > - > - if (!s->s_root) { > - s->s_flags = flags; > - error = devpts_fill_super(s, data, flags & MS_SILENT ? 1 : 0); > - if (error) { > - up_write(&s->s_umount); > - deactivate_super(s); > - return error; > - } > - s->s_flags |= MS_ACTIVE; > - } > - > - simple_set_mnt(mnt, s); > - > - fsi = DEVPTS_SB(mnt->mnt_sb); > - memcpy(&fsi->mount_opts, opts, sizeof(opts)); > - > - return 0; > -} > - > static int devpts_get_sb(struct file_system_type *fs_type, > int flags, const char *dev_name, void *data, struct vfsmount *mnt) > { > int error; > struct pts_mount_opts opts; > + struct super_block *s; > > memset(&opts, 0, sizeof(opts)); > if (data) { > @@ -415,23 +366,37 @@ static int devpts_get_sb(struct file_system_type *fs_type, > } > > if (opts.newinstance) > - error = new_pts_mount(fs_type, flags, data, &opts, mnt); > + s = sget(fs_type, NULL, set_anon_super, NULL); > else > - error = init_pts_mount(fs_type, flags, data, &opts, mnt); > + s = sget(fs_type, compare_init_pts_sb, set_anon_super, NULL); > > - if (error) > - return error; > + if (IS_ERR(s)) > + return PTR_ERR(s); > > - error = mknod_ptmx(mnt->mnt_sb); > + if (!s->s_root) { > + s->s_flags = flags; > + error = devpts_fill_super(s, data, flags & MS_SILENT ? 1 : 0); > + if (error) > + goto out_undo_sget; > + s->s_flags |= MS_ACTIVE; > + } > + > + simple_set_mnt(mnt, s); > + > + memcpy(&(DEVPTS_SB(s))->mount_opts, &opts, sizeof(opts)); > + > + error = mknod_ptmx(s); > if (error) > goto out_dput; > > return 0; > > out_dput: > - dput(mnt->mnt_sb->s_root); > - up_write(&mnt->mnt_sb->s_umount); > - deactivate_super(mnt->mnt_sb); > + dput(s->s_root); > + > +out_undo_sget: > + up_write(&s->s_umount); > + deactivate_super(s); > return error; > } > > -- > 1.5.2.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |