tty idle time and hooking inode_ops from a chardev

tty idle time and hooking inode_ops from a chardev

[Phillip Susi]

I finally have spent some time figuring out an ancient bug that has 
bothered me on and off over the years: why the terminal idle time that 
who reports is sometimes very wrong.  It seems that this feature relies 
on the atime of the tty dev node being updated by the kernel whenever 
input is entered ( or more specifically, when it is read ).  Some 
programs, notably emacs and less, open /dev/tty rather than use stdin, 
and as a result, the atime of that inode is updated instead of the 
specific tty emacs is attached to.  This also means that if you mknod 
another inode to reach that tty ( say, for a chroot ), the times won't 
be updated correctly either.

To solve this, I think that a timestamp needs to be added to the struct 
tty_struct, and this should be updated whenever there is input, rather 
than updating the inode when the input is actually read.  In order to 
maintain compatibility with user space however, I would like to hook the 
stat calls and update the inode's atime from tty_struct whenever user 
space checks it.

To do this, I think that tty_open would need to replace the 
inode_operations pointer to point to its own version that has a set of 
functions that forward to the original inode_operations, but the 
iop->getattr would update the inode's atime from the tty_struct.  This 
means it needs to be able to locate both the tty_struct and the original 
inode_operations given the inode.  Is the i_private member of struct 
inode available for the tty code to use for this?

Alternatively, does anyone have a better idea to accomplish this?

Perhaps instead, struct cdev could gain a function pointer to allow it 
to hook vfs_stat?  I'm pretty sure there is a way to find the tty_struct 
given the cdev ( through dev_t? ), but I'm not sure what it is.

Re: tty idle time and hooking inode_ops from a chardev

[Greg KH]

On Fri, Dec 16, 2011 at 11:52:29AM -0500, Phillip Susi wrote:
> I finally have spent some time figuring out an ancient bug that has
> bothered me on and off over the years: why the terminal idle time
> that who reports is sometimes very wrong.  It seems that this
> feature relies on the atime of the tty dev node being updated by the
> kernel whenever input is entered ( or more specifically, when it is
> read ).  Some programs, notably emacs and less, open /dev/tty rather
> than use stdin, and as a result, the atime of that inode is updated
> instead of the specific tty emacs is attached to.  This also means
> that if you mknod another inode to reach that tty ( say, for a
> chroot ), the times won't be updated correctly either.
> 
> To solve this, I think that a timestamp needs to be added to the
> struct tty_struct, and this should be updated whenever there is
> input, rather than updating the inode when the input is actually
> read.  In order to maintain compatibility with user space however, I
> would like to hook the stat calls and update the inode's atime from
> tty_struct whenever user space checks it.
> 
> To do this, I think that tty_open would need to replace the
> inode_operations pointer to point to its own version that has a set
> of functions that forward to the original inode_operations, but the
> iop->getattr would update the inode's atime from the tty_struct.
> This means it needs to be able to locate both the tty_struct and the
> original inode_operations given the inode.  Is the i_private member
> of struct inode available for the tty code to use for this?

No, I do not think it is.

> Alternatively, does anyone have a better idea to accomplish this?

Don't worry about it, as it's not really an important issue at all? :)

It seems that your userspace programs aren't properly measuring the
correct thing here, it's not that the kernel is doing something wrong,
right?

And atime on a character node is pretty undefined, isn't it?

> Perhaps instead, struct cdev could gain a function pointer to allow
> it to hook vfs_stat?  I'm pretty sure there is a way to find the
> tty_struct given the cdev ( through dev_t? ), but I'm not sure what
> it is.

No, I don't want to touch cdev in this way, sorry.

greg k-h

Re: tty idle time and hooking inode_ops from a chardev

[Phillip Susi]

On 12/16/2011 12:57 PM, Greg KH wrote:
> Don't worry about it, as it's not really an important issue at all? :)

That's why it has been broken for years; because nobody cares enough 
about it to fix it, but that kind of thing irks me.

> It seems that your userspace programs aren't properly measuring the
> correct thing here, it's not that the kernel is doing something wrong,
> right?

I thought so at first, but it appears that the tty layer was originally 
written specifically to work this way.

> And atime on a character node is pretty undefined, isn't it?

Is it?  I would have thought so but after looking at all of this code, 
it looks like it is one of those things that has just been around for 
decades, but nobody knows about, kind of like FIONREAD.

I wonder if that's how who has always worked going back to AT&T Unix. 
If it is, then I'd like to stick with it ( but fix it ) rather than say, 
add an ioctl to read the idle time and change coreutils to use that instead.

Re: tty idle time and hooking inode_ops from a chardev

[Greg KH]

On Fri, Dec 16, 2011 at 01:22:02PM -0500, Phillip Susi wrote:
> On 12/16/2011 12:57 PM, Greg KH wrote:
> >Don't worry about it, as it's not really an important issue at all? :)
> 
> That's why it has been broken for years; because nobody cares enough
> about it to fix it, but that kind of thing irks me.
> 
> >It seems that your userspace programs aren't properly measuring the
> >correct thing here, it's not that the kernel is doing something wrong,
> >right?
> 
> I thought so at first, but it appears that the tty layer was
> originally written specifically to work this way.

But you said that your userspace programs are opening the wrong tty
device for what you are trying to look at, right?

> >And atime on a character node is pretty undefined, isn't it?
> 
> Is it?  I would have thought so but after looking at all of this
> code, it looks like it is one of those things that has just been
> around for decades, but nobody knows about, kind of like FIONREAD.
> 
> I wonder if that's how who has always worked going back to AT&T
> Unix. If it is, then I'd like to stick with it ( but fix it ) rather
> than say, add an ioctl to read the idle time and change coreutils to
> use that instead.

That's not "fixing" it at all, adding an ioctl is the same as adding a
new system call, do you really think that is ok here?

As you are opening the tty node once, that's when atime is set, right?
The fact that you keep it open still keeps the atime to the original
open time, you aren't supposed to check for every single read/write of
the node once it was opened.

But to be sure, what does POSIX say about this?

thanks,

greg k-h

Re: tty idle time and hooking inode_ops from a chardev

[Phillip Susi]

On 12/16/2011 1:36 PM, Greg KH wrote:
> But you said that your userspace programs are opening the wrong tty
> device for what you are trying to look at, right?

They are opening /dev/tty, which is supposed to be an alias for the 
process's controlling tty, but it isn't quite so.  It routes read/write 
calls to the correct tty, but is a separate inode, so has its own 
timestamps.

> That's not "fixing" it at all, adding an ioctl is the same as adding a
> new system call, do you really think that is ok here?

I would prefer not to, which is why I'm trying to figure out how to make 
the atime correct no matter how you opened the tty.

> As you are opening the tty node once, that's when atime is set, right?
> The fact that you keep it open still keeps the atime to the original
> open time, you aren't supposed to check for every single read/write of
> the node once it was opened.

tty_io.c updates the atime of the inode on every successful read(), and 
the mtime on every successful write().  The problem is that several 
different inodes can all point to the same tty, so which inode gets 
updated depends on which process is doing the IO.

> But to be sure, what does POSIX say about this?

No clue.

Re: tty idle time and hooking inode_ops from a chardev

[Phillip Susi]

On 12/16/2011 1:36 PM, Greg KH wrote:
> As you are opening the tty node once, that's when atime is set, right?
> The fact that you keep it open still keeps the atime to the original
> open time, you aren't supposed to check for every single read/write of
> the node once it was opened.
>
> But to be sure, what does POSIX say about this?

Actually it looks like POSIX 4.8 "File Times Update" requires the 
timestamps to be updated on every read/write, which is probably why 
Linux does that.

So the problem is just the /dev/tty alias having a different inode than 
the actual controlling tty.  It also makes the output of lsof less 
useful, so maybe the real fix is to somehow make opening /dev/tty 
actually open the correct tty inode and dentry.

I have a feeling that will require changing /dev/tty from being a 
devnode to a symlink to something in /dev/pts.