Re: [PATCH v2 1/2] fuse: Fix possible deadlock when writing back dirty pages

[Baolin Wang]

在 2021/4/14 17:47, Miklos Szeredi 写道:
> On Wed, Apr 14, 2021 at 11:22 AM Baolin Wang
> <baolin.wang@xxxxxxxxxxxxxxxxx> wrote:
> > 在 2021/4/14 17:02, Miklos Szeredi 写道:
> > > On Wed, Apr 14, 2021 at 10:42 AM Baolin Wang
> > > <baolin.wang@xxxxxxxxxxxxxxxxx> wrote:
> > >
> > > > Sorry I missed this patch before, and I've tested this patch, it seems
> > > > can solve the deadlock issue I met before.
> > >
> > > Great, thanks for testing.
> > >
> > > > But look at this patch in detail, I think this patch only reduced the
> > > > deadlock window, but did not remove the possible deadlock scenario
> > > > completely like I explained in the commit log.
> > > >
> > > > Since the fuse_fill_write_pages() can still lock the partitail page in
> > > > your patch, and will be wait for the partitail page waritehack is
> > > > completed if writeback is set in fuse_send_write_pages().
> > > >
> > > > But at the same time, a writeback worker thread may be waiting for
> > > > trying to lock the partitail page to write a bunch of dirty pages by
> > > > fuse_writepages().
> > >
> > > As you say, fuse_fill_write_pages() will lock a partial page.  This
> > > page cannot become dirty, only after being read completely, which
> > > first requires the page lock.  So dirtying this page can only happen
> > > after the writeback of the fragment was completed.
> >
> > What I mean is the writeback worker had looked up the dirty pages in
> > write_cache_pages() and stored them into a temporary pagevec, then try
> > to lock dirty page one by one and write them.
> >
> > For example, suppose it looked up 2 dirty pages (named page 1 and page
> > 2), and writed down page 1 by fuse_writepages_fill(), unlocked page 1.
> > Then try to lock page 2.
> >
> > At the same time, suppose the fuse_fill_write_pages() will write the
> > same page 1 and partitail page 2, and it will lock partital page 2 and
> > wait for the page 1's writeback is completed. But page 1's writeback can
> > not be completed, since the writeback worker is waiting for locking page
> > 2, which was already locked by fuse_fill_write_pages().
>
> How would page2 become not uptodate, when it was already collected by
> write_cache_pages()?  I.e. page2 is a dirty page, hence it must be
> uptodate, and fuse_writepages_fill() will not keep it locked.

Read your patch carefully again, now I realized you are right, and your 
patch can solve the deadlock issue I met. Please feel free to add my 
tested-by tag for your patch. Thanks.

Tested-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>