From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755381AbZAZVxs (ORCPT ); Mon, 26 Jan 2009 16:53:48 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754858AbZAZVxa (ORCPT ); Mon, 26 Jan 2009 16:53:30 -0500 Received: from verein.lst.de ([213.95.11.210]:55061 "EHLO verein.lst.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752905AbZAZVx2 (ORCPT ); Mon, 26 Jan 2009 16:53:28 -0500 Date: Mon, 26 Jan 2009 22:53:21 +0100 From: Christoph Hellwig To: Sukadev Bhattiprolu Cc: "H. Peter Anvin" , linux-kernel@vger.kernel.org Subject: Re: devpts multiple instances feedback Message-ID: <20090126215321.GA20726@lst.de> References: <20090103155209.GA17988@lst.de> <20090103161544.GA18976@lst.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20090103161544.GA18976@lst.de> User-Agent: Mutt/1.3.28i X-Spam-Score: 0 () Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Jan 03, 2009 at 05:15:44PM +0100, Christoph Hellwig wrote: > This is a little untested patch to massage the mount code into about > how it should look like: ping? > > Index: linux-2.6/fs/devpts/inode.c > =================================================================== > --- linux-2.6.orig/fs/devpts/inode.c 2009-01-03 16:52:22.823672077 +0100 > +++ linux-2.6/fs/devpts/inode.c 2009-01-03 17:14:24.153711598 +0100 > @@ -329,106 +329,6 @@ static int compare_init_pts_sb(struct su > } > > /* > - * Safely parse the mount options in @data and update @opts. > - * > - * devpts ends up parsing options two times during mount, due to the > - * two modes of operation it supports. The first parse occurs in > - * devpts_get_sb() when determining the mode (single-instance or > - * multi-instance mode). The second parse happens in devpts_remount() > - * or new_pts_mount() depending on the mode. > - * > - * Parsing of options modifies the @data making subsequent parsing > - * incorrect. So make a local copy of @data and parse it. > - * > - * Return: 0 On success, -errno on error > - */ > -static int safe_parse_mount_options(void *data, struct pts_mount_opts *opts) > -{ > - int rc; > - void *datacp; > - > - if (!data) > - return 0; > - > - /* Use kstrdup() ? */ > - datacp = kmalloc(PAGE_SIZE, GFP_KERNEL); > - if (!datacp) > - return -ENOMEM; > - > - memcpy(datacp, data, PAGE_SIZE); > - rc = parse_mount_options((char *)datacp, PARSE_MOUNT, opts); > - kfree(datacp); > - > - return rc; > -} > - > -/* > - * 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 vfsmount *mnt) > -{ > - int err; > - struct pts_fs_info *fsi; > - struct pts_mount_opts *opts; > - > - 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); > - opts = &fsi->mount_opts; > - > - err = parse_mount_options(data, PARSE_MOUNT, opts); > - if (err) > - goto fail; > - > - err = mknod_ptmx(mnt->mnt_sb); > - if (err) > - goto fail; > - > - return 0; > - > -fail: > - dput(mnt->mnt_sb->s_root); > - deactivate_super(mnt->mnt_sb); > - return err; > -} > - > -/* > - * Check if 'newinstance' mount option was specified in @data. > - * > - * Return: -errno on error (eg: invalid mount options specified) > - * : 1 if 'newinstance' mount option was specified > - * : 0 if 'newinstance' mount option was NOT specified > - */ > -static int is_new_instance_mount(void *data) > -{ > - int rc; > - struct pts_mount_opts opts; > - > - if (!data) > - return 0; > - > - rc = safe_parse_mount_options(data, &opts); > - if (!rc) > - rc = opts.newinstance; > - > - return rc; > -} > - > -/* > - * get_init_pts_sb() > - * > - * 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(). > - * > * Mounts with 'newinstance' option create a new private namespace. > * > * But for single-mount semantics, devpts cannot use get_sb_single(), > @@ -436,20 +336,43 @@ static int is_new_instance_mount(void *d > * 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 get_init_pts_sb(struct file_system_type *fs_type, int flags, > - void *data, struct vfsmount *mnt) > +static int devpts_get_sb(struct file_system_type *fs_type, int flags, > + const char *dev_name, void *data, struct vfsmount *mnt) > { > + struct pts_mount_opts opts = { 0, }; > struct super_block *s; > int error; > > - s = sget(fs_type, compare_init_pts_sb, set_anon_super, NULL); > - if (IS_ERR(s)) > - return PTR_ERR(s); > + if (data) { > + error = parse_mount_options(data, PARSE_MOUNT, &opts); > + if (error) > + return error; > + } > + > + if (opts.newinstance) { > + /* > + * Mount a new (private) instance of devpts. PTYs created > + * in this instance are independent of the PTYs in other devpts > + * instances. > + */ > + > + printk(KERN_NOTICE "devpts: creating new instance\n"); > + > + s = sget(fs_type, NULL, set_anon_super, NULL); > + if (IS_ERR(s)) > + return PTR_ERR(s); > + } else { > + /* > + * 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. > + */ > + 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; > @@ -461,46 +384,26 @@ static int get_init_pts_sb(struct file_s > } > s->s_flags |= MS_ACTIVE; > } > - do_remount_sb(s, flags, data, 0); > - return simple_set_mnt(mnt, s); > -} > - > -/* > - * 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. > - */ > -static int init_pts_mount(struct file_system_type *fs_type, int flags, > - void *data, struct vfsmount *mnt) > -{ > - int err; > - > - err = get_init_pts_sb(fs_type, flags, data, mnt); > - if (err) > - return err; > - > - err = mknod_ptmx(mnt->mnt_sb); > - if (err) { > - dput(mnt->mnt_sb->s_root); > - deactivate_super(mnt->mnt_sb); > - } > - > - return err; > -} > - > -static int devpts_get_sb(struct file_system_type *fs_type, > - int flags, const char *dev_name, void *data, struct vfsmount *mnt) > -{ > - int new; > > - new = is_new_instance_mount(data); > - if (new < 0) > - return new; > + /* > + * Copy over mount options structure into the superblock. > + */ > + memcpy(&DEVPTS_SB(mnt->mnt_sb)->mount_opts, &opts, sizeof(opts)); > + > + error = simple_set_mnt(mnt, s); > + if (error) > + return error; > + > + error = mknod_ptmx(mnt->mnt_sb); > + if (error) > + goto out_dput; > > - if (new) > - return new_pts_mount(fs_type, flags, data, mnt); > + return 0; > > - return init_pts_mount(fs_type, flags, data, mnt); > + out_dput: > + dput(mnt->mnt_sb->s_root); > + deactivate_super(mnt->mnt_sb); > + return error; > } > #else > /* ---end quoted text---