Re: [Devel] [PATCH hci-8.0 2/2] fs/fuse: unify dropping the fuse file reference of a writeback request

[Konstantin Khorenko <khorenko@virtuozzo.com>]

  ▎ One more spot of the same class, outside this series: the error path of fuse_writepage_locked().
  ▎
  ▎ Since mainline commit 0acad9289be3 ("fuse: refactor out shared logic in fuse_writepages_fill() and
  ▎ fuse_writepage_locked()") the fuse file reference is taken by fuse_write_file_get() before the wpa is allocated, so the
  ▎ wpa allocation failure path now holds a reference and drops it with a plain fuse_file_put(ff, false). Before that commit
  ▎ the reference was taken last, after all allocations, and this path never owned one.
  ▎
  ▎ On a close_wait mount that put is only a decrement: it does not wake fi->page_waitq. fuse_release() removes ff from
  ▎ fi->write_files under fi->lock and then sleeps in wait_event(fi->page_waitq, ff->count == 1). If fuse_writepage_locked()
  ▎ grabbed ff via fuse_write_file_get() just before the list_del_init() and then fails the wpa allocation, its put brings
  ▎ ff->count to 1 without a wakeup, and fuse_release() stays asleep until some unrelated writeback on the same inode
  ▎ happens to wake the queue, which may never come for a file being closed. The same pattern, put plus wake, is what
  ▎ fuse_release_ff() encapsulates and what fuse_writepages() already uses for data.ff.
  ▎
  ▎ The window is narrow (a GFP_NOFS allocation of a few hundred bytes has to fail while a close races with writepage), but
  ▎ the consequence is a task stuck in close() forever. A one-line follow-up patch replacing fuse_file_put() with
  ▎ fuse_release_ff() in that error path is enough; proposed below.

  Patch:

  fs/fuse: wake close_wait waiters when fuse_writepage_locked() bails out

  fuse_writepage_locked() takes the fuse file reference before allocating
  the wpa and drops it with fuse_file_put() if the allocation fails. On a
  close_wait mount that put does not wake fi->page_waitq, while
  fuse_release() may already be sleeping there for ff->count to reach 1
  after removing ff from fi->write_files. If the writepage grabbed ff
  right before that removal and then failed the allocation, the last
  writeback reference goes away silently and close() hangs until some
  unrelated writeback on the inode wakes the queue.

  Use fuse_release_ff(), which does the put and the wakeup, as the other
  writeback paths already do.

  Fixes: 6b5ddd8253eb ("fuse: Wait for real writeback to finish on file")
  https://virtuozzo.atlassian.net/browse/VSTOR-145614
  Feature: vStorage

  ---
   fs/fuse/file.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

  diff --git a/fs/fuse/file.c b/fs/fuse/file.c
  --- a/fs/fuse/file.c
  +++ b/fs/fuse/file.c
  @@ -2572,7 +2572,7 @@ static int fuse_writepage_locked(struct folio *folio)
        return 0;

   err_writepage_args:
  -     fuse_file_put(ff, false);
  +     fuse_release_ff(inode, ff);
   err_nofile:
        folio_put(tmp_folio);
   err:

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel