Message-ID: <CAOQ4uxgCrxMKO7ZgAriMkKU-aKnShN+CG0XqP-yYFiyR=Os82A@mail.gmail.com>
Date:   Mon, 26 Jun 2023 17:53:46 +0300
From:   Amir Goldstein <amir73il@...il.com>
To:     Ahelenia Ziemiańska 
        <nabijaczleweli@...ijaczleweli.xyz>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jan Kara <jack@...e.cz>
Subject: Re: splice(-> FIFO) never wakes up inotify IN_MODIFY?

On Mon, Jun 26, 2023 at 3:19 PM Ahelenia Ziemiańska
<nabijaczleweli@...ijaczleweli.xyz> wrote:
>
> On Mon, Jun 26, 2023 at 09:11:53AM +0300, Amir Goldstein wrote:
> > On Mon, Jun 26, 2023 at 6:54 AM Ahelenia Ziemiańska
> > <nabijaczleweli@...ijaczleweli.xyz> wrote:
> > >
> > > Hi!
> > >
> > > Consider the following programs:
> > > -- >8 --
> > > ==> ino.c <==
> > > #define _GNU_SOURCE
> > > #include <stdio.h>
> > > #include <sys/inotify.h>
> > > #include <unistd.h>
> > > int main() {
> > >   int ino = inotify_init1(IN_CLOEXEC);
> > >   inotify_add_watch(ino, "/dev/fd/0", IN_MODIFY);
> > >
> > >   char buf[64 * 1024];
> > >   struct inotify_event ev;
> > >   while (read(ino, &ev, sizeof(ev)) > 0) {
> > >     fprintf(stderr, "%d: mask=%x, cook=%x, len=%x, name=%.*s\n", ev.wd, ev.mask,
> > >             ev.cookie, ev.len, (int)ev.len, ev.name);
> > >     fprintf(stderr, "rd=%zd\n", read(0, buf, sizeof(buf)));
> > >   }
> > > }
> > >
> >
> > That's a very odd (and wrong) way to implement poll(2).
> > This is not a documented way to use pipes, so it may
> > happen to work with sendfile(2), but there is no guarantee.
> That's what I'm trying to do, yes.
> What's the right way to implement poll here? Because I don't think Linux
> has poll for pipes that behaves like this and POSIX certainly doesn't
> guarantee it, and, indeed, requires that polling a pipe that was hanged
> up instantly returns with POLLHUP forever.
>
> > > -- >8 --
> > > $ make se sp ino
> > > $ mkfifo fifo
> > > $ ./ino < fifo &
> > > [1] 230
> > > $ echo a > fifo
> > > $ echo a > fifo
> > > 1: mask=2, cook=0, len=0, name=
> > > rd=4
> > > $ echo c > fifo
> > > 1: mask=2, cook=0, len=0, name=
> > > rd=2
> > > $ ./se > fifo
> > > abcdef
> > > 1: mask=2, cook=0, len=0, name=
> > > asd
> > > ^D
> > > se=11: Success
> > > rd=11
> > > 1: mask=2, cook=0, len=0, name=
> > > rd=0
> > > $ ./sp > fifo
> > > abcdefg
> > > asd
> > > dsasdadadad
> > > sp=24: Success
> > > $ < sp ./sp > fifo
> > > sp=25856: Success
> > > $ < sp ./sp > fifo
> > > ^C
> > > $ echo sp > fifo
> > > ^C
> > > -- >8 --
> > >
> > > Note how in all ./sp > fifo cases, ./ino doesn't wake up!
> > > Note also how, thus, we've managed to fill the pipe buffer with ./sp
> > > (when it transferred 25856), and now we can't /ever/ write there again
> > > (both splicing and normal writes block, since there's no space left in
> > >  the pipe; ./ino hasn't seen this and will never wake up or service the
> > >  pipe):
> > > so we've effectively "denied service" by slickily using a different
> > > syscall to do the write, right?
> > >
> > > I consider this to be unexpected behaviour because (a) obviously and
> > > (b) sendfile() sends the inotify event.
> > >
> > Only applications that do not check for availability
> > of input in the pipe correctly will get "denied service".
> >
> > The fact is that relying on inotify IN_MODIFY and IN_ACCESS events
> > for pipes is not a good idea.
> Okay, so how /is/ "correctly" then?
> Sleep in a loop and read non-blockingly?
> splice also breaks that (and, well, the pipe it's splicing to in general)
>   https://lore.kernel.org/linux-fsdevel/qk6hjuam54khlaikf2ssom6custxf5is2ekkaequf4hvode3ls@zgf7j5j4ubvw/t/#u
> but that's beside the point I guess.
>
> > splice(2) differentiates three different cases:
> >         if (ipipe && opipe) {
> > ...
> >         if (ipipe) {
> > ...
> >         if (opipe) {
> > ...
> >
> > IN_ACCESS will only be generated for non-pipe input
> > IN_MODIFY will only be generated for non-pipe output
> >
> > Similarly FAN_ACCESS_PERM fanotify permission events
> > will only be generated for non-pipe input.
> inotify(7) and fanotify(7) don't squeak on that,
> and imply the *ACCESS stuff is just for reading.

Correct.

>
> > sendfile(2) OTOH does not special cases the pipe input
> > case at all and it generates IN_MODIFY for the pipe output
> > case as well.
> >
> > My general opinion about IN_ACCESS/IN_MODIFY
> > (as well as FAN_ACCESS_PERM) is that they are not
> > very practical, not well defined for pipes and anyway do
> > not cover all the ways that a file can be modified/accessed
> > (i.e. mmap). Therefore, IMO, there is no incentive to fix
> > something that has been broken for decades unless
> > you have a very real use case - not a made up one.
> My made-up use-case is tail -f, but I can just request
> IN_MOFIFY|IN_ACCESS for pipes, so if that's "correctly" then great.
> If it isn't, then, again, how /do/ you poll pipes.
>
> > If you would insist on fixing this inconsistency, I would be
> > willing to consider a patch that matches sendfile(2) behavior
> > to that of splice(2) and not the other way around.
> Meh, platform-specific API, long-standing behaviour, it's whatever;
> I'll just update *notify(7) to include that *ACCESSses are generated
> for "wants to/has read OR pipe was written".

I guess you already understood that is not what I meant.

> So is it really true that the only way to poll a pipe is a
> sleep()/read(O_NONBLOCK) loop?

I don't think so, but inotify is not the way.

Thanks,
Amir.

Powered by blists - more mailing lists