Re: [PATCH] Revert "kernel/printk: add kmsg SEEK_CUR handling"

[Bruno Meneguele]

On Sun, Jun 21, 2020 at 08:50:09PM -0700, Linus Torvalds wrote:
> On Sun, Jun 21, 2020 at 8:02 PM Jason A. Donenfeld <Jason@xxxxxxxxx> wrote:
> >
> > This reverts commit 8ece3b3eb576a78d2e67ad4c3a80a39fa6708809.
> >
> > This commit broke userspace. Bash uses ESPIPE to determine whether or
> > not the file should be read using "unbuffered I/O", which means reading
> > 1 byte at a time instead of 128 bytes at a time.
> 
> Ack. Somewhat odd behavior, but clearly user space depended on the
> legacy "return EINVAL rather than ESPIPE" behavior.
> 
> I do think there are other reasons to not return ESPIPE. The fact is,
> the printk buffer _is_ seekable, it's just relative seeking that
> doesn't work. You can seek to the beginning and the end and a
> particular offset, just not relative.
> 
> So I kind of see why people wanted to return ESPIPE, but at the same
> time it really is very misleading: ESPIPE is for pure streams that
> can't lseek at all.

That was indeed a misunderstanding of mine wrt ESPIPE meaning.
And I agree with your previous paragraph where you stated that the
buffer is only not "relative" seekable. So, ack for the revert.

However, the issue with glibc is their fd checking on dprintf using:

lseek(offset == 0, whence == SEEK_CUR)

Which, technically, isn't a relative seek operation in my opinion, thus
I'm also not sure that returning EINVAL is correct. 

Would it make sense to return the next buffer index instead? Basically
the same as SEEK_END does? The first "if (offset)" in the function would
prevent any real relative move while SEEK_CUR would return a valid
address following this buffer behavior of specific points it could seek
to.

> 
> The fact that some silly shell internal then reacts very badly to that
> may be extreme, but it may be as well as you can do it you worry about
> leaving data for the next user.
> 
> I've applied the revert.
> 
>              Linus
> 

-- 
bmeneg 
PGP Key: http://bmeneg.com/pubkey.txt

Attachment: signature.asc
Description: PGP signature