On Fri, Jan 21, 2005 at 02:52:29PM -0800, Mitch Williams wrote:
> This patch buffers writes to sysfs files and flushes them to the kobject
> owner when the file is closed.
Why? This breaks the way things work today, right?
What is this patch trying to fix?
>
> Generated from 2.6.11-rc1.
>
> Signed-off-by: Mitch Williams <[EMAIL PROTECTED]>
>
> diff -uprN -X dontdiff linux-2.6.11-clean/fs/sysfs/file.c
> linux-2.6.11/fs/sysfs/file.c
> --- linux-2.6.11-clean/fs/sysfs/file.c 2004-12-24 13:33:50.000000000
> -0800
> +++ linux-2.6.11/fs/sysfs/file.c 2005-01-21 13:09:21.000000000 -0800
> @@ -62,6 +62,7 @@ struct sysfs_buffer {
> struct sysfs_ops * ops;
> struct semaphore sem;
> int read_filled;
> + int needs_write_flush;
> };
>
>
> @@ -178,7 +178,8 @@ out:
> */
>
> static int
> -fill_write_buffer(struct sysfs_buffer * buffer, const char __user * buf,
> size_t count)
> +fill_write_buffer(struct sysfs_buffer *buffer, const char __user * buf,
> + size_t count, size_t pos)
> {
> int error;
>
> @@ -187,10 +189,11 @@ fill_write_buffer(struct sysfs_buffer *
> if (!buffer->page)
> return -ENOMEM;
>
> - if (count >= PAGE_SIZE)
> - count = PAGE_SIZE - 1;
> + if (count + pos > PAGE_SIZE)
> + count = (PAGE_SIZE - 1) - pos;
> error = copy_from_user(buffer->page,buf,count);
> - buffer->needs_read_fill = 1;
> + buffer->needs_write_flush = 1;
> + buffer->count = pos + count;
> return error ? -EFAULT : count;
> }
>
> @@ -206,13 +209,13 @@ fill_write_buffer(struct sysfs_buffer *
> */
>
> static int
> -flush_write_buffer(struct dentry * dentry, struct sysfs_buffer * buffer,
> size_t count)
> +flush_write_buffer(struct dentry * dentry, struct sysfs_buffer * buffer)
> {
> struct attribute * attr = to_attr(dentry);
> struct kobject * kobj = to_kobj(dentry->d_parent);
> struct sysfs_ops * ops = buffer->ops;
>
> - return ops->store(kobj,attr,buffer->page,count);
> + return ops->store(kobj,attr, buffer->page, buffer->count);
> }
>
>
> @@ -225,12 +228,9 @@ flush_write_buffer(struct dentry * dentr
> *
> * Similar to sysfs_read_file(), though working in the opposite direction.
> * We allocate and fill the data from the user in fill_write_buffer(),
> - * then push it to the kobject in flush_write_buffer().
> - * There is no easy way for us to know if userspace is only doing a partial
> - * write, so we don't support them. We expect the entire buffer to come
> - * on the first write.
> - * Hint: if you're writing a value, first read the file, modify only the
> - * the value you're changing, then write entire buffer back.
> + * but don't push it to the kobject until the file is closed.
> + * This allows for buffered writes, but unfortunately also hides error
> + * codes returned individual store functions until close time.
> */
>
> static ssize_t
> @@ -238,10 +238,10 @@ sysfs_write_file(struct file *file, cons
> {
> struct sysfs_buffer * buffer = file->private_data;
>
> + if (*ppos >= PAGE_SIZE)
> + return -ENOSPC;
> down(&buffer->sem);
> - count = fill_write_buffer(buffer,buf,count);
> - if (count > 0)
> - count = flush_write_buffer(file->f_dentry,buffer,count);
> + count = fill_write_buffer(buffer, buf, count, *ppos);
> if (count > 0)
> *ppos += count;
> up(&buffer->sem);
> @@ -337,6 +337,17 @@ static int sysfs_release(struct inode *
> struct attribute * attr = to_attr(filp->f_dentry);
> struct module * owner = attr->owner;
> struct sysfs_buffer * buffer = filp->private_data;
> + int ret;
You used spaces instead of a tab here.
> +
> + /* If data has been written to the file, then flush it
> + * back to the kobject's store function here.
> + */
> + if (buffer && kobj) {
> + down(&buffer->sem);
> + if (buffer->needs_write_flush)
> + ret = flush_write_buffer(filp->f_dentry, buffer);
> + up(&buffer->sem);
> + }
>
> if (kobj)
> kobject_put(kobj);
> @@ -348,7 +352,10 @@ static int sysfs_release(struct inode *
> free_page((unsigned long)buffer->page);
> kfree(buffer);
> }
> - return 0;
> + /* If flush_write_buffer returned an error, pass it up.
> + * Otherwise, return success.
> + */
> + return (ret < 0 ? ret : 0);
I think this comment is not needed. Actually, what's wrong with just:
return ret;
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
