Re: [PATCH v3 0/2] fs/pipe: reduce pipe->mutex contention by pre-allocating outside the lock

[Oleg Nesterov <oleg@redhat.com>]

On 06/17, Mateusz Guzik wrote:
>
> > -static void anon_pipe_get_page_prealloc(struct anon_pipe_prealloc *prealloc,
> > +static void anon_pipe_get_page_prealloc(struct pipe_inode_info *pipe,
> > +                                       struct anon_pipe_prealloc *prealloc,
> >                                         size_t total_len)
> >  {
> >         unsigned int want, i;
> > @@ -144,6 +145,11 @@ static void anon_pipe_get_page_prealloc(struct anon_pipe_prealloc *prealloc,
> >         want = min_t(unsigned int, DIV_ROUND_UP(total_len, PAGE_SIZE),
> >                      PIPE_PREALLOC_MAX);
> >
> > +       for (i = 0; i < ARRAY_SIZE(pipe->tmp_page); i++) {
> > +               if (pipe->tmp_page[i] && !--want)
> > +                       return;
> > +       }

> As proposed this will guarantee a big write which fits fine into pages
> cached into tmp_page followed by a small write will have to resort to
> an allocation under the mutex, partially defeating the original patch.
> So you would need to add some provisions to check if you need to
> allocate something even in that case.

Yes, with the change like this, at least the "total_len <= PAGE_SIZE"
check should be revisited.

But let me repeat: I'm not sure this makes sense, and I have no idea how
it would impact performance in "real" workloads.

In particular, I don't know if the case when another writer steals the
pages from ->tmp_page[] is actually "unlikely".

Oleg.