public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>, linux-kernel@vger.kernel.org
Subject: devpts multiple instances feedback
Date: Sat, 3 Jan 2009 16:52:09 +0100	[thread overview]
Message-ID: <20090103155209.GA17988@lst.de> (raw)

I just took a look at the changes going into Linus current tree and
here's some feedback about the devpts multiple instances code:

 - the ptmx node is quite useful, I think it should always be around,
   even for normal devpts mounts.  That way distros can slowly migrate
   over to just using it by default and making the containers
   interaction easier.  It's also in many ways much nicer to have
   all the pty handling in one filesystems instead of sometimes
   using the character device.
 - the 000 mode is very weird, given how the /dev/ptmx operates
   it doesn't really make much sense to have it different than 0666
   unless you want to disable ptys.
 - why does pts_sb_from_inode have to check s_magic, I can't see
   it ever used on an inode not from the devpts filesystem
 - parsing the options twice is rather odd, I'd rather parse it into
   a once allocated structure then passed on through the private
   data void pointer into get_sb_nodev
 - creating the ptmx node should happen inside devfs_fill_super
 - once the ptmx mknod is gone I think new_pts_mount,
   is_new_instance_mount, init_pts_mount and maybe even get_init_pts_sb
   should be merged into devpts_get_sb to make the whole mounting
   scenario easier to follow instead of having to jump through half
   a dozen functions
 - I think CONFIG_DEVPTS_MULTIPLE_INSTANCES is not a good idea,
   it's not much code and could either be enabled unconditionally or
   based on the presence of a generic namespaces config option.
   (btw, this also applies to the other namespaces options, there's
   not much of a reason to have millions of options for them,
   one single option would be a lot easier for the user..)

             reply	other threads:[~2009-01-03 15:52 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-03 15:52 Christoph Hellwig [this message]
2009-01-03 16:15 ` devpts multiple instances feedback Christoph Hellwig
2009-01-26 21:53   ` Christoph Hellwig
2009-01-27  3:32     ` Sukadev Bhattiprolu
2009-01-05 21:09 ` Sukadev Bhattiprolu
2009-01-26 21:55   ` Christoph Hellwig
2009-01-26 21:58     ` Alan Cox
2009-02-01 16:29       ` Christoph Hellwig
2009-02-01 16:41         ` Alan Cox
2009-01-26 21:58     ` H. Peter Anvin
2009-02-01 16:31       ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090103155209.GA17988@lst.de \
    --to=hch@lst.de \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sukadev@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox