Messages in this thread | | | Date | Sun, 5 Oct 2014 16:02:18 -0400 | From | Tejun Heo <> | Subject | Re: [PATCH 2/2] sysfs/kernfs: make read requests on pre-alloc files use the buffer. |
| |
On Tue, Sep 30, 2014 at 12:33:34PM +1000, NeilBrown wrote: > To match the previous patch which used the pre-alloc buffer for > writes, this patch causes reads to use the same buffer. > This is not strictly necessary as the current seq_read() will allocate > on first read, so user-space can trigger the required pre-alloc. But > consistency is valuable. > > The read function is somewhat simpler than seq_read() and, for example, > does not support reading from an offset into the file: reads must be > at the start of the file. > > As the buffer is shared with writes and other reads, the mutex is > extended to cover the copy_to_user. > > Signed-off-by: NeilBrown <neilb@suse.de> > --- > fs/kernfs/file.c | 17 ++++++++++------- > fs/sysfs/file.c | 28 ++++++++++++++++++++++++---- > 2 files changed, 34 insertions(+), 11 deletions(-) > > diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c > index 73bd5ed143cd..7072240604f5 100644 > --- a/fs/kernfs/file.c > +++ b/fs/kernfs/file.c > @@ -189,7 +189,9 @@ static ssize_t kernfs_file_direct_read(struct kernfs_open_file *of, > const struct kernfs_ops *ops; > char *buf; > > - buf = kmalloc(len, GFP_KERNEL); > + buf = of->buf; > + if (!buf) > + buf = kmalloc(len, GFP_KERNEL); > if (!buf) > return -ENOMEM; > > @@ -210,21 +212,22 @@ static ssize_t kernfs_file_direct_read(struct kernfs_open_file *of, > else > len = -EINVAL; > > - kernfs_put_active(of->kn); > - mutex_unlock(&of->mutex); > - > if (len < 0) > - goto out_free; > + goto out_unlock; > > if (copy_to_user(user_buf, buf, len)) { > len = -EFAULT; > - goto out_free; > + goto out_unlock; > } > > *ppos += len; > > + out_unlock: > + kernfs_put_active(of->kn); > + mutex_unlock(&of->mutex); > out_free: > - kfree(buf); > + if (buf != of->buf) > + kfree(buf); > return len; > }
Can you please also make kernfs reject ->seq_show() if .prealloc is being used?
> +/* kernfs read callback for regular sysfs files with pre-alloc */ > +static ssize_t sysfs_kf_read(struct kernfs_open_file *of, char *buf, > + size_t count, loff_t pos) > +{ > + const struct sysfs_ops *ops = sysfs_file_ops(of->kn); > + struct kobject *kobj = of->kn->parent->priv; > + > + if (pos) > + return 0; > + return ops->show(kobj, of->kn->priv, buf);
Shouldn't it also memset?
Thanks.
-- tejun
| |