On Wed, Feb 27, 2019 at 09:26:32PM +0100, Miklos Szeredi wrote: > On Wed, Feb 27, 2019 at 9:02 PM Kirill Smelkov <kirr@xxxxxxxxxx> wrote: > > > > Miklos, first of all thanks for feedback. > > > > On Tue, Feb 26, 2019 at 04:14:22PM +0100, Miklos Szeredi wrote: > > > On Tue, Feb 19, 2019 at 10:42 AM Kirill Smelkov <kirr@xxxxxxxxxx> wrote: > > > > > > > > A successful call to NOTIFY_RETRIEVE by filesystem carries promise from > > > > the kernel to send back NOTIFY_REPLY message. However if the filesystem > > > > is not reading requests with fuse_conn->max_pages capacity, > > > > > > That's a violation of the contract by the fuse server, not the kernel. > > > > Do you mean that even if filesystem server configures via > > init_out.max_write that it is accepting e.g. only 32K max writes, it > > still has to be issuing sys_read with buffer of 128K (= hardcoded > > fuse_conn->max_pages before Linux 4.20, and default since Linux 4.20)? > > Filesystem is asking for a specific number of bytes to retrieve. It > does not have to be less than max_writes, but it does have to fit into > the request buffer it is using. If the filesystem is asking to > retrieve 64k and it is using a 32k request buffer, then that obviously > won't work. Kernel could limit the retrieve length to max_writes, > that may make sense, but it doesn't fundamentally change the fact that > if the filesystem is not properly sizing the request buffer, it may > result in various forms of breakage. I more or less agree with this statement. However can we please make the breakage to be explicitly visible with an error instead of exhibiting it via harder to debug stucks/deadlocks? For example sys_read < max_write -> error instead of getting stuck. And if notify_retrieve requests buffer larger than max_write -> error or cut to max_write, but don't return OK when we know we will never send what was requested to filesystem even if it uses max_write sized reads. What is the point of breaking in hard to diagnose way when we can make the breakage showing itself explicitly? Would a patch for such behaviour accepted? Thanks, Kirill
