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@nexedi.com 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@nexedi.com 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