From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-kernel@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 2/6] fs: do not set no_llseek in fops
Date: Sat, 25 Jun 2022 12:46:04 +0200 [thread overview]
Message-ID: <YrbnbMcI+lbBoiHh@zx2c4.com> (raw)
In-Reply-To: <YrYxOC5dgCKBHwVE@ZenIV>
On Fri, Jun 24, 2022 at 10:48:40PM +0100, Al Viro wrote:
> On Fri, Jun 24, 2022 at 06:56:27PM +0200, Jason A. Donenfeld wrote:
> > vfs_llseek already does something with this, and it makes it difficult
> > to distinguish between llseek being supported and not.
>
> How about something along the lines of
>
> ===
> struct file_operations ->llseek() method gets called only in two places:
> vfs_llseek() and dump_skip(). Both treat NULL and no_llseek as
> equivalent.
>
> The value of ->llseek is also examined in __full_proxy_fops_init() and
> ovl_copy_up_data(). For the former we could as well treat no_llseek
> as NULL; no need to do a proxy wrapper around the function that fails
> with -ESPIPE without so much as looking at its arguments.
> Same for the latter - there no_llseek would end up with skip_hole
> set true until the first time we look at it. At that point we
> call vfs_llseek(), observe that it has failed (-ESPIPE), shrug and
> set skip_hole false. Might as well have done that from the very
> beginning.
>
> In other words, any place where .llseek is set to no_llseek
> could just as well set it to NULL.
> ===
>
> for commit message?
>
> Next commit would remove the checks for no_llseek and have vfs_llseek()
> just do
> if (file->f_mode & FMODE_LSEEK) {
> if (file->f_op->llseek)
> return file->f_op->llseek(file, offset, whence);
> }
> return -ESPIPE;
> and kill no_llseek() off. And once you have guaranteed that FMODE_LSEEK
> is never set with NULL ->llseek, vfs_llseek() gets trimmed in obvious
> way and tests in dump_skip() and ovl_copy_up_data() would become simply
> file->f_mode & FMODE_LSEEK - no need to check ->f_op->llseek there
> after that. At the same time dump_skip() could switch to calling
> vfs_llseek() instead of direct method call...
Thanks. I'll split things into steps more or less like that and borrow
that commit text for v2 (which I'll send out somewhat soon).
Jason
next prev parent reply other threads:[~2022-06-25 10:46 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-24 16:56 [PATCH 0/6] cleanup llseek and splice Jason A. Donenfeld
2022-06-24 16:56 ` [PATCH 1/6] ksmbd: use vfs_llseek instead of dereferencing NULL Jason A. Donenfeld
2022-06-24 21:20 ` Al Viro
2022-06-25 4:10 ` Namjae Jeon
2022-06-24 16:56 ` [PATCH 2/6] fs: do not set no_llseek in fops Jason A. Donenfeld
2022-06-24 21:48 ` Al Viro
2022-06-25 10:46 ` Jason A. Donenfeld [this message]
2022-06-24 16:56 ` [PATCH 3/6] fs: clear FMODE_LSEEK if no llseek function Jason A. Donenfeld
2022-06-24 17:04 ` Al Viro
2022-06-24 17:09 ` Jason A. Donenfeld
2022-06-24 17:13 ` Al Viro
2022-06-24 17:17 ` Jason A. Donenfeld
2022-06-24 16:56 ` [PATCH 4/6] fs: check FMODE_LSEEK to control internal pipe splicing Jason A. Donenfeld
2022-06-24 16:56 ` [PATCH 5/6] dma-buf: remove useless FMODE_LSEEK flag Jason A. Donenfeld
2022-06-24 21:51 ` Al Viro
2022-06-24 16:56 ` [PATCH 6/6] vfio: do not set " Jason A. Donenfeld
2022-06-24 21:53 ` Al Viro
2022-06-25 10:52 ` Jason A. Donenfeld
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=YrbnbMcI+lbBoiHh@zx2c4.com \
--to=jason@zx2c4.com \
--cc=axboe@kernel.dk \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®