Re: PROBLEM: Firmware loader fallback mechanism no longer works with sendfile

Siddharth Gupta
2021-01-20T20:35:42Z

On 1/20/2021 1:10 AM, Christoph Hellwig wrote:
> Can you give this patch a spin?
Thanks! This patch fixed the fallback mechanism for me. Attaching logs:

[   84.410162][  T244] qcom_q6v5_pas xxxxxxxx.remoteproc-cdsp: Direct
firmware load for cdsp.bX failed with error -2
[   84.418276][  T244] qcom_q6v5_pas xxxxxxxx.remoteproc-cdsp: Falling
back to sysfs fallback for: cdsp.bX
[   84.471558][  T393] ueventd: firmware: loading 'cdsp.bX' for
'/devices/platform/soc/xxxxxxxx.remoteproc-cdsp/firmware/cdsp.bX'
[   84.491936][  T393] ueventd: loading
/devices/platform/soc/xxxxxxxx.remoteproc-cdsp/firmware/cdsp.bX took 22ms
[  103.331486][  T244] remoteproc remoteproc1: remote processor
xxxxxxxx.remoteproc-cdsp is now up

- Sid
>
>
> diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
> index f277d023ebcd14..4b5833b3059f9c 100644
> --- a/fs/kernfs/file.c
> +++ b/fs/kernfs/file.c
> @@ -14,6 +14,7 @@
> #include <linux/pagemap.h>
> #include <linux/sched/mm.h>
> #include <linux/fsnotify.h>
> +#include <linux/uio.h>
>
> #include "kernfs-internal.h"
>
> @@ -180,11 +181,10 @@ static const struct seq_operations kernfs_seq_ops = {
> * it difficult to use seq_file. Implement simplistic custom buffering for
> * bin files.
> */
> -static ssize_t kernfs_file_direct_read(struct kernfs_open_file *of,
> - char __user *user_buf, size_t count,
> - loff_t *ppos)
> +static ssize_t kernfs_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
> {
> - ssize_t len = min_t(size_t, count, PAGE_SIZE);
> + struct kernfs_open_file *of = kernfs_of(iocb->ki_filp);
> + ssize_t len = min_t(size_t, iov_iter_count(iter), PAGE_SIZE);
> const struct kernfs_ops *ops;
> char *buf;
>
> @@ -210,7 +210,7 @@ static ssize_t kernfs_file_direct_read(struct kernfs_open_file *of,
> of->event = atomic_read(&of->kn->attr.open->event);
> ops = kernfs_ops(of->kn);
> if (ops->read)
> - len = ops->read(of, buf, len, *ppos);
> + len = ops->read(of, buf, len, iocb->ki_pos);
> else
> len = -EINVAL;
>
> @@ -220,12 +220,12 @@ static ssize_t kernfs_file_direct_read(struct kernfs_open_file *of,
> if (len < 0)
> goto out_free;
>
> - if (copy_to_user(user_buf, buf, len)) {
> + if (copy_to_iter(buf, len, iter) != len) {
> len = -EFAULT;
> goto out_free;
> }
>
> - *ppos += len;
> + iocb->ki_pos += len;
>
> out_free:
> if (buf == of->prealloc_buf)
> @@ -235,31 +235,14 @@ static ssize_t kernfs_file_direct_read(struct kernfs_open_file *of,
> return len;
> }
>
> -/**
> - * kernfs_fop_read - kernfs vfs read callback
> - * @file: file pointer
> - * @user_buf: data to write
> - * @count: number of bytes
> - * @ppos: starting offset
> - */
> -static ssize_t kernfs_fop_read(struct file *file, char __user *user_buf,
> - size_t count, loff_t *ppos)
> +static ssize_t kernfs_fop_read_iter(struct kiocb *iocb, struct iov_iter *iter)
> {
> - struct kernfs_open_file *of = kernfs_of(file);
> -
> - if (of->kn->flags & KERNFS_HAS_SEQ_SHOW)
> - return seq_read(file, user_buf, count, ppos);
> - else
> - return kernfs_file_direct_read(of, user_buf, count, ppos);
> + if (kernfs_of(iocb->ki_filp)->kn->flags & KERNFS_HAS_SEQ_SHOW)
> + return seq_read_iter(iocb, iter);
> + return kernfs_file_read_iter(iocb, iter);
> }
>
> -/**
> - * kernfs_fop_write - kernfs vfs write callback
> - * @file: file pointer
> - * @user_buf: data to write
> - * @count: number of bytes
> - * @ppos: starting offset
> - *
> +/*
> * Copy data in from userland and pass it to the matching kernfs write
> * operation.
> *
> @@ -269,20 +252,18 @@ static ssize_t kernfs_fop_read(struct file *file, char __user *user_buf,
> * modify only the the value you're changing, then write entire buffer
> * back.
> */
> -static ssize_t kernfs_fop_write(struct file *file, const char __user *user_buf,
> - size_t count, loff_t *ppos)
> +static ssize_t kernfs_fop_write_iter(struct kiocb *iocb, struct iov_iter *iter)
> {
> - struct kernfs_open_file *of = kernfs_of(file);
> + struct kernfs_open_file *of = kernfs_of(iocb->ki_filp);
> + ssize_t len = iov_iter_count(iter);
> const struct kernfs_ops *ops;
> - ssize_t len;
> char *buf;
>
> if (of->atomic_write_len) {
> - len = count;
> if (len > of->atomic_write_len)
> return -E2BIG;
> } else {
> - len = min_t(size_t, count, PAGE_SIZE);
> + len = min_t(size_t, len, PAGE_SIZE);
> }
>
> buf = of->prealloc_buf;
> @@ -293,7 +274,7 @@ static ssize_t kernfs_fop_write(struct file *file, const char __user *user_buf,
> if (!buf)
> return -ENOMEM;
>
> - if (copy_from_user(buf, user_buf, len)) {
> + if (copy_from_iter(buf, len, iter) != len) {
> len = -EFAULT;
> goto out_free;
> }
> @@ -312,7 +293,7 @@ static ssize_t kernfs_fop_write(struct file *file, const char __user *user_buf,
>
> ops = kernfs_ops(of->kn);
> if (ops->write)
> - len = ops->write(of, buf, len, *ppos);
> + len = ops->write(of, buf, len, iocb->ki_pos);
> else
> len = -EINVAL;
>
> @@ -320,7 +301,7 @@ static ssize_t kernfs_fop_write(struct file *file, const char __user *user_buf,
> mutex_unlock(&of->mutex);
>
> if (len > 0)
> - *ppos += len;
> + iocb->ki_pos += len;
>
> out_free:
> if (buf == of->prealloc_buf)
> @@ -960,14 +941,16 @@ void kernfs_notify(struct kernfs_node *kn)
> EXPORT_SYMBOL_GPL(kernfs_notify);
>
> const struct file_operations kernfs_file_fops = {
> - .read = kernfs_fop_read,
> - .write = kernfs_fop_write,
> + .read_iter = kernfs_fop_read_iter,
> + .write_iter = kernfs_fop_write_iter,
> .llseek = generic_file_llseek,
> .mmap = kernfs_fop_mmap,
> .open = kernfs_fop_open,
> .release = kernfs_fop_release,
> .poll = kernfs_fop_poll,
> .fsync = noop_fsync,
> + .splice_read = generic_file_splice_read,
> + .splice_write = iter_file_splice_write,
> };
>
> /**