public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: gregkh@linuxfoundation.org
Cc: kay@vrfy.org, linux-kernel@vger.kernel.org,
	ebiederm@xmission.com, bhelgaas@google.com,
	Tejun Heo <tj@kernel.org>
Subject: [PATCH 02/15] sysfs: remove sysfs_buffer->needs_read_fill
Date: Tue,  1 Oct 2013 17:41:56 -0400	[thread overview]
Message-ID: <1380663729-18243-3-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1380663729-18243-1-git-send-email-tj@kernel.org>

->needs_read_fill is used to implement the following behaviors.

1. Ensure buffer filling on the first read.
2. Force buffer filling after a write.
3. Force buffer filling after a successful poll.

However, #2 and #3 don't really work as sysfs doesn't reset file
position.  While the read buffer would be refilled, the next read
would continue from the position after the last read or write,
requiring an explicit seek to the start for it to be useful, which
makes ->needs_read_fill superflous as read buffer is always refilled
if f_pos == 0.

Update sysfs_read_file() to test buffer->page for #1 instead and
remove ->needs_read_fill.  While this changes behavior in extreme
corner cases - e.g. re-reading a sysfs file after seeking to non-zero
position after a write or poll, it's highly unlikely to lead to actual
breakage.  This change is to prepare for using seq_file in the read
path.

While at it, reformat a comment in fill_write_buffer().

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Kay Sievers <kay@vrfy.org>
---
 fs/sysfs/file.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 81e3f72..e2fafc0 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -47,7 +47,6 @@ struct sysfs_buffer {
 	char			*page;
 	const struct sysfs_ops	*ops;
 	struct mutex		mutex;
-	int			needs_read_fill;
 	int			event;
 	struct list_head	list;
 };
@@ -95,12 +94,10 @@ static int fill_read_buffer(struct dentry *dentry, struct sysfs_buffer *buffer)
 		/* Try to struggle along */
 		count = PAGE_SIZE - 1;
 	}
-	if (count >= 0) {
-		buffer->needs_read_fill = 0;
+	if (count >= 0)
 		buffer->count = count;
-	} else {
+	else
 		ret = count;
-	}
 	return ret;
 }
 
@@ -130,7 +127,11 @@ sysfs_read_file(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	ssize_t retval = 0;
 
 	mutex_lock(&buffer->mutex);
-	if (buffer->needs_read_fill || *ppos == 0) {
+	/*
+	 * Fill on zero offset and the first read so that silly things like
+	 * "dd bs=1 skip=N" can work on sysfs files.
+	 */
+	if (*ppos == 0 || !buffer->page) {
 		retval = fill_read_buffer(file->f_path.dentry, buffer);
 		if (retval)
 			goto out;
@@ -166,14 +167,15 @@ static int fill_write_buffer(struct sysfs_buffer *buffer,
 	if (count >= PAGE_SIZE)
 		count = PAGE_SIZE - 1;
 	error = copy_from_user(buffer->page, buf, count);
-	buffer->needs_read_fill = 1;
-	/* if buf is assumed to contain a string, terminate it by \0,
-	   so e.g. sscanf() can scan the string easily */
+
+	/*
+	 * If buf is assumed to contain a string, terminate it by \0, so
+	 * e.g. sscanf() can scan the string easily.
+	 */
 	buffer->page[count] = 0;
 	return error ? -EFAULT : count;
 }
 
-
 /**
  *	flush_write_buffer - push buffer to kobject.
  *	@dentry:	dentry to the attribute
@@ -368,7 +370,6 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
 		goto err_out;
 
 	mutex_init(&buffer->mutex);
-	buffer->needs_read_fill = 1;
 	buffer->ops = ops;
 	file->private_data = buffer;
 
@@ -435,7 +436,6 @@ static unsigned int sysfs_poll(struct file *filp, poll_table *wait)
 	return DEFAULT_POLLMASK;
 
  trigger:
-	buffer->needs_read_fill = 1;
 	return DEFAULT_POLLMASK|POLLERR|POLLPRI;
 }
 
-- 
1.8.3.1


  parent reply	other threads:[~2013-10-01 21:46 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-01 21:41 [PATCHSET v2] sysfs: use seq_file and unify regular and bin file handling Tejun Heo
2013-10-01 21:41 ` [PATCH 01/15] sysfs: remove unused sysfs_buffer->pos Tejun Heo
2013-10-01 21:41 ` Tejun Heo [this message]
2013-10-01 21:41 ` [PATCH 03/15] sysfs: remove sysfs_buffer->ops Tejun Heo
2013-10-01 21:41 ` [PATCH 04/15] sysfs: add sysfs_open_file_mutex Tejun Heo
2013-10-01 21:41 ` [PATCH 05/15] sysfs: rename sysfs_buffer to sysfs_open_file Tejun Heo
2013-10-01 21:42 ` [PATCH 06/15] sysfs: add sysfs_open_file->sd and ->file Tejun Heo
2013-10-01 21:42 ` [PATCH 07/15] sysfs: use transient write buffer Tejun Heo
2013-10-01 21:42 ` [PATCH 08/15] sysfs: use seq_file when reading regular files Tejun Heo
2013-10-01 21:42 ` [PATCH 09/15] sysfs: skip bin_buffer->buffer while reading Tejun Heo
2013-10-01 21:42 ` [PATCH 10/15] sysfs: collapse fs/sysfs/bin.c::fill_read() into read() Tejun Heo
2013-10-01 21:42 ` [PATCH 11/15] sysfs: prepare path write for unified regular / bin file handling Tejun Heo
2013-10-01 21:42 ` [PATCH 12/15] sysfs: add sysfs_bin_read() Tejun Heo
2013-10-01 21:42 ` [PATCH 13/15] sysfs: copy bin mmap support from fs/sysfs/bin.c to fs/sysfs/file.c Tejun Heo
2013-10-01 21:42 ` [PATCH 14/15] sysfs: prepare open path for unified regular / bin file handling Tejun Heo
2013-10-01 21:42 ` [PATCH 15/15] sysfs: merge regular and " Tejun Heo
2013-10-06  0:40 ` [PATCHSET v2] sysfs: use seq_file and unify " Greg KH
2013-10-07 17:00   ` Tejun Heo
2013-10-07 23:11     ` Greg KH
2013-10-11 19:11       ` Yinghai Lu
2013-10-11 20:49         ` Greg KH
2013-10-14 13:27           ` [PATCH driver-core-next] sysfs: make sysfs_file_ops() follow ignore_lockdep flag Tejun Heo
2013-10-14 18:41             ` Yinghai Lu
2013-10-14 12:47         ` [PATCHSET v2] sysfs: use seq_file and unify regular and bin file handling Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1380663729-18243-3-git-send-email-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=ebiederm@xmission.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kay@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox