On Wed, Mar 27, 2019 at 11:15 AM Kirill Smelkov <k...@nexedi.com> wrote: > > FUSE filesystem server and kernel client negotiate during initialization > phase, what should be the maximum write size the client will ever issue. > Correspondingly the filesystem server then queues sys_read calls to read > requests with buffer capacity large enough to carry request header > + that max_write bytes. A filesystem server is free to set its max_write > in anywhere in the range between [1·page, fc->max_pages·page]. In > particular go-fuse[2] sets max_write by default as 64K, wheres default > fc->max_pages corresponds to 128K. Libfuse also allows users to > configure max_write, but by default presets it to possible maximum. > > If max_write is < fc->max_pages·page, and in NOTIFY_RETRIEVE handler we > allow to retrieve more than max_write bytes, corresponding prepared > NOTIFY_REPLY will be thrown away by fuse_dev_do_read, because the > filesystem server, in full correspondence with server/client contract, > will be only queuing sys_read with ~max_write buffer capacity, and > fuse_dev_do_read throws away requests that cannot fit into server > request buffer. In turn the filesystem server could get stuck waiting > indefinitely for NOTIFY_REPLY since NOTIFY_RETRIEVE handler returned OK > which is understood by clients as that NOTIFY_REPLY was queued and will > be sent back. > > -> Cap requested size to negotiate max_write to avoid the problem. > This aligns with the way NOTIFY_RETRIEVE handler works, which already > unconditionally caps requested retrieve size to fuse_conn->max_pages. > This way it should not hurt NOTIFY_RETRIEVE semantic if we return less > data than was originally requested. > > Please see [1] for context where the problem of stuck filesystem was hit > for real, how the situation was traced and for more involving patch that > did not make it into the tree. > > [1] https://marc.info/?l=linux-fsdevel&m=155057023600853&w=2 > [2] https://github.com/hanwen/go-fuse > > Signed-off-by: Kirill Smelkov <k...@nexedi.com> > Cc: Han-Wen Nienhuys <han...@google.com> > Cc: Jakob Unterwurzacher <jakob...@gmail.com> > Cc: <sta...@vger.kernel.org> # v2.6.36+ > --- > fs/fuse/dev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > index 8a63e52785e9..38e94bc43053 100644 > --- a/fs/fuse/dev.c > +++ b/fs/fuse/dev.c > @@ -1749,7 +1749,7 @@ static int fuse_retrieve(struct fuse_conn *fc, struct > inode *inode, > offset = outarg->offset & ~PAGE_MASK; > file_size = i_size_read(inode); > > - num = outarg->size; > + num = min(outarg->size, fc->max_write);
This is wrong: the max_size limited num is overwritten if constrained by file size. Also the patch is whitespace damaged. Thanks, Miklos
