On Sat, Sep 22, 2012 at 10:06:39PM +0400, Cyrill Gorcunov wrote:
> On Thu, Sep 13, 2012 at 05:25:07PM +0100, Alan Cox wrote:
> > On Thu, 13 Sep 2012 16:54:01 +0400
> > Cyrill Gorcunov <gorcu...@openvz.org> wrote:
> >
> > > On Thu, Sep 13, 2012 at 01:51:31PM +0100, Alan Cox wrote:
> > > > > +static int pty_get_lock(struct tty_struct *tty, int __user *arg)
> > > > > +{
> > > > > + int locked = test_bit(TTY_PTY_LOCK, &tty->flags);
> > > > > + if (put_user(locked, arg))
> > > > > + return -EFAULT;
> > > >
> > > > Now explain exactly how this doesn't race with another thread chanigng
> > > > the lock setting ?
> > >
> > > It's the same as to set/clear this bit, isn't it? Please correct me
> > > if I'm wrong.
> >
> > So by the time you've finished the test bit and returned it to user space
> > the answer may have changed ?
> >
> > > > The other comment I have is that it might be better put these in now
> > > > there are sysfs patches for the tty layer bouncing about to provide the
> > > > needed infrastructure ?
> > >
> > > Alan, could you please point me where these patches are living, so I would
> > > take a look and check them out
> >
> > linux-serial or check tty-next as I think Greg has now merged the test
> > patch.
>
> Guys, you mean something like below? Look, I must admit I'm not really
> sure if I've done all locking right, and there is no need for additional
> kref counting on tty_struct. Could you please check if it looks more-less
> sane (I've tested it but still...)
>
> ---
> From: Cyrill Gorcunov <gorcu...@openvz.org>
> Subject: tty, pty: Add pty_state attribute to fetch tty flags
>
> For checkpoint/restore we need to know if tty has
> exclusive or packet mode set, as well as if pty
> is currently locked (just to be able to restore
> this characteristics).
>
> To serve this the pty_state attribute is introduced
> for pty devices. A typical output looks like
>
> | [root@neptune ~]# cat /sys/devices/virtual/tty/ptmp0/pty_state
> | locked: 0 exclusive: 0 packet: 0
>
> Signed-off-by: Cyrill Gorcunov <gorcu...@openvz.org>
> CC: Alan Cox <a...@lxorguk.ukuu.org.uk>
> CC: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> CC: "H. Peter Anvin" <h...@zytor.com>
> CC: Jiri Slaby <jsl...@suse.cz>
> CC: Pavel Emelyanov <xe...@parallels.com>
> ---
> drivers/tty/pty.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 44 insertions(+), 1 deletion(-)
>
> Index: tty.git/drivers/tty/pty.c
> ===================================================================
> --- tty.git.orig/drivers/tty/pty.c
> +++ tty.git/drivers/tty/pty.c
> @@ -283,6 +283,46 @@ done:
> return 0;
> }
>
> +static ssize_t pty_show_state(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct tty_struct *tty = dev_get_drvdata(dev);
> + int locked, exclusive, packet;
> +
> + tty_lock(tty);
> + locked = test_bit(TTY_PTY_LOCK, &tty->flags);
> + exclusive = test_bit(TTY_EXCLUSIVE, &tty->flags);
> + packet = tty->packet;
> + tty_unlock(tty);
> +
> + return snprintf(buf, PAGE_SIZE, "locked: %d exclusive: %d packet: %d\n",
> + locked, exclusive, packet);
> +}
Sysfs is one value per file, you have three values here, please make 3
files.
And document them in Documentation/ABI/.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
