linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miklos Szeredi <miklos@szeredi.hu>
To: Kirill Smelkov <kirr@nexedi.com>
Cc: Miklos Szeredi <mszeredi@redhat.com>,
	Han-Wen Nienhuys <hanwen@google.com>,
	Jakob Unterwurzacher <jakobunt@gmail.com>,
	Kirill Tkhai <ktkhai@virtuozzo.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	fuse-devel <fuse-devel@lists.sourceforge.net>,
	stable <stable@vger.kernel.org>
Subject: Re: [RESEND4, PATCH 1/2] fuse: retrieve: cap requested size to negotiated max_write
Date: Wed, 24 Apr 2019 17:02:42 +0200	[thread overview]
Message-ID: <CAJfpegtX6p+j+kRuudZ=yVM9Tgj7x4mBaiGuDoDCrZoRvBOt9Q@mail.gmail.com> (raw)
In-Reply-To: <20190424142249.GA28070@deco.navytux.spb.ru>

On Wed, Apr 24, 2019 at 4:22 PM Kirill Smelkov <kirr@nexedi.com> wrote:
> - FUSE_PRECISE_INVAL_DATA:
>
>     --- b/include/uapi/linux/fuse.h
>     +++ b/include/uapi/linux/fuse.h
>     @@ -266,7 +266,7 @@
>       * FUSE_MAX_PAGES: init_out.max_pages contains the max number of req pages
>       * FUSE_CACHE_SYMLINKS: cache READLINK responses
>       * FUSE_NO_OPENDIR_SUPPORT: kernel supports zero-message opendir
>     - * FUSE_PRECISE_INVAL_DATA: filesystem is fully responsible for data cache invalidation
>     + * FUSE_PRECISE_INVAL_DATA: filesystem is fully responsible for invalidation
>       */
>      #define FUSE_ASYNC_READ                (1 << 0)
>      #define FUSE_POSIX_LOCKS       (1 << 1)
>
> the "data cache" in "for data cache invalidation" has particular meaning
> and semantic: the filesystem promises to explicitly invalidate data of

Right; better name: FUSE_EXPLICIT_INVAL_DATA.  Will push fixed version.

> Your amendment for FOPEN_STREAM in uapi/linux/fuse.h (see above) also
> suggests that it is better to be more explicit in that file.
>
>     --- b/fs/fuse/inode.c
>     +++ b/fs/fuse/inode.c
>     @@ -913,13 +913,8 @@
>                                     fc->dont_mask = 1;
>                             if (arg->flags & FUSE_AUTO_INVAL_DATA)
>                                     fc->auto_inval_data = 1;
>     -                       if (arg->flags & FUSE_PRECISE_INVAL_DATA)
>     +                       else if (arg->flags & FUSE_PRECISE_INVAL_DATA)
>                                     fc->precise_inval_data = 1;
>     -                       if (fc->auto_inval_data && fc->precise_inval_data) {
>     -                               pr_warn("filesystem requested both auto and "
>     -                                       "precise cache control - using auto\n");
>     -                               fc->precise_inval_data = 0;
>     -                       }
>                             if (arg->flags & FUSE_DO_READDIRPLUS) {
>                                     fc->do_readdirplus = 1;
>                                     if (arg->flags & FUSE_READDIRPLUS_AUTO)
>
> Even though it is ok for me personally (I could be careful and use only
> FUSE_PRECISE_INVAL_DATA) I still think usage of both "auto" and "precise"
> invalidation modes deserves a warning. It is only at filesystem init time. What
> is the reason not to print it?

The warning makes no sense.  It should either be failure or silent override.

> - "fuse: retrieve: cap requested size to negotiated max_write"
>
>      Signed-off-by: Kirill Smelkov <kirr@nexedi.com>
>      Cc: Han-Wen Nienhuys <hanwen@google.com>
>      Cc: Jakob Unterwurzacher <jakobunt@gmail.com>
>     -Cc: <stable@vger.kernel.org> # v2.6.36+
>
> what is the reason not to include this patch into stable series?

This doens't fix any bugs out there, but there is a slight chance of
regression (so it might possibly have to be reverted in the future) so
it absolutely makes no sense to backport it to stable.

Thanks,
Miklos

  reply	other threads:[~2019-04-24 15:02 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-27 10:15 [RESEND4, PATCH 0/2] fuse: don't stuck clients on retrieve_notify with size > max_write Kirill Smelkov
2019-03-27 10:15 ` [RESEND4, PATCH 2/2] fuse: require /dev/fuse reads to have enough buffer capacity as negotiated Kirill Smelkov
2019-04-24 10:48   ` Miklos Szeredi
2019-04-24 11:58     ` Kirill Smelkov
2019-03-27 10:15 ` [RESEND4, PATCH 1/2] fuse: retrieve: cap requested size to negotiated max_write Kirill Smelkov
2019-04-24 10:44   ` Miklos Szeredi
2019-04-24 11:56     ` Kirill Smelkov
2019-04-24 12:17       ` Miklos Szeredi
2019-04-24 12:31         ` Kirill Smelkov
2019-04-24 13:19           ` Miklos Szeredi
2019-04-24 14:22             ` Kirill Smelkov
2019-04-24 15:02               ` Miklos Szeredi [this message]
2019-04-24 18:10                 ` Kirill Smelkov
2019-04-24 18:59                   ` 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='CAJfpegtX6p+j+kRuudZ=yVM9Tgj7x4mBaiGuDoDCrZoRvBOt9Q@mail.gmail.com' \
    --to=miklos@szeredi.hu \
    --cc=akpm@linux-foundation.org \
    --cc=fuse-devel@lists.sourceforge.net \
    --cc=hanwen@google.com \
    --cc=jakobunt@gmail.com \
    --cc=kirr@nexedi.com \
    --cc=ktkhai@virtuozzo.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mszeredi@redhat.com \
    --cc=stable@vger.kernel.org \
    --subject='Re: [RESEND4, PATCH 1/2] fuse: retrieve: cap requested size to negotiated max_write' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).