From: Amir Goldstein <firstname.lastname@example.org>
To: Andreas Dilger <email@example.com>
Cc: Miklos Szeredi <firstname.lastname@example.org>,
Al Viro <email@example.com>,
Dave Chinner <firstname.lastname@example.org>,
Subject: Re: [PATCH v3 6/6] ovl: add ovl_fadvise()
Date: Sat, 28 Dec 2019 12:10:38 +0200 [thread overview]
Message-ID: <CAOQ4uxjuJ-6Tw3vw1qahjp2LrGPx=eZfZA9qk47=mWSamEiFemail@example.com> (raw)
On Sat, Dec 28, 2019 at 7:49 AM Andreas Dilger <firstname.lastname@example.org> wrote:
> On Aug 27, 2018, at 6:56 AM, Amir Goldstein <email@example.com> wrote:
> > Implement stacked fadvise to fix syscalls readahead(2) and fadvise64(2)
> > on an overlayfs file.
> I was just looking into the existence of the "new" fadvise() method in
> the VFS being able to communicate application hints directly to the
> filesystem to see if it could be used to address the word size issue in
> https://bugzilla.kernel.org/show_bug.cgi?id=205957 without adding a new
> syscall, and came across this patch and the 4/6 patch that adds the
> vfs_fadvise() function itself (copied below for clarity).
> It seems to me that this implementation is broken? Only vfs_fadvise()
> is called from the fadvise64() syscall, and it will call f_op->fadvise()
> if the filesystem provides this method. Only overlayfs provides the
> .fadvise method today. However, it looks that ovl_fadvise() calls back
> into vfs_fadvise() again, in a seemingly endless loop?
You are confusing endless loop with recursion that has a stop condition.
The entire concept of stacked filesystem is recursion back into vfs.
This is essentially what most of the ovl file operations do, but they recurse
on the "real.file", which is supposed to be on a filesystem with lower
sb->s_stack_depth (FILESYSTEM_MAX_STACK_DEPTH is 2).
> It seems like generic_fadvise() should be EXPORT_SYMBOL() so that any
> filesystem that implements its own .fadvise method can do its own thing,
> and then call generic_fadvise() to handle the remaining MM-specific work.
Sure makes sense.
Overlayfs just doesn't need to call the generic helper.
next prev parent reply other threads:[~2019-12-28 10:10 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <firstname.lastname@example.org>
2018-08-27 12:55 ` [PATCH v3 1/6] ovl: respect FIEMAP_FLAG_SYNC flag Amir Goldstein
[not found] ` <email@example.com>
2018-08-27 13:56 ` [PATCH v3 4/6] vfs: add the fadvise() file operation Steve French
[not found] ` <firstname.lastname@example.org>
2018-08-27 18:52 ` [PATCH v3 6/6] ovl: add ovl_fadvise() Vivek Goyal
2018-08-27 19:05 ` Amir Goldstein
2018-08-27 19:29 ` Miklos Szeredi
2019-12-28 5:49 ` Andreas Dilger
2019-12-28 10:10 ` Amir Goldstein [this message]
[not found] ` <email@example.com>
2018-08-27 23:30 ` [PATCH v3 5/6] vfs: implement readahead(2) using POSIX_FADV_WILLNEED Dave Chinner
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:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* 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 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).