Messages in this thread | | | Date | Tue, 19 Dec 2023 16:35:54 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH] kernel: relay: remove relay_file_splice_read ‒ dead code, doesn't work |
| |
On Tue, 19 Dec 2023 23:24:14 +0100 Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> wrote:
> Documentation/filesystems/relay.rst says to use > return debugfs_create_file(filename, mode, parent, buf, > &relay_file_operations); > and this is the only way relay_file_operations is used. > > Thus: debugfs_create_file(&relay_file_operations) > -> __debugfs_create_file(&debugfs_full_proxy_file_operations, > &relay_file_operations) > -> dentry{inode: {i_fop: &debugfs_full_proxy_file_operations}, > d_fsdata: &relay_file_operations > | DEBUGFS_FSDATA_IS_REAL_FOPS_BIT} > > debugfs_full_proxy_file_operations.open is full_proxy_open, which > extracts the &relay_file_operations from the dentry, and allocates > via __full_proxy_fops_init() new fops, with trivial wrappers around > release, llseek, read, write, poll, and unlocked_ioctl, then replaces > the fops on the opened file therewith. > > Naturally, all thusly-created debugfs files have .splice_read = NULL. > This was introduced in > commit 49d200deaa680501f19a247b1fffb29301e51d2b ("debugfs: prevent > access to removed files' private data") from 2016-03-22. > > AFAICT, relay_file_operations is the only struct file_operations > used for debugfs which defines a .splice_read callback. > Hooking it up with > > diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c > > index 5063434be0fc..952fcf5b2afa 100644 > > --- a/fs/debugfs/file.c > > +++ b/fs/debugfs/file.c > > @@ -328,6 +328,11 @@ FULL_PROXY_FUNC(write, ssize_t, filp, > > loff_t *ppos), > > ARGS(filp, buf, size, ppos)); > > > > +FULL_PROXY_FUNC(splice_read, long, in, > > + PROTO(struct file *in, loff_t *ppos, struct pipe_inode_info *pipe, > > + size_t len, unsigned int flags), > > + ARGS(in, ppos, pipe, len, flags)); > > + > > FULL_PROXY_FUNC(unlocked_ioctl, long, filp, > > PROTO(struct file *filp, unsigned int cmd, unsigned long arg), > > ARGS(filp, cmd, arg)); > > @@ -382,6 +387,8 @@ static void __full_proxy_fops_init(struct file_operations *proxy_fops, > > proxy_fops->write = full_proxy_write; > > if (real_fops->poll) > > proxy_fops->poll = full_proxy_poll; > > + if (real_fops->splice_read) > > + proxy_fops->splice_read = full_proxy_splice_read; > > if (real_fops->unlocked_ioctl) > > proxy_fops->unlocked_ioctl = full_proxy_unlocked_ioctl; > > } > shows it just doesn't work, and splicing always instantly returns empty > (subsequent reads actually return the contents). > > No-one noticed it became dead code in 2016, who knows if it worked back > then. Clearly no-one cares; just delete it. >
All checks out for me. How on earth did you notice this?
> --- a/kernel/relay.c > +++ b/kernel/relay.c > @@ -1073,167 +1073,6 @@ static ssize_t relay_file_read(struct file *filp, > return written; > } > > -static void relay_consume_bytes(struct rchan_buf *rbuf, int bytes_consumed)
And all this goop wasn't even inside #ifdef DEBUF_FS.
| |