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.
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
---
kernel/relay.c | 162 -------------------------------------------------
1 file changed, 162 deletions(-)
diff --git a/kernel/relay.c b/kernel/relay.c
index 83fe0325cde1..a8e90e98bf2c 100644
--- 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)
-{
- rbuf->bytes_consumed += bytes_consumed;
-
- if (rbuf->bytes_consumed >= rbuf->chan->subbuf_size) {
- relay_subbufs_consumed(rbuf->chan, rbuf->cpu, 1);
- rbuf->bytes_consumed %= rbuf->chan->subbuf_size;
- }
-}
-
-static void relay_pipe_buf_release(struct pipe_inode_info *pipe,
- struct pipe_buffer *buf)
-{
- struct rchan_buf *rbuf;
-
- rbuf = (struct rchan_buf *)page_private(buf->page);
- relay_consume_bytes(rbuf, buf->private);
-}
-
-static const struct pipe_buf_operations relay_pipe_buf_ops = {
- .release = relay_pipe_buf_release,
- .try_steal = generic_pipe_buf_try_steal,
- .get = generic_pipe_buf_get,
-};
-
-static void relay_page_release(struct splice_pipe_desc *spd, unsigned int i)
-{
-}
-
-/*
- * subbuf_splice_actor - splice up to one subbuf's worth of data
- */
-static ssize_t subbuf_splice_actor(struct file *in,
- loff_t *ppos,
- struct pipe_inode_info *pipe,
- size_t len,
- unsigned int flags,
- int *nonpad_ret)
-{
- unsigned int pidx, poff, total_len, subbuf_pages, nr_pages;
- struct rchan_buf *rbuf = in->private_data;
- unsigned int subbuf_size = rbuf->chan->subbuf_size;
- uint64_t pos = (uint64_t) *ppos;
- uint32_t alloc_size = (uint32_t) rbuf->chan->alloc_size;
- size_t read_start = (size_t) do_div(pos, alloc_size);
- size_t read_subbuf = read_start / subbuf_size;
- size_t padding = rbuf->padding[read_subbuf];
- size_t nonpad_end = read_subbuf * subbuf_size + subbuf_size - padding;
- struct page *pages[PIPE_DEF_BUFFERS];
- struct partial_page partial[PIPE_DEF_BUFFERS];
- struct splice_pipe_desc spd = {
- .pages = pages,
- .nr_pages = 0,
- .nr_pages_max = PIPE_DEF_BUFFERS,
- .partial = partial,
- .ops = &relay_pipe_buf_ops,
- .spd_release = relay_page_release,
- };
- ssize_t ret;
-
- if (rbuf->subbufs_produced == rbuf->subbufs_consumed)
- return 0;
- if (splice_grow_spd(pipe, &spd))
- return -ENOMEM;
-
- /*
- * Adjust read len, if longer than what is available
- */
- if (len > (subbuf_size - read_start % subbuf_size))
- len = subbuf_size - read_start % subbuf_size;
-
- subbuf_pages = rbuf->chan->alloc_size >> PAGE_SHIFT;
- pidx = (read_start / PAGE_SIZE) % subbuf_pages;
- poff = read_start & ~PAGE_MASK;
- nr_pages = min_t(unsigned int, subbuf_pages, spd.nr_pages_max);
-
- for (total_len = 0; spd.nr_pages < nr_pages; spd.nr_pages++) {
- unsigned int this_len, this_end, private;
- unsigned int cur_pos = read_start + total_len;
-
- if (!len)
- break;
-
- this_len = min_t(unsigned long, len, PAGE_SIZE - poff);
- private = this_len;
-
- spd.pages[spd.nr_pages] = rbuf->page_array[pidx];
- spd.partial[spd.nr_pages].offset = poff;
-
- this_end = cur_pos + this_len;
- if (this_end >= nonpad_end) {
- this_len = nonpad_end - cur_pos;
- private = this_len + padding;
- }
- spd.partial[spd.nr_pages].len = this_len;
- spd.partial[spd.nr_pages].private = private;
-
- len -= this_len;
- total_len += this_len;
- poff = 0;
- pidx = (pidx + 1) % subbuf_pages;
-
- if (this_end >= nonpad_end) {
- spd.nr_pages++;
- break;
- }
- }
-
- ret = 0;
- if (!spd.nr_pages)
- goto out;
-
- ret = *nonpad_ret = splice_to_pipe(pipe, &spd);
- if (ret < 0 || ret < total_len)
- goto out;
-
- if (read_start + ret == nonpad_end)
- ret += padding;
-
-out:
- splice_shrink_spd(&spd);
- return ret;
-}
-
-static ssize_t relay_file_splice_read(struct file *in,
- loff_t *ppos,
- struct pipe_inode_info *pipe,
- size_t len,
- unsigned int flags)
-{
- ssize_t spliced;
- int ret;
- int nonpad_ret = 0;
-
- ret = 0;
- spliced = 0;
-
- while (len && !spliced) {
- ret = subbuf_splice_actor(in, ppos, pipe, len, flags, &nonpad_ret);
- if (ret < 0)
- break;
- else if (!ret) {
- if (flags & SPLICE_F_NONBLOCK)
- ret = -EAGAIN;
- break;
- }
-
- *ppos += ret;
- if (ret > len)
- len = 0;
- else
- len -= ret;
- spliced += nonpad_ret;
- nonpad_ret = 0;
- }
-
- if (spliced)
- return spliced;
-
- return ret;
-}
const struct file_operations relay_file_operations = {
.open = relay_file_open,
@@ -1242,6 +1081,5 @@ const struct file_operations relay_file_operations = {
.read = relay_file_read,
.llseek = no_llseek,
.release = relay_file_release,
- .splice_read = relay_file_splice_read,
};
EXPORT_SYMBOL_GPL(relay_file_operations);
--
2.39.2
On Tue, 19 Dec 2023 23:24:14 +0100 Ahelenia Ziemiańska <[email protected]> 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.
On Tue, Dec 19, 2023 at 04:35:54PM -0800, Andrew Morton wrote:
> On Tue, 19 Dec 2023 23:24:14 +0100 Ahelenia Ziemiańska <[email protected]> wrote:
> > 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?
Trying to explicitly trigger every .splice_read callback to test patches for
https://nabijaczleweli.xyz/content/blogn_t/011-linux-splice-exclusion.html