sysfs: Improve read/write buffer filling/flushing semantics.

These are sysfs internal changes only, though they have positive external 
effects. 

A new data type (struct sysfs_buffer) is defined to describe and maintain
state of the data for an attribute file. This object is allocated and 
initialized when the file is opened. It contains a page-sized buffer that
is allocated during the first read or write. 

For read(), the buffer is filled when it is allocated by calling the 
attribute's show() method. This only happens when a read occurs from offset
0 in the file. For all read()s, the portion of the file requested is copied
to userspace. This finally fixes a long-standing bug in partial read()s.

For write(), the buffer is filled when it is allocated by copying the user-
supplied buffer from userspace into it. This buffer is then passed to the
attribute's store() method. This is not much different than what was present
before.

Note that partial-writes are not-supported. There is no way to know if a 
user process is doing a partial write, and has more buffer to write. So, we
take the easy route and assume that the entire buffer is passed during the
first write.

fs/sysfs/inode.c

@@ -183,6 +183,73 @@ static struct sysfs_ops subsys_sysfs_ops = {
 	.store	= subsys_attr_store,
 };
 
+
+struct sysfs_buffer {
+	size_t			count;
+	loff_t			pos;
+	char			* page;
+	struct sysfs_ops	* ops;
+};
+
+
+/**
+ *	fill_read_buffer - allocate and fill buffer from object.
+ *	@file:		file pointer.
+ *	@buffer:	data buffer for file.
+ *
+ *	Allocate @buffer->page, if it hasn't been already, then call the
+ *	kobject's show() method to fill the buffer with this attribute's 
+ *	data. 
+ *	This is called only once, on the file's first read. 
+ */
+static int fill_read_buffer(struct file * file, struct sysfs_buffer * buffer)
+{
+	struct attribute * attr = file->f_dentry->d_fsdata;
+	struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
+	struct sysfs_ops * ops = buffer->ops;
+	int ret = 0;
+	size_t count;
+
+	if (!buffer->page)
+		buffer->page = (char *) __get_free_page(GFP_KERNEL);
+	if (!buffer->page)
+		return -ENOMEM;
+
+	count = ops->show(kobj,attr,buffer->page,PAGE_SIZE,0);
+	if (count >= 0)
+		buffer->count = count;
+	else
+		ret = count;
+	return ret;
+}
+
+
+/**
+ *	flush_read_buffer - push buffer to userspace.
+ *	@buffer:	data buffer for file.
+ *	@userbuf:	user-passed buffer.
+ *	@count:		number of bytes requested.
+ *	@ppos:		file position.
+ *
+ *	Copy the buffer we filled in fill_read_buffer() to userspace.
+ *	This is done at the reader's leisure, copying and advancing 
+ *	the amount they specify each time.
+ *	This may be called continuously until the buffer is empty.
+ */
+static int flush_read_buffer(struct sysfs_buffer * buffer, char * buf, 
+			     size_t count, loff_t * ppos)
+{
+	int error;
+
+	if (count > (buffer->count - *ppos))
+		count = buffer->count - *ppos;
+
+	error = copy_to_user(buf,buffer->page + *ppos,count);
+	if (!error)
+		*ppos += count;
+	return error ? error : count;
+}
+
 /**
  *	sysfs_read_file - read an attribute. 
  *	@file:	file pointer.
@@ -192,55 +259,79 @@ static struct sysfs_ops subsys_sysfs_ops = {
  *
  *	Userspace wants to read an attribute file. The attribute descriptor
  *	is in the file's ->d_fsdata. The target object is in the directory's
- *	->d_fsdata. 
+ *	->d_fsdata.
  *
- *	We allocate a %PAGE_SIZE buffer, and pass it to the object's ->show()
- *	method (along with the object). We loop doing this until @count is 
- *	satisfied, or ->show() returns %0. 
+ *	We call fill_read_buffer() to allocate and fill the buffer from the
+ *	object's show() method exactly once (if the read is happening from
+ *	the beginning of the file). That should fill the entire buffer with
+ *	all the data the object has to offer for that attribute.
+ *	We then call flush_read_buffer() to copy the buffer to userspace
+ *	in the increments specified.
  */
 
 static ssize_t
 sysfs_read_file(struct file *file, char *buf, size_t count, loff_t *ppos)
 {
-	struct attribute * attr = file->f_dentry->d_fsdata;
-	struct sysfs_ops * ops = file->private_data;
-	struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
-	unsigned char *page;
+	struct sysfs_buffer * buffer = file->private_data;
 	ssize_t retval = 0;
 
-	if (count > PAGE_SIZE)
-		count = PAGE_SIZE;
+	if (!*ppos) {
+		if ((retval = fill_read_buffer(file,buffer)))
+			return retval;
+	}
+	pr_debug("%s: count = %d, ppos = %lld, buf = %s\n",
+		 __FUNCTION__,count,*ppos,buffer->page);
+	return flush_read_buffer(buffer,buf,count,ppos);
+}
 
-	page = (unsigned char*)__get_free_page(GFP_KERNEL);
-	if (!page)
+
+/**
+ *	fill_write_buffer - copy buffer from userspace.
+ *	@buffer:	data buffer for file.
+ *	@userbuf:	data from user.
+ *	@count:		number of bytes in @userbuf.
+ *
+ *	Allocate @buffer->page if it hasn't been already, then
+ *	copy the user-supplied buffer into it.
+ */
+
+static int 
+fill_write_buffer(struct sysfs_buffer * buffer, const char * buf, size_t count)
+{
+	int error;
+
+	if (!buffer->page)
+		buffer->page = (char *)__get_free_page(GFP_KERNEL);
+	if (!buffer->page)
 		return -ENOMEM;
 
-	while (count > 0) {
-		ssize_t len;
+	if (count >= PAGE_SIZE)
+		count = PAGE_SIZE - 1;
+	error = copy_from_user(buffer->page,buf,count);
+	return error ? error : count;
+}
 
-		len = ops->show(kobj,attr,page,count,*ppos);
 
-		if (len <= 0) {
-			if (len < 0)
-				retval = len;
-			break;
-		} else if (len > count)
-			len = count;
+/**
+ *	flush_write_buffer - push buffer to kobject.
+ *	@file:		file pointer.
+ *	@buffer:	data buffer for file.
+ *
+ *	Get the correct pointers for the kobject and the attribute we're
+ *	dealing with, then call the store() method for the attribute, 
+ *	passing the buffer that we acquired in fill_write_buffer().
+ */
 
-		if (copy_to_user(buf,page,len)) {
-			retval = -EFAULT;
-			break;
-		}
+static int flush_write_buffer(struct file * file, struct sysfs_buffer * buffer)
+{
+	struct attribute * attr = file->f_dentry->d_fsdata;
+	struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
+	struct sysfs_ops * ops = buffer->ops;
 
-		*ppos += len;
-		count -= len;
-		buf += len;
-		retval += len;
-	}
-	free_page((unsigned long)page);
-	return retval;
+	return ops->store(kobj,attr,buffer->page,PAGE_SIZE,0);
 }
 
+
 /**
  *	sysfs_write_file - write an attribute.
  *	@file:	file pointer
@@ -248,56 +339,34 @@ sysfs_read_file(struct file *file, char *buf, size_t count, loff_t *ppos)
  *	@count:	number of bytes
  *	@ppos:	starting offset
  *
- *	Identical to sysfs_read_file(), though going the opposite direction.
- *	We allocate a %PAGE_SIZE buffer and copy in the userspace buffer. We
- *	pass that to the object's ->store() method until we reach @count or 
- *	->store() returns %0 or less.
+ *	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. 
  */
 
 static ssize_t
 sysfs_write_file(struct file *file, const char *buf, size_t count, loff_t *ppos)
 {
-	struct attribute * attr = file->f_dentry->d_fsdata;
-	struct sysfs_ops * ops = file->private_data;
-	struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
-	ssize_t retval = 0;
-	char * page;
-
-
-	page = (char *)__get_free_page(GFP_KERNEL);
-	if (!page)
-		return -ENOMEM;
-
-	if (count >= PAGE_SIZE)
-		count = PAGE_SIZE - 1;
-	if (copy_from_user(page,buf,count))
-		goto done;
-	*(page + count) = '\0';
-
-	while (count > 0) {
-		ssize_t len;
-
-		len = ops->store(kobj,attr,page + retval,count,*ppos);
-
-		if (len <= 0) {
-			if (len < 0)
-				retval = len;
-			break;
-		}
-		retval += len;
-		count -= len;
-		*ppos += len;
-		buf += len;
-	}
- done:
-	free_page((unsigned long)page);
-	return retval;
+	struct sysfs_buffer * buffer = file->private_data;
+
+	count = fill_write_buffer(buffer,buf,count);
+	if (count > 0)
+		count = flush_write_buffer(file,buffer);
+	if (count > 0)
+		*ppos += count;
+	return count;
 }
 
 static int check_perm(struct inode * inode, struct file * file)
 {
 	struct kobject * kobj = kobject_get(file->f_dentry->d_parent->d_fsdata);
 	struct attribute * attr = file->f_dentry->d_fsdata;
+	struct sysfs_buffer * buffer;
 	struct sysfs_ops * ops = NULL;
 	int error = 0;
 
@@ -344,10 +413,16 @@ static int check_perm(struct inode * inode, struct file * file)
 			goto Eaccess;
 	}
 
-	/* No error? Great, store the ops in file->private_data
-	 * for easy access in the read/write functions.
+	/* No error? Great, allocate a buffer for the file, and store it
+	 * it in file->private_data for easy access.
 	 */
-	file->private_data = ops;
+	buffer = kmalloc(sizeof(struct sysfs_buffer),GFP_KERNEL);
+	if (buffer) {
+		memset(buffer,0,sizeof(struct sysfs_buffer));
+		buffer->ops = ops;
+		file->private_data = buffer;
+	} else
+		error = -ENOMEM;
 	goto Done;
 
  Einval:
@@ -359,6 +434,8 @@ static int check_perm(struct inode * inode, struct file * file)
  Eperm:
 	error = -EPERM;
  Done:
+	if (error && kobj)
+		kobject_put(kobj);
 	return error;
 }
 
@@ -370,8 +447,16 @@ static int sysfs_open_file(struct inode * inode, struct file * filp)
 static int sysfs_release(struct inode * inode, struct file * filp)
 {
 	struct kobject * kobj = filp->f_dentry->d_parent->d_fsdata;
+	struct sysfs_buffer * buffer = filp->private_data;
+
 	if (kobj) 
 		kobject_put(kobj);
+
+	if (buffer) {
+		if (buffer->page)
+			free_page((unsigned long)buffer->page);
+		kfree(buffer);
+	}
 	return 0;
 }