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. Your patch may make sense regardless, but it needs to have a clear analysis about why the fuse_wait_on_page_writeback() was needed in the first place (it's not clear from the history) or why it's okay to move it. Thanks, Miklos
