| Message ID | 20220625110115.39956-4-Jason@zx2c4.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cleanup llseek and splice | expand |
On Sat, Jun 25, 2022 at 01:01:10PM +0200, Jason A. Donenfeld wrote: > This helps unify a longstanding wart where FMODE_LSEEK hasn't been > uniformly unset when it should be. I think we could just remove FMODE_LSEEK after the previous patch as we can just check for the presence of a ->llseek method instead.
On Sat, Jun 25, 2022 at 06:07:45AM -0700, Christoph Hellwig wrote: > On Sat, Jun 25, 2022 at 01:01:10PM +0200, Jason A. Donenfeld wrote: > > This helps unify a longstanding wart where FMODE_LSEEK hasn't been > > uniformly unset when it should be. > > I think we could just remove FMODE_LSEEK after the previous patch > as we can just check for the presence of a ->llseek method instead. I wouldn't bet on that - as it is, an ->open() instance can decide in some cases to clear FMODE_LSEEK, despite having file_operations with non-NULL ->llseek.
On Sat, Jun 25, 2022 at 02:29:09PM +0100, Al Viro wrote: > I wouldn't bet on that - as it is, an ->open() instance can decide > in some cases to clear FMODE_LSEEK, despite having file_operations > with non-NULL ->llseek. The interesting cases here are nonseekable_open and stream_open, and I don't see why we could not fix this up in the file_operations.
On Sat, Jun 25, 2022 at 06:39:52AM -0700, Christoph Hellwig wrote: > On Sat, Jun 25, 2022 at 02:29:09PM +0100, Al Viro wrote: > > I wouldn't bet on that - as it is, an ->open() instance can decide > > in some cases to clear FMODE_LSEEK, despite having file_operations > > with non-NULL ->llseek. > > The interesting cases here are nonseekable_open and stream_open, > and I don't see why we could not fix this up in the file_operations. What's the point, really? We can easily enforce "no FMODE_LSEEK ever observed on files with NULL ->llseek" (this series does that), so we can use that check alone in e.g. vfs_llseek() or dump_skip(). Sure, we are tight on bits in ->f_mode, but there's a better way to relieve that problem - split the field into "stuff that needs to be preserved all the way until __fput()" and the rest; the latter could sit next to ->f_iocb_flags, with no increase of struct file size. So if you are worried about FMODE_... space getting exhausted, that's better dealt with in a different way, IMO.
diff --git a/fs/file_table.c b/fs/file_table.c index 5424e3a8df5f..933686f84bf8 100644 --- a/fs/file_table.c +++ b/fs/file_table.c @@ -243,6 +243,8 @@ static struct file *alloc_file(const struct path *path, int flags, file->f_mode |= FMODE_CAN_WRITE; file->f_mode |= FMODE_OPENED; file->f_op = fop; + if (file->f_op->llseek) + file->f_mode |= FMODE_LSEEK; if ((file->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) i_readcount_inc(path->dentry->d_inode); return file; diff --git a/fs/open.c b/fs/open.c index 1d57fbde2feb..07c332753a36 100644 --- a/fs/open.c +++ b/fs/open.c @@ -858,6 +858,8 @@ static int do_dentry_open(struct file *f, if ((f->f_mode & FMODE_WRITE) && likely(f->f_op->write || f->f_op->write_iter)) f->f_mode |= FMODE_CAN_WRITE; + if ((f->f_mode & FMODE_LSEEK) && !f->f_op->llseek) + f->f_mode &= ~FMODE_LSEEK; if (f->f_mapping->a_ops && f->f_mapping->a_ops->direct_IO) f->f_mode |= FMODE_CAN_ODIRECT;
This helps unify a longstanding wart where FMODE_LSEEK hasn't been uniformly unset when it should be. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- fs/file_table.c | 2 ++ fs/open.c | 2 ++ 2 files changed, 4 insertions(+)