Re: [PATCH v2 2/2] fuse: remove tmp folio for writebacks and internal rb tree

[Jingbo Xu <jefflexu@linux.alibaba.com>]

On 10/26/24 2:19 AM, Joanne Koong wrote:
> On Fri, Oct 25, 2024 at 11:02 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>>
>> On Fri, 25 Oct 2024 at 19:36, Joanne Koong <joannelkoong@gmail.com> wrote:
>>
>>> That's a great point. It seems like we can just skip waiting on
>>> writeback to finish for fuse folios in sync(2) altogether then. I'll
>>> look into what's the best way to do this.
>>
>> I just tested this, and it turns out this doesn't quite work the way
>> I'd expected.  I can trigger sync(2) being blocked by a suspended fuse
>> server:
>>
>> task:kworker/u16:3   state:D stack:0     pid:172   tgid:172   ppid:2
>>    flags:0x00004000
>> Workqueue: writeback wb_workfn (flush-0:30)
>> Call Trace:
>>  __schedule+0x40b/0xad0
>>  schedule+0x36/0x120
>>  inode_sleep_on_writeback+0x9d/0xb0
>>  wb_writeback+0x104/0x3d0
>>  wb_workfn+0x325/0x490
>>  process_one_work+0x1d8/0x520
>>  worker_thread+0x1af/0x390
>>  kthread+0xcc/0x100
>>  ret_from_fork+0x2d/0x50
>>  ret_from_fork_asm+0x1a/0x30
>>
>> task:dd              state:S stack:0     pid:1364  tgid:1364
>> ppid:1336   flags:0x00000002
>> Call Trace:
>>  __schedule+0x40b/0xad0
>>  schedule+0x36/0x120
>>  request_wait_answer+0x16b/0x200
>>  __fuse_simple_request+0xd6/0x290
>>  fuse_flush_times+0x119/0x140
>>  fuse_write_inode+0x6d/0xc0
>>  __writeback_single_inode+0x36d/0x480
>>  writeback_single_inode+0xa8/0x170
>>  write_inode_now+0x75/0xa0
>>  fuse_flush+0x85/0x1c0
>>  filp_flush+0x2c/0x70
>>  __x64_sys_close+0x2e/0x80
>>  do_syscall_64+0x64/0x140
>>  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>
>> task:sync            state:D stack:0     pid:1365  tgid:1365
>> ppid:1336   flags:0x00004002
>> Call Trace:
>>  __schedule+0x40b/0xad0
>>  schedule+0x36/0x120
>>  wb_wait_for_completion+0x56/0x80
>>  sync_inodes_sb+0xc5/0x450
>>  iterate_supers+0x69/0xd0
>>  ksys_sync+0x40/0xa0
>>  __do_sys_sync+0xa/0x20
>>  do_syscall_64+0x64/0x140
>>  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>
> 
> Thanks for the trace. If i'm understanding it correctly, this only
> blocks temporarily until the writeback wb_workfn is rescheduled?
> 
>> Maybe I'm too paranoid about this, and in practice we can just let
>> sync(2) block in any case.  But I want to understand this better
> 
> in the case of a malicious fuse server, it could block sync(2) forever
> so I think this means we have to skip the wait for fuse folios
> altogether.

Right. In summary we need to skip waiting on the completion for the
writeback in case of a malicious fuse server could block sync(2)
forever, and meanwhile the change won't break the backward compatibility
as the current fuse implementation also doesn't wait for the data
actually being written back and persistent on the backstore in sync(2).


-- 
Thanks,
Jingbo