Re: [RFC] tty: Add get- ioctls to fetch tty status
On Thu, Sep 27, 2012 at 05:00:37PM +0100, Alan Cox wrote: > > > Otherwise - TICOGPKT is specific to pty devices. Please therefore put it > > > in the pty.c code and note that not all platforms use asm-geneic TTY > > > ioctls so check carefully they all build. > > > > I put it into tty_ioctl.c simply because TIOCPKT was there already so > > I thought it's good idea to keep them close to each other in code. > > Perhaps we should move TIOCPKT out as well? OK, so I'll do two patches instead of squashing everything on one patch. Btw, as far as I see wee are zeroifying tty_struct always twice, first in alloc_tty_struct and then in initialize_tty_struct again. And alloc_tty_struct + initialize_tty_struct always used as a tuple in current code, so I presume they might be squashed in one helper instead to eliminate overhead, but it's different task. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
> > Otherwise - TICOGPKT is specific to pty devices. Please therefore put it > > in the pty.c code and note that not all platforms use asm-geneic TTY > > ioctls so check carefully they all build. > > I put it into tty_ioctl.c simply because TIOCPKT was there already so > I thought it's good idea to keep them close to each other in code. Perhaps we should move TIOCPKT out as well ? -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Thu, Sep 27, 2012 at 04:13:12PM +0100, Alan Cox wrote:
> > Well, sure inside our tool before doing checkpoint we stop all
> > tasks which are part of dumpee process tree. This unfortunatly
> > doesn't apply to these ioctl calls. Peter, any idea how to deal
> > with that?
>
> I suspect you can't deal with that. Which isn't to say you can't still
> build something useful. The query interfaces are trivial enough and
> beyond that it's simply a case of documenting that they are not intended
> to be generically "safe" but provide a spot sample.
>
> Otherwise - TICOGPKT is specific to pty devices. Please therefore put it
> in the pty.c code and note that not all platforms use asm-geneic TTY
> ioctls so check carefully they all build.
Alan, I've moved the TICOGPKT to pty code but where to puts information
about unsafety of the data obtained? Also I fear I can test the build
results and run-time behaviour on x86 machine only, which I own (well,
couple of them). Though I defined this ioctls the same way as existing
ioctl codes are defined so I think all should build the same.
---
From: Cyrill Gorcunov
Subject: tty: Add get- ioctls to fetch tty status v2
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.
For this sake the following ioctl codes are introduced
- TIOCGPKT to get packet mode state
- TIOCGPTLCK to get Pty locked state
- TIOCGEXCL to get Exclusive mode state
v2:
- Use TIOC prefix for ioctl codes (by jslaby@)
- Move TIOCGPKT to pty code (by alan@)
Signed-off-by: Cyrill Gorcunov
---
drivers/tty/pty.c| 34 ++
drivers/tty/tty_io.c |7 +++
fs/compat_ioctl.c|3 +++
include/asm-generic/ioctls.h |3 +++
4 files changed, 47 insertions(+)
Index: tty.git/drivers/tty/pty.c
===
--- tty.git.orig/drivers/tty/pty.c
+++ tty.git/drivers/tty/pty.c
@@ -174,6 +174,32 @@ static int pty_set_lock(struct tty_struc
return 0;
}
+static int pty_get_lock(struct tty_struct *tty, int __user *arg)
+{
+ int locked = test_bit(TTY_PTY_LOCK, >flags);
+ if (put_user(locked, arg))
+ return -EFAULT;
+ return 0;
+}
+
+static int pty_get_pktmode(struct tty_struct *tty, int __user *arg)
+{
+ unsigned long flags;
+ int pktmode;
+
+ if (tty->driver->subtype != PTY_TYPE_MASTER)
+ return -ENOTTY;
+
+ spin_lock_irqsave(>ctrl_lock, flags);
+ pktmode = tty->packet;
+ spin_unlock_irqrestore(>ctrl_lock, flags);
+
+ if (put_user(pktmode, arg))
+ return -EFAULT;
+
+ return 0;
+}
+
/* Send a signal to the slave */
static int pty_signal(struct tty_struct *tty, int sig)
{
@@ -393,8 +419,12 @@ static int pty_bsd_ioctl(struct tty_stru
switch (cmd) {
case TIOCSPTLCK: /* Set PT Lock (disallow slave open) */
return pty_set_lock(tty, (int __user *) arg);
+ case TIOCGPTLCK: /* Get PT Lock status */
+ return pty_get_lock(tty, (int __user *)arg);
case TIOCSIG:/* Send signal to other side of pty */
return pty_signal(tty, (int) arg);
+ case TIOCGPKT: /* Get PT packet mode status */
+ return pty_get_pktmode(tty, (int __user *)arg);
}
return -ENOIOCTLCMD;
}
@@ -507,10 +537,14 @@ static int pty_unix98_ioctl(struct tty_s
switch (cmd) {
case TIOCSPTLCK: /* Set PT Lock (disallow slave open) */
return pty_set_lock(tty, (int __user *)arg);
+ case TIOCGPTLCK: /* Get PT Lock status */
+ return pty_get_lock(tty, (int __user *)arg);
case TIOCGPTN: /* Get PT Number */
return put_user(tty->index, (unsigned int __user *)arg);
case TIOCSIG:/* Send signal to other side of pty */
return pty_signal(tty, (int) arg);
+ case TIOCGPKT: /* Get PT packet mode status */
+ return pty_get_pktmode(tty, (int __user *)arg);
}
return -ENOIOCTLCMD;
Index: tty.git/drivers/tty/tty_io.c
===
--- tty.git.orig/drivers/tty/tty_io.c
+++ tty.git/drivers/tty/tty_io.c
@@ -2693,6 +2693,13 @@ long tty_ioctl(struct file *file, unsign
case TIOCNXCL:
clear_bit(TTY_EXCLUSIVE, >flags);
return 0;
+ case TIOCGEXCL:
+ {
+ int excl = test_bit(TTY_EXCLUSIVE, >flags);
+ if (put_user(excl, (int __user *)p))
+ return -EFAULT;
+ return 0;
+ }
case TIOCNOTTY:
if (current->signal->tty != tty)
return -ENOTTY;
Index: tty.git/fs/compat_ioctl.c
===
--- tty.git.orig/fs/compat_ioctl.c
+++
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Thu, Sep 27, 2012 at 04:13:12PM +0100, Alan Cox wrote: > > Well, sure inside our tool before doing checkpoint we stop all > > tasks which are part of dumpee process tree. This unfortunatly > > doesn't apply to these ioctl calls. Peter, any idea how to deal > > with that? > > I suspect you can't deal with that. Which isn't to say you can't still > build something useful. The query interfaces are trivial enough and > beyond that it's simply a case of documenting that they are not intended > to be generically "safe" but provide a spot sample. I see, thanks Alan! > Otherwise - TICOGPKT is specific to pty devices. Please therefore put it > in the pty.c code and note that not all platforms use asm-geneic TTY > ioctls so check carefully they all build. I put it into tty_ioctl.c simply because TIOCPKT was there already so I thought it's good idea to keep them close to each other in code. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
> Well, sure inside our tool before doing checkpoint we stop all > tasks which are part of dumpee process tree. This unfortunatly > doesn't apply to these ioctl calls. Peter, any idea how to deal > with that? I suspect you can't deal with that. Which isn't to say you can't still build something useful. The query interfaces are trivial enough and beyond that it's simply a case of documenting that they are not intended to be generically "safe" but provide a spot sample. Otherwise - TICOGPKT is specific to pty devices. Please therefore put it in the pty.c code and note that not all platforms use asm-geneic TTY ioctls so check carefully they all build. Alan -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On 09/27/2012 07:48 AM, Cyrill Gorcunov wrote: On Thu, Sep 27, 2012 at 07:43:44AM -0700, H. Peter Anvin wrote: If you can't guarantee that ALL those processes are stopped and checkpointed/restarted, you have a huge problem. Well, sure inside our tool before doing checkpoint we stop all tasks which are part of dumpee process tree. This unfortunatly doesn't apply to these ioctl calls. Peter, any idea how to deal with that? Sorry, no. I suspect you might be trying to do something that is impossible to do in a general fashion without application changes. I see. What if I wrap all this entries with CONFIG_CHECKPOINT_RESTORE and say add some sysctl which would enable/disbale this ioctls. Then it'll be up to user-space callers to make sure that data received is valid and can be used? The problem is that I need to fetch this bits somehow with minimal changes in kernel. I don't see how that solves anything. -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Thu, Sep 27, 2012 at 07:43:44AM -0700, H. Peter Anvin wrote: > >>If you can't guarantee that ALL those processes are stopped and > >>checkpointed/restarted, you have a huge problem. > > > >Well, sure inside our tool before doing checkpoint we stop all > >tasks which are part of dumpee process tree. This unfortunatly > >doesn't apply to these ioctl calls. Peter, any idea how to deal > >with that? > > Sorry, no. I suspect you might be trying to do something that is > impossible to do in a general fashion without application changes. I see. What if I wrap all this entries with CONFIG_CHECKPOINT_RESTORE and say add some sysctl which would enable/disbale this ioctls. Then it'll be up to user-space callers to make sure that data received is valid and can be used? The problem is that I need to fetch this bits somehow with minimal changes in kernel. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On 09/27/2012 07:21 AM, Cyrill Gorcunov wrote: On Thu, Sep 27, 2012 at 07:17:49AM -0700, H. Peter Anvin wrote: While we easily can fetch termios settings and such, there are few bits which are missed to expord. So this patch provides them to user-space. What bothers me (and the same applies to termios) is that you have NO idea if your particular process is the "owner" of that tty. tty users use out-of-band protocols, often implicit, to determine which process "owns" the tty state. If you can't guarantee that ALL those processes are stopped and checkpointed/restarted, you have a huge problem. Well, sure inside our tool before doing checkpoint we stop all tasks which are part of dumpee process tree. This unfortunatly doesn't apply to these ioctl calls. Peter, any idea how to deal with that? Sorry, no. I suspect you might be trying to do something that is impossible to do in a general fashion without application changes. -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Thu, Sep 27, 2012 at 07:17:49AM -0700, H. Peter Anvin wrote: > >While we easily can fetch termios settings and such, there are few bits which > >are missed to expord. So this patch provides them to user-space. > > > > What bothers me (and the same applies to termios) is that you have > NO idea if your particular process is the "owner" of that tty. tty > users use out-of-band protocols, often implicit, to determine which > process "owns" the tty state. > > If you can't guarantee that ALL those processes are stopped and > checkpointed/restarted, you have a huge problem. Well, sure inside our tool before doing checkpoint we stop all tasks which are part of dumpee process tree. This unfortunatly doesn't apply to these ioctl calls. Peter, any idea how to deal with that? Cyrill -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On 09/27/2012 07:14 AM, Cyrill Gorcunov wrote: On Thu, Sep 27, 2012 at 03:14:47PM +0100, Alan Cox wrote: Alan, Greg, what's opinion? This flags fetching is the same as say fetching of termios settings, once fetched they can be changed immediately, and it's up to caller what to do with termios settings. No? I think you need to explain what you expect to be doing with it, and why it is safe in that application. OK, it seems it was unclear from changelog. We need to know this parameters to be able to restore tty connection after checkpoint. While we easily can fetch termios settings and such, there are few bits which are missed to expord. So this patch provides them to user-space. What bothers me (and the same applies to termios) is that you have NO idea if your particular process is the "owner" of that tty. tty users use out-of-band protocols, often implicit, to determine which process "owns" the tty state. If you can't guarantee that ALL those processes are stopped and checkpointed/restarted, you have a huge problem. -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Thu, Sep 27, 2012 at 03:14:47PM +0100, Alan Cox wrote: > > Alan, Greg, what's opinion? This flags fetching is the same as say fetching > > of termios settings, once fetched they can be changed immediately, and it's > > up to caller what to do with termios settings. No? > > I think you need to explain what you expect to be doing with it, and why > it is safe in that application. OK, it seems it was unclear from changelog. We need to know this parameters to be able to restore tty connection after checkpoint. While we easily can fetch termios settings and such, there are few bits which are missed to expord. So this patch provides them to user-space. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
> Alan, Greg, what's opinion? This flags fetching is the same as say fetching > of termios settings, once fetched they can be changed immediately, and it's > up to caller what to do with termios settings. No? I think you need to explain what you expect to be doing with it, and why it is safe in that application. Alan -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Mon, Sep 24, 2012 at 06:14:41PM +0400, Cyrill Gorcunov wrote:
>
> As to Alan's point on "what's the use of this if it can instantly change
> after you read the value" I guess it's the same as what we have when we
> simply set the value. Imagine we have two tasks fork'ed, first task do
> lock the pty while another task does unlock it immediately after that,
> as far as I see there is nothing preventing user space to do that, thus
> first task will think it has locked the peer while in real the peer remains
> unlocked. Same here with reading this value -- it's valid once read but
> can be changed right after. Isn't it?
Alan, Greg, what's opinion? This flags fetching is the same as say fetching
of termios settings, once fetched they can be changed immediately, and it's
up to caller what to do with termios settings. No?
---
From: Cyrill Gorcunov
Subject: tty: Add get- ioctls to fetch tty status
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.
For this sake the following ioctl codes are introduced
- TIOCGPKT to get packet mode state
- TIOCGPTLCK to get Pty locked state
- TIOCGEXCL to get Exclusive mode state
Signed-off-by: Cyrill Gorcunov
---
drivers/tty/pty.c| 12
drivers/tty/tty_io.c |7 +++
drivers/tty/tty_ioctl.c | 16
fs/compat_ioctl.c|3 +++
include/asm-generic/ioctls.h |3 +++
5 files changed, 41 insertions(+)
Index: tty.git/drivers/tty/pty.c
===
--- tty.git.orig/drivers/tty/pty.c
+++ tty.git/drivers/tty/pty.c
@@ -174,6 +174,14 @@ static int pty_set_lock(struct tty_struc
return 0;
}
+static int pty_get_lock(struct tty_struct *tty, int __user *arg)
+{
+ int locked = test_bit(TTY_PTY_LOCK, >flags);
+ if (put_user(locked, arg))
+ return -EFAULT;
+ return 0;
+}
+
/* Send a signal to the slave */
static int pty_signal(struct tty_struct *tty, int sig)
{
@@ -393,6 +401,8 @@ static int pty_bsd_ioctl(struct tty_stru
switch (cmd) {
case TIOCSPTLCK: /* Set PT Lock (disallow slave open) */
return pty_set_lock(tty, (int __user *) arg);
+ case TIOCGPTLCK:
+ return pty_get_lock(tty, (int __user *) arg);
case TIOCSIG:/* Send signal to other side of pty */
return pty_signal(tty, (int) arg);
}
@@ -507,6 +517,8 @@ static int pty_unix98_ioctl(struct tty_s
switch (cmd) {
case TIOCSPTLCK: /* Set PT Lock (disallow slave open) */
return pty_set_lock(tty, (int __user *)arg);
+ case TIOCGPTLCK:
+ return pty_get_lock(tty, (int __user *) arg);
case TIOCGPTN: /* Get PT Number */
return put_user(tty->index, (unsigned int __user *)arg);
case TIOCSIG:/* Send signal to other side of pty */
Index: tty.git/drivers/tty/tty_io.c
===
--- tty.git.orig/drivers/tty/tty_io.c
+++ tty.git/drivers/tty/tty_io.c
@@ -2693,6 +2693,13 @@ long tty_ioctl(struct file *file, unsign
case TIOCNXCL:
clear_bit(TTY_EXCLUSIVE, >flags);
return 0;
+ case TIOCGEXCL:
+ {
+ int excl = test_bit(TTY_EXCLUSIVE, >flags);
+ if (put_user(excl, (int __user *)p))
+ return -EFAULT;
+ return 0;
+ }
case TIOCNOTTY:
if (current->signal->tty != tty)
return -ENOTTY;
Index: tty.git/drivers/tty/tty_ioctl.c
===
--- tty.git.orig/drivers/tty/tty_ioctl.c
+++ tty.git/drivers/tty/tty_ioctl.c
@@ -1173,6 +1173,22 @@ int n_tty_ioctl_helper(struct tty_struct
spin_unlock_irqrestore(>ctrl_lock, flags);
return 0;
}
+ case TIOCGPKT:
+ {
+ int pktmode;
+
+ if (tty->driver->type != TTY_DRIVER_TYPE_PTY ||
+ tty->driver->subtype != PTY_TYPE_MASTER)
+ return -ENOTTY;
+
+ spin_lock_irqsave(>ctrl_lock, flags);
+ pktmode = tty->packet;
+ spin_unlock_irqrestore(>ctrl_lock, flags);
+
+ if (put_user(pktmode, (int __user *) arg))
+ return -EFAULT;
+ return 0;
+ }
default:
/* Try the mode commands */
return tty_mode_ioctl(tty, file, cmd, arg);
Index: tty.git/fs/compat_ioctl.c
===
--- tty.git.orig/fs/compat_ioctl.c
+++ tty.git/fs/compat_ioctl.c
@@ -842,6 +842,9 @@ COMPATIBLE_IOCTL(TIOCGDEV)
COMPATIBLE_IOCTL(TIOCCBRK)
COMPATIBLE_IOCTL(TIOCGSID)
COMPATIBLE_IOCTL(TIOCGICOUNT)
+COMPATIBLE_IOCTL(TIOCGPKT)
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Mon, Sep 24, 2012 at 06:14:41PM +0400, Cyrill Gorcunov wrote:
As to Alan's point on what's the use of this if it can instantly change
after you read the value I guess it's the same as what we have when we
simply set the value. Imagine we have two tasks fork'ed, first task do
lock the pty while another task does unlock it immediately after that,
as far as I see there is nothing preventing user space to do that, thus
first task will think it has locked the peer while in real the peer remains
unlocked. Same here with reading this value -- it's valid once read but
can be changed right after. Isn't it?
Alan, Greg, what's opinion? This flags fetching is the same as say fetching
of termios settings, once fetched they can be changed immediately, and it's
up to caller what to do with termios settings. No?
---
From: Cyrill Gorcunov gorcu...@openvz.org
Subject: tty: Add get- ioctls to fetch tty status
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.
For this sake the following ioctl codes are introduced
- TIOCGPKT to get packet mode state
- TIOCGPTLCK to get Pty locked state
- TIOCGEXCL to get Exclusive mode state
Signed-off-by: Cyrill Gorcunov gorcu...@openvz.org
---
drivers/tty/pty.c| 12
drivers/tty/tty_io.c |7 +++
drivers/tty/tty_ioctl.c | 16
fs/compat_ioctl.c|3 +++
include/asm-generic/ioctls.h |3 +++
5 files changed, 41 insertions(+)
Index: tty.git/drivers/tty/pty.c
===
--- tty.git.orig/drivers/tty/pty.c
+++ tty.git/drivers/tty/pty.c
@@ -174,6 +174,14 @@ static int pty_set_lock(struct tty_struc
return 0;
}
+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;
+ return 0;
+}
+
/* Send a signal to the slave */
static int pty_signal(struct tty_struct *tty, int sig)
{
@@ -393,6 +401,8 @@ static int pty_bsd_ioctl(struct tty_stru
switch (cmd) {
case TIOCSPTLCK: /* Set PT Lock (disallow slave open) */
return pty_set_lock(tty, (int __user *) arg);
+ case TIOCGPTLCK:
+ return pty_get_lock(tty, (int __user *) arg);
case TIOCSIG:/* Send signal to other side of pty */
return pty_signal(tty, (int) arg);
}
@@ -507,6 +517,8 @@ static int pty_unix98_ioctl(struct tty_s
switch (cmd) {
case TIOCSPTLCK: /* Set PT Lock (disallow slave open) */
return pty_set_lock(tty, (int __user *)arg);
+ case TIOCGPTLCK:
+ return pty_get_lock(tty, (int __user *) arg);
case TIOCGPTN: /* Get PT Number */
return put_user(tty-index, (unsigned int __user *)arg);
case TIOCSIG:/* Send signal to other side of pty */
Index: tty.git/drivers/tty/tty_io.c
===
--- tty.git.orig/drivers/tty/tty_io.c
+++ tty.git/drivers/tty/tty_io.c
@@ -2693,6 +2693,13 @@ long tty_ioctl(struct file *file, unsign
case TIOCNXCL:
clear_bit(TTY_EXCLUSIVE, tty-flags);
return 0;
+ case TIOCGEXCL:
+ {
+ int excl = test_bit(TTY_EXCLUSIVE, tty-flags);
+ if (put_user(excl, (int __user *)p))
+ return -EFAULT;
+ return 0;
+ }
case TIOCNOTTY:
if (current-signal-tty != tty)
return -ENOTTY;
Index: tty.git/drivers/tty/tty_ioctl.c
===
--- tty.git.orig/drivers/tty/tty_ioctl.c
+++ tty.git/drivers/tty/tty_ioctl.c
@@ -1173,6 +1173,22 @@ int n_tty_ioctl_helper(struct tty_struct
spin_unlock_irqrestore(tty-ctrl_lock, flags);
return 0;
}
+ case TIOCGPKT:
+ {
+ int pktmode;
+
+ if (tty-driver-type != TTY_DRIVER_TYPE_PTY ||
+ tty-driver-subtype != PTY_TYPE_MASTER)
+ return -ENOTTY;
+
+ spin_lock_irqsave(tty-ctrl_lock, flags);
+ pktmode = tty-packet;
+ spin_unlock_irqrestore(tty-ctrl_lock, flags);
+
+ if (put_user(pktmode, (int __user *) arg))
+ return -EFAULT;
+ return 0;
+ }
default:
/* Try the mode commands */
return tty_mode_ioctl(tty, file, cmd, arg);
Index: tty.git/fs/compat_ioctl.c
===
--- tty.git.orig/fs/compat_ioctl.c
+++ tty.git/fs/compat_ioctl.c
@@ -842,6 +842,9 @@ COMPATIBLE_IOCTL(TIOCGDEV)
COMPATIBLE_IOCTL(TIOCCBRK)
COMPATIBLE_IOCTL(TIOCGSID)
Re: [RFC] tty: Add get- ioctls to fetch tty status
Alan, Greg, what's opinion? This flags fetching is the same as say fetching of termios settings, once fetched they can be changed immediately, and it's up to caller what to do with termios settings. No? I think you need to explain what you expect to be doing with it, and why it is safe in that application. Alan -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Thu, Sep 27, 2012 at 03:14:47PM +0100, Alan Cox wrote: Alan, Greg, what's opinion? This flags fetching is the same as say fetching of termios settings, once fetched they can be changed immediately, and it's up to caller what to do with termios settings. No? I think you need to explain what you expect to be doing with it, and why it is safe in that application. OK, it seems it was unclear from changelog. We need to know this parameters to be able to restore tty connection after checkpoint. While we easily can fetch termios settings and such, there are few bits which are missed to expord. So this patch provides them to user-space. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On 09/27/2012 07:14 AM, Cyrill Gorcunov wrote: On Thu, Sep 27, 2012 at 03:14:47PM +0100, Alan Cox wrote: Alan, Greg, what's opinion? This flags fetching is the same as say fetching of termios settings, once fetched they can be changed immediately, and it's up to caller what to do with termios settings. No? I think you need to explain what you expect to be doing with it, and why it is safe in that application. OK, it seems it was unclear from changelog. We need to know this parameters to be able to restore tty connection after checkpoint. While we easily can fetch termios settings and such, there are few bits which are missed to expord. So this patch provides them to user-space. What bothers me (and the same applies to termios) is that you have NO idea if your particular process is the owner of that tty. tty users use out-of-band protocols, often implicit, to determine which process owns the tty state. If you can't guarantee that ALL those processes are stopped and checkpointed/restarted, you have a huge problem. -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Thu, Sep 27, 2012 at 07:17:49AM -0700, H. Peter Anvin wrote: While we easily can fetch termios settings and such, there are few bits which are missed to expord. So this patch provides them to user-space. What bothers me (and the same applies to termios) is that you have NO idea if your particular process is the owner of that tty. tty users use out-of-band protocols, often implicit, to determine which process owns the tty state. If you can't guarantee that ALL those processes are stopped and checkpointed/restarted, you have a huge problem. Well, sure inside our tool before doing checkpoint we stop all tasks which are part of dumpee process tree. This unfortunatly doesn't apply to these ioctl calls. Peter, any idea how to deal with that? Cyrill -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On 09/27/2012 07:21 AM, Cyrill Gorcunov wrote: On Thu, Sep 27, 2012 at 07:17:49AM -0700, H. Peter Anvin wrote: While we easily can fetch termios settings and such, there are few bits which are missed to expord. So this patch provides them to user-space. What bothers me (and the same applies to termios) is that you have NO idea if your particular process is the owner of that tty. tty users use out-of-band protocols, often implicit, to determine which process owns the tty state. If you can't guarantee that ALL those processes are stopped and checkpointed/restarted, you have a huge problem. Well, sure inside our tool before doing checkpoint we stop all tasks which are part of dumpee process tree. This unfortunatly doesn't apply to these ioctl calls. Peter, any idea how to deal with that? Sorry, no. I suspect you might be trying to do something that is impossible to do in a general fashion without application changes. -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Thu, Sep 27, 2012 at 07:43:44AM -0700, H. Peter Anvin wrote: If you can't guarantee that ALL those processes are stopped and checkpointed/restarted, you have a huge problem. Well, sure inside our tool before doing checkpoint we stop all tasks which are part of dumpee process tree. This unfortunatly doesn't apply to these ioctl calls. Peter, any idea how to deal with that? Sorry, no. I suspect you might be trying to do something that is impossible to do in a general fashion without application changes. I see. What if I wrap all this entries with CONFIG_CHECKPOINT_RESTORE and say add some sysctl which would enable/disbale this ioctls. Then it'll be up to user-space callers to make sure that data received is valid and can be used? The problem is that I need to fetch this bits somehow with minimal changes in kernel. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On 09/27/2012 07:48 AM, Cyrill Gorcunov wrote: On Thu, Sep 27, 2012 at 07:43:44AM -0700, H. Peter Anvin wrote: If you can't guarantee that ALL those processes are stopped and checkpointed/restarted, you have a huge problem. Well, sure inside our tool before doing checkpoint we stop all tasks which are part of dumpee process tree. This unfortunatly doesn't apply to these ioctl calls. Peter, any idea how to deal with that? Sorry, no. I suspect you might be trying to do something that is impossible to do in a general fashion without application changes. I see. What if I wrap all this entries with CONFIG_CHECKPOINT_RESTORE and say add some sysctl which would enable/disbale this ioctls. Then it'll be up to user-space callers to make sure that data received is valid and can be used? The problem is that I need to fetch this bits somehow with minimal changes in kernel. I don't see how that solves anything. -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
Well, sure inside our tool before doing checkpoint we stop all tasks which are part of dumpee process tree. This unfortunatly doesn't apply to these ioctl calls. Peter, any idea how to deal with that? I suspect you can't deal with that. Which isn't to say you can't still build something useful. The query interfaces are trivial enough and beyond that it's simply a case of documenting that they are not intended to be generically safe but provide a spot sample. Otherwise - TICOGPKT is specific to pty devices. Please therefore put it in the pty.c code and note that not all platforms use asm-geneic TTY ioctls so check carefully they all build. Alan -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Thu, Sep 27, 2012 at 04:13:12PM +0100, Alan Cox wrote: Well, sure inside our tool before doing checkpoint we stop all tasks which are part of dumpee process tree. This unfortunatly doesn't apply to these ioctl calls. Peter, any idea how to deal with that? I suspect you can't deal with that. Which isn't to say you can't still build something useful. The query interfaces are trivial enough and beyond that it's simply a case of documenting that they are not intended to be generically safe but provide a spot sample. I see, thanks Alan! Otherwise - TICOGPKT is specific to pty devices. Please therefore put it in the pty.c code and note that not all platforms use asm-geneic TTY ioctls so check carefully they all build. I put it into tty_ioctl.c simply because TIOCPKT was there already so I thought it's good idea to keep them close to each other in code. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Thu, Sep 27, 2012 at 04:13:12PM +0100, Alan Cox wrote:
Well, sure inside our tool before doing checkpoint we stop all
tasks which are part of dumpee process tree. This unfortunatly
doesn't apply to these ioctl calls. Peter, any idea how to deal
with that?
I suspect you can't deal with that. Which isn't to say you can't still
build something useful. The query interfaces are trivial enough and
beyond that it's simply a case of documenting that they are not intended
to be generically safe but provide a spot sample.
Otherwise - TICOGPKT is specific to pty devices. Please therefore put it
in the pty.c code and note that not all platforms use asm-geneic TTY
ioctls so check carefully they all build.
Alan, I've moved the TICOGPKT to pty code but where to puts information
about unsafety of the data obtained? Also I fear I can test the build
results and run-time behaviour on x86 machine only, which I own (well,
couple of them). Though I defined this ioctls the same way as existing
ioctl codes are defined so I think all should build the same.
---
From: Cyrill Gorcunov gorcu...@openvz.org
Subject: tty: Add get- ioctls to fetch tty status v2
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.
For this sake the following ioctl codes are introduced
- TIOCGPKT to get packet mode state
- TIOCGPTLCK to get Pty locked state
- TIOCGEXCL to get Exclusive mode state
v2:
- Use TIOC prefix for ioctl codes (by jslaby@)
- Move TIOCGPKT to pty code (by alan@)
Signed-off-by: Cyrill Gorcunov gorcu...@openvz.org
---
drivers/tty/pty.c| 34 ++
drivers/tty/tty_io.c |7 +++
fs/compat_ioctl.c|3 +++
include/asm-generic/ioctls.h |3 +++
4 files changed, 47 insertions(+)
Index: tty.git/drivers/tty/pty.c
===
--- tty.git.orig/drivers/tty/pty.c
+++ tty.git/drivers/tty/pty.c
@@ -174,6 +174,32 @@ static int pty_set_lock(struct tty_struc
return 0;
}
+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;
+ return 0;
+}
+
+static int pty_get_pktmode(struct tty_struct *tty, int __user *arg)
+{
+ unsigned long flags;
+ int pktmode;
+
+ if (tty-driver-subtype != PTY_TYPE_MASTER)
+ return -ENOTTY;
+
+ spin_lock_irqsave(tty-ctrl_lock, flags);
+ pktmode = tty-packet;
+ spin_unlock_irqrestore(tty-ctrl_lock, flags);
+
+ if (put_user(pktmode, arg))
+ return -EFAULT;
+
+ return 0;
+}
+
/* Send a signal to the slave */
static int pty_signal(struct tty_struct *tty, int sig)
{
@@ -393,8 +419,12 @@ static int pty_bsd_ioctl(struct tty_stru
switch (cmd) {
case TIOCSPTLCK: /* Set PT Lock (disallow slave open) */
return pty_set_lock(tty, (int __user *) arg);
+ case TIOCGPTLCK: /* Get PT Lock status */
+ return pty_get_lock(tty, (int __user *)arg);
case TIOCSIG:/* Send signal to other side of pty */
return pty_signal(tty, (int) arg);
+ case TIOCGPKT: /* Get PT packet mode status */
+ return pty_get_pktmode(tty, (int __user *)arg);
}
return -ENOIOCTLCMD;
}
@@ -507,10 +537,14 @@ static int pty_unix98_ioctl(struct tty_s
switch (cmd) {
case TIOCSPTLCK: /* Set PT Lock (disallow slave open) */
return pty_set_lock(tty, (int __user *)arg);
+ case TIOCGPTLCK: /* Get PT Lock status */
+ return pty_get_lock(tty, (int __user *)arg);
case TIOCGPTN: /* Get PT Number */
return put_user(tty-index, (unsigned int __user *)arg);
case TIOCSIG:/* Send signal to other side of pty */
return pty_signal(tty, (int) arg);
+ case TIOCGPKT: /* Get PT packet mode status */
+ return pty_get_pktmode(tty, (int __user *)arg);
}
return -ENOIOCTLCMD;
Index: tty.git/drivers/tty/tty_io.c
===
--- tty.git.orig/drivers/tty/tty_io.c
+++ tty.git/drivers/tty/tty_io.c
@@ -2693,6 +2693,13 @@ long tty_ioctl(struct file *file, unsign
case TIOCNXCL:
clear_bit(TTY_EXCLUSIVE, tty-flags);
return 0;
+ case TIOCGEXCL:
+ {
+ int excl = test_bit(TTY_EXCLUSIVE, tty-flags);
+ if (put_user(excl, (int __user *)p))
+ return -EFAULT;
+ return 0;
+ }
case TIOCNOTTY:
if (current-signal-tty != tty)
return -ENOTTY;
Index: tty.git/fs/compat_ioctl.c
===
---
Re: [RFC] tty: Add get- ioctls to fetch tty status
Otherwise - TICOGPKT is specific to pty devices. Please therefore put it in the pty.c code and note that not all platforms use asm-geneic TTY ioctls so check carefully they all build. I put it into tty_ioctl.c simply because TIOCPKT was there already so I thought it's good idea to keep them close to each other in code. Perhaps we should move TIOCPKT out as well ? -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Thu, Sep 27, 2012 at 05:00:37PM +0100, Alan Cox wrote: Otherwise - TICOGPKT is specific to pty devices. Please therefore put it in the pty.c code and note that not all platforms use asm-geneic TTY ioctls so check carefully they all build. I put it into tty_ioctl.c simply because TIOCPKT was there already so I thought it's good idea to keep them close to each other in code. Perhaps we should move TIOCPKT out as well? OK, so I'll do two patches instead of squashing everything on one patch. Btw, as far as I see wee are zeroifying tty_struct always twice, first in alloc_tty_struct and then in initialize_tty_struct again. And alloc_tty_struct + initialize_tty_struct always used as a tuple in current code, so I presume they might be squashed in one helper instead to eliminate overhead, but it's different task. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Sun, Sep 23, 2012 at 07:46:24AM -0700, H. Peter Anvin wrote: > Cyrill Gorcunov wrote: > > >On Sat, Sep 22, 2012 at 06:09:53PM -0700, Greg Kroah-Hartman wrote: > >> On Sun, Sep 23, 2012 at 01:52:32AM +0400, Cyrill Gorcunov wrote: > >> > On Sun, Sep 23, 2012 at 12:11:44AM +0400, Cyrill Gorcunov wrote: > >> > > > >> > > > Sysfs is one value per file, you have three values here, please > >make 3 > >> > > > files. > >> > > > > >> > > > And document them in Documentation/ABI/. > >> > > > >> > > Hmm, sure Greg, I'll update. Thanks! > >> > > >> > Something like below I suppose? Look, if there will be no complains > >> > on tech aspects on the patch (locking and tty refs) -- I'll update > >> > Documentation. Just want be sure I've made no mistakes here. > >> > > >> > Another question -- would not it be worth to wrap code with > >> > CONFIG_CHECKPOINT_RESTORE? > >> > >> Alan's point stands, what's the use of this if it can instantly > >> change after you read the value? > > > >We use it when we do a checkpoint, ie when tasks are stopped. I think > >it's close to data obtained from procfs (ie valid once you read it but can > >be changed right after that operation). Maybe I should put everything to > >procfs, or stick back with ioctl calls? > > The problem as I see it is that you don't know if your process is the lock > holder. Guys, after being trying sysfs approach I think ioctls is a bit better, because it requires only local changes not propagated to devpts, also this calls are allowed via file descriptor owner only. Actually I thought what if extend existing set-(lock,packet-mode,exclusive) calls to return previous state of appropriate variable, but this will break abi so i had to refuse this idea. As to Alan's point on "what's the use of this if it can instantly change after you read the value" I guess it's the same as what we have when we simply set the value. Imagine we have two tasks fork'ed, first task do lock the pty while another task does unlock it immediately after that, as far as I see there is nothing preventing user space to do that, thus first task will think it has locked the peer while in real the peer remains unlocked. Same here with reading this value -- it's valid once read but can be changed right after. Isn't it? -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Sun, Sep 23, 2012 at 07:46:24AM -0700, H. Peter Anvin wrote: Cyrill Gorcunov gorcu...@openvz.org wrote: On Sat, Sep 22, 2012 at 06:09:53PM -0700, Greg Kroah-Hartman wrote: On Sun, Sep 23, 2012 at 01:52:32AM +0400, Cyrill Gorcunov wrote: On Sun, Sep 23, 2012 at 12:11:44AM +0400, Cyrill Gorcunov wrote: Sysfs is one value per file, you have three values here, please make 3 files. And document them in Documentation/ABI/. Hmm, sure Greg, I'll update. Thanks! Something like below I suppose? Look, if there will be no complains on tech aspects on the patch (locking and tty refs) -- I'll update Documentation. Just want be sure I've made no mistakes here. Another question -- would not it be worth to wrap code with CONFIG_CHECKPOINT_RESTORE? Alan's point stands, what's the use of this if it can instantly change after you read the value? We use it when we do a checkpoint, ie when tasks are stopped. I think it's close to data obtained from procfs (ie valid once you read it but can be changed right after that operation). Maybe I should put everything to procfs, or stick back with ioctl calls? The problem as I see it is that you don't know if your process is the lock holder. Guys, after being trying sysfs approach I think ioctls is a bit better, because it requires only local changes not propagated to devpts, also this calls are allowed via file descriptor owner only. Actually I thought what if extend existing set-(lock,packet-mode,exclusive) calls to return previous state of appropriate variable, but this will break abi so i had to refuse this idea. As to Alan's point on what's the use of this if it can instantly change after you read the value I guess it's the same as what we have when we simply set the value. Imagine we have two tasks fork'ed, first task do lock the pty while another task does unlock it immediately after that, as far as I see there is nothing preventing user space to do that, thus first task will think it has locked the peer while in real the peer remains unlocked. Same here with reading this value -- it's valid once read but can be changed right after. Isn't it? -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Sun, Sep 23, 2012 at 07:44:54AM -0700, H. Peter Anvin wrote: > > Funny... I gave you that feedback the last go around. I'm sorry, Peter, I guess I simply missed it. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Sun, Sep 23, 2012 at 07:46:24AM -0700, H. Peter Anvin wrote: > >changed right after that operation). Maybe I should put everything to > >procfs, or stick back with ioctl calls? > > The problem as I see it is that you don't know if your process is > the lock holder. Wait, Peter, this lock is set via ioctl call on the fd obtained from open(ptmx) call, so i fail to see how one can setup such lock without having fd on the hands. When we do checkpoint the processes we expect the user passes us the pid of process which is the leader not some subtree (we can deal with process subtrees too but it is not encouraged ;) -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
Cyrill Gorcunov wrote: >On Sat, Sep 22, 2012 at 06:09:53PM -0700, Greg Kroah-Hartman wrote: >> On Sun, Sep 23, 2012 at 01:52:32AM +0400, Cyrill Gorcunov wrote: >> > On Sun, Sep 23, 2012 at 12:11:44AM +0400, Cyrill Gorcunov wrote: >> > > >> > > > Sysfs is one value per file, you have three values here, please >make 3 >> > > > files. >> > > > >> > > > And document them in Documentation/ABI/. >> > > >> > > Hmm, sure Greg, I'll update. Thanks! >> > >> > Something like below I suppose? Look, if there will be no complains >> > on tech aspects on the patch (locking and tty refs) -- I'll update >> > Documentation. Just want be sure I've made no mistakes here. >> > >> > Another question -- would not it be worth to wrap code with >> > CONFIG_CHECKPOINT_RESTORE? >> >> Alan's point stands, what's the use of this if it can instantly >change >> after you read the value? > >We use it when we do a checkpoint, ie when tasks are stopped. I think >it's >close to data obtained from procfs (ie valid once you read it but can >be >changed right after that operation). Maybe I should put everything to >procfs, or stick back with ioctl calls? The problem as I see it is that you don't know if your process is the lock holder. -- Sent from my mobile phone. Please excuse brevity and lack of formatting. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
Cyrill Gorcunov wrote: >On Sat, Sep 22, 2012 at 06:43:37PM -0700, Eric W. Biederman wrote: >> >> I don't know where this conversation comes from but putting ptys in >> sysfs in combination with the newinstance mount option is completely >> broken unless the device name and device number duplication is >handled, >> which I don't see here. > >Good point, Eric, thanks! I'll take a look on this aspect (I must admit >I've missed this). Funny... I gave you that feedback the last go around. -- Sent from my mobile phone. Please excuse brevity and lack of formatting. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Sat, Sep 22, 2012 at 06:43:37PM -0700, Eric W. Biederman wrote: > > I don't know where this conversation comes from but putting ptys in > sysfs in combination with the newinstance mount option is completely > broken unless the device name and device number duplication is handled, > which I don't see here. Good point, Eric, thanks! I'll take a look on this aspect (I must admit I've missed this). -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Sat, Sep 22, 2012 at 06:09:53PM -0700, Greg Kroah-Hartman wrote: > On Sun, Sep 23, 2012 at 01:52:32AM +0400, Cyrill Gorcunov wrote: > > On Sun, Sep 23, 2012 at 12:11:44AM +0400, Cyrill Gorcunov wrote: > > > > > > > Sysfs is one value per file, you have three values here, please make 3 > > > > files. > > > > > > > > And document them in Documentation/ABI/. > > > > > > Hmm, sure Greg, I'll update. Thanks! > > > > Something like below I suppose? Look, if there will be no complains > > on tech aspects on the patch (locking and tty refs) -- I'll update > > Documentation. Just want be sure I've made no mistakes here. > > > > Another question -- would not it be worth to wrap code with > > CONFIG_CHECKPOINT_RESTORE? > > Alan's point stands, what's the use of this if it can instantly change > after you read the value? We use it when we do a checkpoint, ie when tasks are stopped. I think it's close to data obtained from procfs (ie valid once you read it but can be changed right after that operation). Maybe I should put everything to procfs, or stick back with ioctl calls? -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Sat, Sep 22, 2012 at 06:09:53PM -0700, Greg Kroah-Hartman wrote: On Sun, Sep 23, 2012 at 01:52:32AM +0400, Cyrill Gorcunov wrote: On Sun, Sep 23, 2012 at 12:11:44AM +0400, Cyrill Gorcunov wrote: Sysfs is one value per file, you have three values here, please make 3 files. And document them in Documentation/ABI/. Hmm, sure Greg, I'll update. Thanks! Something like below I suppose? Look, if there will be no complains on tech aspects on the patch (locking and tty refs) -- I'll update Documentation. Just want be sure I've made no mistakes here. Another question -- would not it be worth to wrap code with CONFIG_CHECKPOINT_RESTORE? Alan's point stands, what's the use of this if it can instantly change after you read the value? We use it when we do a checkpoint, ie when tasks are stopped. I think it's close to data obtained from procfs (ie valid once you read it but can be changed right after that operation). Maybe I should put everything to procfs, or stick back with ioctl calls? -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Sat, Sep 22, 2012 at 06:43:37PM -0700, Eric W. Biederman wrote: I don't know where this conversation comes from but putting ptys in sysfs in combination with the newinstance mount option is completely broken unless the device name and device number duplication is handled, which I don't see here. Good point, Eric, thanks! I'll take a look on this aspect (I must admit I've missed this). -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
Cyrill Gorcunov gorcu...@openvz.org wrote: On Sat, Sep 22, 2012 at 06:43:37PM -0700, Eric W. Biederman wrote: I don't know where this conversation comes from but putting ptys in sysfs in combination with the newinstance mount option is completely broken unless the device name and device number duplication is handled, which I don't see here. Good point, Eric, thanks! I'll take a look on this aspect (I must admit I've missed this). Funny... I gave you that feedback the last go around. -- Sent from my mobile phone. Please excuse brevity and lack of formatting. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
Cyrill Gorcunov gorcu...@openvz.org wrote: On Sat, Sep 22, 2012 at 06:09:53PM -0700, Greg Kroah-Hartman wrote: On Sun, Sep 23, 2012 at 01:52:32AM +0400, Cyrill Gorcunov wrote: On Sun, Sep 23, 2012 at 12:11:44AM +0400, Cyrill Gorcunov wrote: Sysfs is one value per file, you have three values here, please make 3 files. And document them in Documentation/ABI/. Hmm, sure Greg, I'll update. Thanks! Something like below I suppose? Look, if there will be no complains on tech aspects on the patch (locking and tty refs) -- I'll update Documentation. Just want be sure I've made no mistakes here. Another question -- would not it be worth to wrap code with CONFIG_CHECKPOINT_RESTORE? Alan's point stands, what's the use of this if it can instantly change after you read the value? We use it when we do a checkpoint, ie when tasks are stopped. I think it's close to data obtained from procfs (ie valid once you read it but can be changed right after that operation). Maybe I should put everything to procfs, or stick back with ioctl calls? The problem as I see it is that you don't know if your process is the lock holder. -- Sent from my mobile phone. Please excuse brevity and lack of formatting. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Sun, Sep 23, 2012 at 07:46:24AM -0700, H. Peter Anvin wrote: changed right after that operation). Maybe I should put everything to procfs, or stick back with ioctl calls? The problem as I see it is that you don't know if your process is the lock holder. Wait, Peter, this lock is set via ioctl call on the fd obtained from open(ptmx) call, so i fail to see how one can setup such lock without having fd on the hands. When we do checkpoint the processes we expect the user passes us the pid of process which is the leader not some subtree (we can deal with process subtrees too but it is not encouraged ;) -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Sun, Sep 23, 2012 at 07:44:54AM -0700, H. Peter Anvin wrote: Funny... I gave you that feedback the last go around. I'm sorry, Peter, I guess I simply missed it. -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
I don't know where this conversation comes from but putting ptys in
sysfs in combination with the newinstance mount option is completely
broken unless the device name and device number duplication is handled,
which I don't see here.
Cyrill Gorcunov writes:
>
> 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
> 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
> CC: Alan Cox
> CC: Greg Kroah-Hartman
> CC: "H. Peter Anvin"
> CC: Jiri Slaby
> CC: Pavel Emelyanov
> ---
> 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, >flags);
> + exclusive = test_bit(TTY_EXCLUSIVE, >flags);
> + packet = tty->packet;
> + tty_unlock(tty);
> +
> + return snprintf(buf, PAGE_SIZE, "locked: %d exclusive: %d packet: %d\n",
> + locked, exclusive, packet);
> +}
> +
> +static DEVICE_ATTR(pty_state, S_IRUSR | S_IRGRP, pty_show_state, NULL);
> +
> +static struct attribute *pty_state_dev_attrs[] = {
> + _attr_pty_state.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group pty_dev_attr_group = {
> + .attrs = pty_state_dev_attrs,
> +};
> +
> +static const struct attribute_group *pty_dev_attr_groups[] = {
> + _dev_attr_group,
> + NULL,
> +};
> +
> +static int pty_register_attr(struct tty_driver *driver, struct tty_struct
> *tty)
> +{
> + struct device *dev = tty_register_device_attr(driver, tty->index,
> tty->dev,
> + (void *)tty,
> pty_dev_attr_groups);
> + return PTR_RET(dev);
> +}
> +
> /**
> * pty_common_install - set up the pty pair
> * @driver: the pty driver
> @@ -351,7 +391,9 @@ static int pty_common_install(struct tty
>
> tty_driver_kref_get(driver);
> tty->count++;
> - return 0;
> +
> + return pty_register_attr(driver, tty);
> +
> err_free_termios:
> if (legacy)
> tty_free_termios(tty);
> @@ -368,6 +410,7 @@ err:
>
> static void pty_cleanup(struct tty_struct *tty)
> {
> + tty_unregister_device(tty->driver, tty->index);
> kfree(tty->port);
> }
>
> --
> 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/
--
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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Sun, Sep 23, 2012 at 01:52:32AM +0400, Cyrill Gorcunov wrote: > On Sun, Sep 23, 2012 at 12:11:44AM +0400, Cyrill Gorcunov wrote: > > > > > Sysfs is one value per file, you have three values here, please make 3 > > > files. > > > > > > And document them in Documentation/ABI/. > > > > Hmm, sure Greg, I'll update. Thanks! > > Something like below I suppose? Look, if there will be no complains > on tech aspects on the patch (locking and tty refs) -- I'll update > Documentation. Just want be sure I've made no mistakes here. > > Another question -- would not it be worth to wrap code with > CONFIG_CHECKPOINT_RESTORE? Alan's point stands, what's the use of this if it can instantly change after you read the value? 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
> > 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...) This still doesn't answer the question about 'why is this actually useful'. It can change as or after you query it so the value is useless. Alan -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Sun, Sep 23, 2012 at 12:11:44AM +0400, Cyrill Gorcunov wrote:
>
> > Sysfs is one value per file, you have three values here, please make 3
> > files.
> >
> > And document them in Documentation/ABI/.
>
> Hmm, sure Greg, I'll update. Thanks!
Something like below I suppose? Look, if there will be no complains
on tech aspects on the patch (locking and tty refs) -- I'll update
Documentation. Just want be sure I've made no mistakes here.
Another question -- would not it be worth to wrap code with
CONFIG_CHECKPOINT_RESTORE?
---
From: Cyrill Gorcunov
Subject: tty, pty: Add sysfs attributes for pty device to fetch flags
For checkpoint/restore we need to know if pty 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 following attrubutes: lock, exclusive, packet,
are introduced for pty devices.
For example
| cat /sys/devices/virtual/tty/ptmp1/lock
| 1
| cat /sys/devices/virtual/tty/ptmp1/exclusive
| 0
| cat /sys/devices/virtual/tty/ptmp1/packet
| 0
Signed-off-by: Cyrill Gorcunov
CC: Alan Cox
CC: Greg Kroah-Hartman
CC: "H. Peter Anvin"
CC: Jiri Slaby
CC: Pavel Emelyanov
---
drivers/tty/pty.c | 71 +-
1 file changed, 70 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,72 @@ done:
return 0;
}
+static ssize_t pty_show_lock(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct tty_struct *tty = dev_get_drvdata(dev);
+ int lock;
+
+ tty_lock(tty);
+ lock = test_bit(TTY_PTY_LOCK, >flags);
+ tty_unlock(tty);
+
+ return snprintf(buf, PAGE_SIZE, "%d\n", lock);
+}
+
+static ssize_t pty_show_exclusive(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct tty_struct *tty = dev_get_drvdata(dev);
+ int exclusive;
+
+ tty_lock(tty);
+ exclusive = test_bit(TTY_EXCLUSIVE, >flags);
+ tty_unlock(tty);
+
+ return snprintf(buf, PAGE_SIZE, "%d\n", exclusive);
+}
+
+static ssize_t pty_show_packet(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct tty_struct *tty = dev_get_drvdata(dev);
+ int packet;
+
+ tty_lock(tty);
+ packet = tty->packet;
+ tty_unlock(tty);
+
+ return snprintf(buf, PAGE_SIZE, "%d\n", packet);
+}
+
+static DEVICE_ATTR(lock, S_IRUSR | S_IRGRP, pty_show_lock, NULL);
+static DEVICE_ATTR(exclusive, S_IRUSR | S_IRGRP, pty_show_exclusive, NULL);
+static DEVICE_ATTR(packet, S_IRUSR | S_IRGRP, pty_show_packet, NULL);
+
+static struct attribute *pty_attrs[] = {
+ _attr_lock.attr,
+ _attr_exclusive.attr,
+ _attr_packet.attr,
+ NULL,
+};
+
+static const struct attribute_group pty_attrs_group = {
+ .attrs = pty_attrs,
+};
+
+static const struct attribute_group *pty_attr_groups[] = {
+ _attrs_group,
+ NULL,
+};
+
+static int pty_register_attr(struct tty_driver *driver, struct tty_struct *tty)
+{
+ struct device *dev = tty_register_device_attr(driver, tty->index,
tty->dev,
+ (void *)tty,
pty_attr_groups);
+ return PTR_RET(dev);
+}
+
/**
* pty_common_install - set up the pty pair
* @driver: the pty driver
@@ -351,7 +417,9 @@ static int pty_common_install(struct tty
tty_driver_kref_get(driver);
tty->count++;
- return 0;
+
+ return pty_register_attr(driver, tty);
+
err_free_termios:
if (legacy)
tty_free_termios(tty);
@@ -368,6 +436,7 @@ err:
static void pty_cleanup(struct tty_struct *tty)
{
+ tty_unregister_device(tty->driver, tty->index);
kfree(tty->port);
}
--
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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Sat, Sep 22, 2012 at 01:07:31PM -0700, Greg Kroah-Hartman wrote:
> > 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, >flags);
> > + exclusive = test_bit(TTY_EXCLUSIVE, >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/.
Hmm, sure Greg, I'll update. Thanks!
--
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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
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 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, >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
> 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
> CC: Alan Cox
> CC: Greg Kroah-Hartman
> CC: "H. Peter Anvin"
> CC: Jiri Slaby
> CC: Pavel Emelyanov
> ---
> 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, >flags);
> + exclusive = test_bit(TTY_EXCLUSIVE, >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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Thu, Sep 13, 2012 at 05:25:07PM +0100, Alan Cox wrote:
> On Thu, 13 Sep 2012 16:54:01 +0400
> Cyrill Gorcunov 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, >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
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
CC: Alan Cox
CC: Greg Kroah-Hartman
CC: "H. Peter Anvin"
CC: Jiri Slaby
CC: Pavel Emelyanov
---
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, >flags);
+ exclusive = test_bit(TTY_EXCLUSIVE, >flags);
+ packet = tty->packet;
+ tty_unlock(tty);
+
+ return snprintf(buf, PAGE_SIZE, "locked: %d exclusive: %d packet: %d\n",
+ locked, exclusive, packet);
+}
+
+static DEVICE_ATTR(pty_state, S_IRUSR | S_IRGRP, pty_show_state, NULL);
+
+static struct attribute *pty_state_dev_attrs[] = {
+ _attr_pty_state.attr,
+ NULL,
+};
+
+static const struct attribute_group pty_dev_attr_group = {
+ .attrs = pty_state_dev_attrs,
+};
+
+static const struct attribute_group *pty_dev_attr_groups[] = {
+ _dev_attr_group,
+ NULL,
+};
+
+static int pty_register_attr(struct tty_driver *driver, struct tty_struct *tty)
+{
+ struct device *dev = tty_register_device_attr(driver, tty->index,
tty->dev,
+ (void *)tty,
pty_dev_attr_groups);
+ return PTR_RET(dev);
+}
+
/**
* pty_common_install - set up the pty pair
* @driver: the pty driver
@@ -351,7 +391,9 @@ static int pty_common_install(struct tty
tty_driver_kref_get(driver);
tty->count++;
- return 0;
+
+ return pty_register_attr(driver, tty);
+
err_free_termios:
if (legacy)
tty_free_termios(tty);
@@ -368,6 +410,7 @@ err:
static void pty_cleanup(struct tty_struct *tty)
{
+ tty_unregister_device(tty->driver, tty->index);
kfree(tty->port);
}
--
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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
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);
+}
+
+static DEVICE_ATTR(pty_state, S_IRUSR | S_IRGRP, pty_show_state, NULL);
+
+static struct attribute *pty_state_dev_attrs[] = {
+ dev_attr_pty_state.attr,
+ NULL,
+};
+
+static const struct attribute_group pty_dev_attr_group = {
+ .attrs = pty_state_dev_attrs,
+};
+
+static const struct attribute_group *pty_dev_attr_groups[] = {
+ pty_dev_attr_group,
+ NULL,
+};
+
+static int pty_register_attr(struct tty_driver *driver, struct tty_struct *tty)
+{
+ struct device *dev = tty_register_device_attr(driver, tty-index,
tty-dev,
+ (void *)tty,
pty_dev_attr_groups);
+ return PTR_RET(dev);
+}
+
/**
* pty_common_install - set up the pty pair
* @driver: the pty driver
@@ -351,7 +391,9 @@ static int pty_common_install(struct tty
tty_driver_kref_get(driver);
tty-count++;
- return 0;
+
+ return pty_register_attr(driver, tty);
+
err_free_termios:
if (legacy)
tty_free_termios(tty);
@@ -368,6 +410,7 @@ err:
static void pty_cleanup(struct tty_struct *tty)
{
+ tty_unregister_device(tty-driver, tty-index);
kfree(tty-port);
}
--
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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Sat, Sep 22, 2012 at 01:07:31PM -0700, Greg Kroah-Hartman wrote:
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/.
Hmm, sure Greg, I'll update. Thanks!
--
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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Sun, Sep 23, 2012 at 12:11:44AM +0400, Cyrill Gorcunov wrote:
Sysfs is one value per file, you have three values here, please make 3
files.
And document them in Documentation/ABI/.
Hmm, sure Greg, I'll update. Thanks!
Something like below I suppose? Look, if there will be no complains
on tech aspects on the patch (locking and tty refs) -- I'll update
Documentation. Just want be sure I've made no mistakes here.
Another question -- would not it be worth to wrap code with
CONFIG_CHECKPOINT_RESTORE?
---
From: Cyrill Gorcunov gorcu...@openvz.org
Subject: tty, pty: Add sysfs attributes for pty device to fetch flags
For checkpoint/restore we need to know if pty 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 following attrubutes: lock, exclusive, packet,
are introduced for pty devices.
For example
| cat /sys/devices/virtual/tty/ptmp1/lock
| 1
| cat /sys/devices/virtual/tty/ptmp1/exclusive
| 0
| cat /sys/devices/virtual/tty/ptmp1/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 | 71 +-
1 file changed, 70 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,72 @@ done:
return 0;
}
+static ssize_t pty_show_lock(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct tty_struct *tty = dev_get_drvdata(dev);
+ int lock;
+
+ tty_lock(tty);
+ lock = test_bit(TTY_PTY_LOCK, tty-flags);
+ tty_unlock(tty);
+
+ return snprintf(buf, PAGE_SIZE, %d\n, lock);
+}
+
+static ssize_t pty_show_exclusive(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct tty_struct *tty = dev_get_drvdata(dev);
+ int exclusive;
+
+ tty_lock(tty);
+ exclusive = test_bit(TTY_EXCLUSIVE, tty-flags);
+ tty_unlock(tty);
+
+ return snprintf(buf, PAGE_SIZE, %d\n, exclusive);
+}
+
+static ssize_t pty_show_packet(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct tty_struct *tty = dev_get_drvdata(dev);
+ int packet;
+
+ tty_lock(tty);
+ packet = tty-packet;
+ tty_unlock(tty);
+
+ return snprintf(buf, PAGE_SIZE, %d\n, packet);
+}
+
+static DEVICE_ATTR(lock, S_IRUSR | S_IRGRP, pty_show_lock, NULL);
+static DEVICE_ATTR(exclusive, S_IRUSR | S_IRGRP, pty_show_exclusive, NULL);
+static DEVICE_ATTR(packet, S_IRUSR | S_IRGRP, pty_show_packet, NULL);
+
+static struct attribute *pty_attrs[] = {
+ dev_attr_lock.attr,
+ dev_attr_exclusive.attr,
+ dev_attr_packet.attr,
+ NULL,
+};
+
+static const struct attribute_group pty_attrs_group = {
+ .attrs = pty_attrs,
+};
+
+static const struct attribute_group *pty_attr_groups[] = {
+ pty_attrs_group,
+ NULL,
+};
+
+static int pty_register_attr(struct tty_driver *driver, struct tty_struct *tty)
+{
+ struct device *dev = tty_register_device_attr(driver, tty-index,
tty-dev,
+ (void *)tty,
pty_attr_groups);
+ return PTR_RET(dev);
+}
+
/**
* pty_common_install - set up the pty pair
* @driver: the pty driver
@@ -351,7 +417,9 @@ static int pty_common_install(struct tty
tty_driver_kref_get(driver);
tty-count++;
- return 0;
+
+ return pty_register_attr(driver, tty);
+
err_free_termios:
if (legacy)
tty_free_termios(tty);
@@ -368,6 +436,7 @@ err:
static void pty_cleanup(struct tty_struct *tty)
{
+ tty_unregister_device(tty-driver, tty-index);
kfree(tty-port);
}
--
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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
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...) This still doesn't answer the question about 'why is this actually useful'. It can change as or after you query it so the value is useless. Alan -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Sun, Sep 23, 2012 at 01:52:32AM +0400, Cyrill Gorcunov wrote: On Sun, Sep 23, 2012 at 12:11:44AM +0400, Cyrill Gorcunov wrote: Sysfs is one value per file, you have three values here, please make 3 files. And document them in Documentation/ABI/. Hmm, sure Greg, I'll update. Thanks! Something like below I suppose? Look, if there will be no complains on tech aspects on the patch (locking and tty refs) -- I'll update Documentation. Just want be sure I've made no mistakes here. Another question -- would not it be worth to wrap code with CONFIG_CHECKPOINT_RESTORE? Alan's point stands, what's the use of this if it can instantly change after you read the value? 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
I don't know where this conversation comes from but putting ptys in
sysfs in combination with the newinstance mount option is completely
broken unless the device name and device number duplication is handled,
which I don't see here.
Cyrill Gorcunov gorcu...@openvz.org writes:
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);
+}
+
+static DEVICE_ATTR(pty_state, S_IRUSR | S_IRGRP, pty_show_state, NULL);
+
+static struct attribute *pty_state_dev_attrs[] = {
+ dev_attr_pty_state.attr,
+ NULL,
+};
+
+static const struct attribute_group pty_dev_attr_group = {
+ .attrs = pty_state_dev_attrs,
+};
+
+static const struct attribute_group *pty_dev_attr_groups[] = {
+ pty_dev_attr_group,
+ NULL,
+};
+
+static int pty_register_attr(struct tty_driver *driver, struct tty_struct
*tty)
+{
+ struct device *dev = tty_register_device_attr(driver, tty-index,
tty-dev,
+ (void *)tty,
pty_dev_attr_groups);
+ return PTR_RET(dev);
+}
+
/**
* pty_common_install - set up the pty pair
* @driver: the pty driver
@@ -351,7 +391,9 @@ static int pty_common_install(struct tty
tty_driver_kref_get(driver);
tty-count++;
- return 0;
+
+ return pty_register_attr(driver, tty);
+
err_free_termios:
if (legacy)
tty_free_termios(tty);
@@ -368,6 +410,7 @@ err:
static void pty_cleanup(struct tty_struct *tty)
{
+ tty_unregister_device(tty-driver, tty-index);
kfree(tty-port);
}
--
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/
--
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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Thu, Sep 13, 2012 at 05:25:07PM +0100, Alan Cox wrote:
> On Thu, 13 Sep 2012 16:54:01 +0400
> Cyrill Gorcunov 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, >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?
Yes. Btw, as far as I see the same applies to lock bit on its own. One
process may lock it while another process may unlock it right after that.
> > > 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.
Ah, thanks a lot, Alan, I'll take a look!
Cyrill
--
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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Thu, 13 Sep 2012 16:54:01 +0400
Cyrill Gorcunov 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, >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.
--
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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
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, >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.
> 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.
Cyrill
--
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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
> +static int pty_get_lock(struct tty_struct *tty, int __user *arg)
> +{
> + int locked = test_bit(TTY_PTY_LOCK, >flags);
> + if (put_user(locked, arg))
> + return -EFAULT;
Now explain exactly how this doesn't race with another thread chanigng
the lock setting ?
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
--
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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Thu, Sep 13, 2012 at 01:46:53PM +0200, Jiri Slaby wrote: > On 09/13/2012 11:56 AM, Cyrill Gorcunov wrote: > > 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. > > > > For this sake the following ioctl codes are introduced > > > > - TIOGPKT to get packet mode state > > - TIOGPTLCK to get Pty locked state > > - TIOGEXCL to get Exclusive mode state > > Hi! I have only a comment regarding the naming. Why not to follow TIOC*? Hi Jiri, yeah, seems to be a good idea. I'll update if there will be no objection about idea in general. Thanks! Cyrill -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On 09/13/2012 11:56 AM, Cyrill Gorcunov wrote: > 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. > > For this sake the following ioctl codes are introduced > > - TIOGPKT to get packet mode state > - TIOGPTLCK to get Pty locked state > - TIOGEXCL to get Exclusive mode state Hi! I have only a comment regarding the naming. Why not to follow TIOC*? thanks, -- js suse labs -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On 09/13/2012 11:56 AM, Cyrill Gorcunov wrote: 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. For this sake the following ioctl codes are introduced - TIOGPKT to get packet mode state - TIOGPTLCK to get Pty locked state - TIOGEXCL to get Exclusive mode state Hi! I have only a comment regarding the naming. Why not to follow TIOC*? thanks, -- js suse labs -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
On Thu, Sep 13, 2012 at 01:46:53PM +0200, Jiri Slaby wrote: On 09/13/2012 11:56 AM, Cyrill Gorcunov wrote: 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. For this sake the following ioctl codes are introduced - TIOGPKT to get packet mode state - TIOGPTLCK to get Pty locked state - TIOGEXCL to get Exclusive mode state Hi! I have only a comment regarding the naming. Why not to follow TIOC*? Hi Jiri, yeah, seems to be a good idea. I'll update if there will be no objection about idea in general. Thanks! Cyrill -- 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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
+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 ?
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
--
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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
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.
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.
Cyrill
--
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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
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.
--
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/
Re: [RFC] tty: Add get- ioctls to fetch tty status
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?
Yes. Btw, as far as I see the same applies to lock bit on its own. One
process may lock it while another process may unlock it right after that.
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.
Ah, thanks a lot, Alan, I'll take a look!
Cyrill
--
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/
