All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kirill Smelkov <kirr@nexedi.com>
To: Miklos Szeredi <miklos@szeredi.hu>, Miklos Szeredi <mszeredi@redhat.com>
Cc: <linux-fsdevel@vger.kernel.org>,
	<fuse-devel@lists.sourceforge.net>,
	Kirill Smelkov <kirr@nexedi.com>,
	Han-Wen Nienhuys <hanwen@google.com>,
	Jakob Unterwurzacher <jakobunt@gmail.com>
Subject: [RESEND3, PATCH 0/2] fuse: don't stuck clients on retrieve_notify with size > max_write
Date: Thu, 14 Mar 2019 10:45:55 +0000	[thread overview]
Message-ID: <cover.1552558717.git.kirr@nexedi.com> (raw)
In-Reply-To: <20190307093421.GA4620@deco.navytux.spb.ru>

Miklos,

On Thu, Feb 28, 2019 at 02:47:57PM +0300, Kirill Smelkov wrote:
> On Thu, Feb 28, 2019 at 09:10:15AM +0100, Miklos Szeredi wrote:
> > On Wed, Feb 27, 2019 at 9:39 PM Kirill Smelkov <kirr@nexedi.com> wrote:
> > 
> > > 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?
> > 
> > Sure, if it's only adds a couple of lines.   Adding more than say ten
> > lines for such a non-bug fix is definitely excessive.
>
> Ok, thanks. Please consider applying the following patch. (It's a bit
> pity to hear the problem is not considered to be a bug, but anyway).
>
> I will also send the second patch as another mail, since I could not
> made `git am --scissors` to apply several patched extracted from one
> mail successfully.

[...]

On Thu, Mar 07, 2019 at 12:34:21PM +0300, Kirill Smelkov wrote:
> Ping. Miklos, is there anything wrong with this patch and its
> second counterpart?

As we were talking here are those patches. The first one cuts notify_retrieve
request to max_write and is one line only. The second one returns error to
filesystem server if it is buggy and does sys_read with buffer size <
max_write. It is 2 lines of code and 7 lines of comments.

I still think that the patches fix real bugs. It is a bug if server behaviour
is a bit non-confirming or simply on an edge of being correct or questionable,
and instead of properly getting plain error from kernel, the whole system gets
stuck. It is a bug because bug amplification factor here is at least one order
of magnitude instead of staying ~1x.

I'm sending the patches for the third time already, but did not get any
feedback. Could you please have a look?

Thanks beforehand,
Kirill


Kirill Smelkov (2):
  fuse: retrieve: cap requested size to negotiated max_write
  fuse: require /dev/fuse reads to have enough buffer capacity as negotiated

 fs/fuse/dev.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

-- 
2.21.0.225.g810b269d1a

  reply	other threads:[~2019-03-14 11:00 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-19  9:42 [RESEND, PATCH v2] fuse: Don't drop NOTIFY_REPLY if we promised it Kirill Smelkov
2019-02-19  9:42 ` Kirill Smelkov
2019-02-26 15:14 ` Miklos Szeredi
2019-02-27 20:02   ` Kirill Smelkov
2019-02-27 20:02     ` Kirill Smelkov
2019-02-27 20:26     ` Miklos Szeredi
2019-02-27 20:39       ` Kirill Smelkov
2019-02-28  8:10         ` Miklos Szeredi
2019-02-28 11:48           ` Kirill Smelkov
2019-02-28 11:48             ` Kirill Smelkov
2019-02-28 11:50             ` [PATCH 2/2] fuse: require /dev/fuse reads to have enough buffer capacity as negotiated Kirill Smelkov
2019-03-07  9:34             ` [RESEND, PATCH v2] fuse: Don't drop NOTIFY_REPLY if we promised it Kirill Smelkov
2019-03-14 10:45               ` Kirill Smelkov [this message]
2019-03-14 10:46                 ` [PATCH 1/2] fuse: retrieve: cap requested size to negotiated max_write Kirill Smelkov
2019-03-14 10:46                 ` [PATCH 2/2] fuse: require /dev/fuse reads to have enough buffer capacity as negotiated Kirill Smelkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1552558717.git.kirr@nexedi.com \
    --to=kirr@nexedi.com \
    --cc=fuse-devel@lists.sourceforge.net \
    --cc=hanwen@google.com \
    --cc=jakobunt@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=mszeredi@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.