lkml.org 
[lkml]   [2005]   [Jan]   [22]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
    /
    DateSat, 22 Jan 2005 00:09:30 -0800
    FromGreg KH <>
    SubjectRe: [PATCH 2/3] buffer writes to sysfs
    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 <mitch.a.williams@intel.com>
    > 
    > 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 majordomo@vger.kernel.org
    More majordomo info at  http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at  http://www.tux.org/lkml/
    
    
    \
     
     \ /
      Last update: 2005-03-22 14:09    [from the cache]
    ©2003-2008