Messages in this thread | | | Subject | Re: [PATCH v2 1/2] fuse: Fix possible deadlock when writing back dirty pages | From | Baolin Wang <> | Date | Wed, 14 Apr 2021 18:18:34 +0800 |
| |
在 2021/4/14 17:47, Miklos Szeredi 写道: > On Wed, Apr 14, 2021 at 11:22 AM Baolin Wang > <baolin.wang@linux.alibaba.com> wrote: >> >> >> >> 在 2021/4/14 17:02, Miklos Szeredi 写道: >>> On Wed, Apr 14, 2021 at 10:42 AM Baolin Wang >>> <baolin.wang@linux.alibaba.com> 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@linux.alibaba.com>
| |