Re: git: 9ac6eda6c6a3 - main - pipe: try to skip locking the pipe if a non-blocking fd is used

On Wed, Aug 17, 2022 at 07:07:26PM +0200, Mateusz Guzik wrote:
M> On 8/17/22, Gleb Smirnoff <glebius@freebsd.org> wrote:
M> > On Wed, Aug 17, 2022 at 06:21:33PM +0200, Mateusz Guzik wrote:
M> > M> I don't understand this whatsoever. I patched the read side, not write.
M> > M>
M> > M> So in particular, should you find there is free space in the pipe to
M> > M> write to, there is literally no change.
M> > M>
M> > M> At worst, if a non-blocking read was racing a write, there is a bigger
M> > M> window where the reader will conclude there is nothing to get and
M> > M> leave.
M> > M>
M> > M> I see the following races:
M> > M> 1. non-blocking reader arrived before the writer managed to busy the
M> > M> pipe, in which case the outcome is the same in both old and new code
M> > M> 2. non-blocking reader arrived after the writer managed to unbusy the
M> > M> pipe, in which case the outcome is once again the same -- the write
M> > M> was completed and reader sees it
M> > M> 3. non-blocking reader arrived after the writer managed to busy the
M> > M> pipe, but before it got unbusied. here the old code would always wait,
M> > M> while the new will or will not wait depending on whether the writer
M> > M> managed to bump data counters or not.
M> > M>
M> > M> Ultimately there is no *new* outcome for any of it.
M> >
M> > The problem is symmetrical for both read and write.  I'm sorry that
M> > I used write scenario to describe a problem.
M> >
M> > So, let's look into third case. We got event dispatcher reporting
M> > to us that next read(2) would be successful:
M> >
M> >      POLLRDNORM     Normal data may be read without blocking.
M> >
M> > or
M> >
M> >      EVFILT_READ         Takes a descriptor as the identifier, and returns
M> >                          whenever there is data available to read.  The
M> >                          behavior of the filter is slightly different
M> >                          depending on the descriptor type.
M> >                          Fifos, Pipes
M> >                              Returns when the there is data to read; data
M> >                              contains the number of bytes available.
M> >
M> > But next read(2) would return EAGAIN. This is broken contract.
M> >
M> 
M> Once more I don't see it, can you show a simple graph how this is
M> supposed to happen all while not being possible on the previous
M> kernel? See below for an example.
M> 
M> Say there is only one thread which sees the reader side and that
M> thread performs the poll/select/kqueue call. Once it gets the
M> notification, it proceeds to do the read which of course works fine.
M> 
M> Say there are more threads or even multiple processes. If userspace
M> took no measures to synchronize read calls vs poll/select/kqueue calls
M> (that is some threads just read while others poll and only then read,
M> for example), it runs into races which are the same for both old and
M> new code (with one slight distinction of what happens when the write
M> is in progress)
M> 
M> thread1    thread2
M> poll
M>            write
M>   ret
M> read
M> 
M> thread1 finds data no problem.

Yes, you are right and I was wrong. Now I see that, since we always check
the counters to be 0. Sorry, for taking your time.

-- 
Gleb Smirnoff