* [PATCH 01/14] sysfs: remove unused sysfs_buffer->pos
2013-09-28 21:49 [PATCHSET] sysfs: use seq_file and unify regular and bin file handling Tejun Heo
@ 2013-09-28 21:49 ` Tejun Heo
2013-09-28 21:49 ` [PATCH 02/14] sysfs: remove sysfs_buffer->needs_read_fill Tejun Heo
` (13 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2013-09-28 21:49 UTC (permalink / raw)
To: gregkh; +Cc: kay, linux-kernel, ebiederm, Tejun Heo
Signed-off-by: Tejun Heo <tj@kernel.org>
---
fs/sysfs/file.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 1656a79..81e3f72 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -44,7 +44,6 @@ struct sysfs_open_dirent {
struct sysfs_buffer {
size_t count;
- loff_t pos;
char *page;
const struct sysfs_ops *ops;
struct mutex mutex;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 02/14] sysfs: remove sysfs_buffer->needs_read_fill
2013-09-28 21:49 [PATCHSET] sysfs: use seq_file and unify regular and bin file handling Tejun Heo
2013-09-28 21:49 ` [PATCH 01/14] sysfs: remove unused sysfs_buffer->pos Tejun Heo
@ 2013-09-28 21:49 ` Tejun Heo
2013-09-28 21:49 ` [PATCH 03/14] sysfs: remove sysfs_buffer->ops Tejun Heo
` (12 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2013-09-28 21:49 UTC (permalink / raw)
To: gregkh; +Cc: kay, linux-kernel, ebiederm, Tejun Heo
->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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 03/14] sysfs: remove sysfs_buffer->ops
2013-09-28 21:49 [PATCHSET] sysfs: use seq_file and unify regular and bin file handling Tejun Heo
2013-09-28 21:49 ` [PATCH 01/14] sysfs: remove unused sysfs_buffer->pos Tejun Heo
2013-09-28 21:49 ` [PATCH 02/14] sysfs: remove sysfs_buffer->needs_read_fill Tejun Heo
@ 2013-09-28 21:49 ` Tejun Heo
2013-09-28 21:49 ` [PATCH 04/14] sysfs: add sysfs_open_file_mutex Tejun Heo
` (11 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2013-09-28 21:49 UTC (permalink / raw)
To: gregkh; +Cc: kay, linux-kernel, ebiederm, Tejun Heo
Currently, sysfs_ops is fetched during sysfs_open_file() and cached in
sysfs_buffer->ops to be used while the file is open. This patch
removes the caching and makes each operation directly fetch sysfs_ops.
This patch doesn't introduce any behavior difference and is to prepare
for merging regular and bin file supports.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
fs/sysfs/file.c | 33 +++++++++++++++++++++------------
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index e2fafc0..7dfcc33 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -45,12 +45,23 @@ struct sysfs_open_dirent {
struct sysfs_buffer {
size_t count;
char *page;
- const struct sysfs_ops *ops;
struct mutex mutex;
int event;
struct list_head list;
};
+/*
+ * Determine ktype->sysfs_ops for the given sysfs_dirent. This function
+ * must be called while holding an active reference.
+ */
+static const struct sysfs_ops *sysfs_file_ops(struct sysfs_dirent *sd)
+{
+ struct kobject *kobj = sd->s_parent->s_dir.kobj;
+
+ lockdep_assert_held(sd);
+ return kobj->ktype ? kobj->ktype->sysfs_ops : NULL;
+}
+
/**
* fill_read_buffer - allocate and fill buffer from object.
* @dentry: dentry pointer.
@@ -66,7 +77,7 @@ static int fill_read_buffer(struct dentry *dentry, struct sysfs_buffer *buffer)
{
struct sysfs_dirent *attr_sd = dentry->d_fsdata;
struct kobject *kobj = attr_sd->s_parent->s_dir.kobj;
- const struct sysfs_ops *ops = buffer->ops;
+ const struct sysfs_ops *ops;
int ret = 0;
ssize_t count;
@@ -80,6 +91,8 @@ static int fill_read_buffer(struct dentry *dentry, struct sysfs_buffer *buffer)
return -ENODEV;
buffer->event = atomic_read(&attr_sd->s_attr.open->event);
+
+ ops = sysfs_file_ops(attr_sd);
count = ops->show(kobj, attr_sd->s_attr.attr, buffer->page);
sysfs_put_active(attr_sd);
@@ -191,13 +204,14 @@ static int flush_write_buffer(struct dentry *dentry,
{
struct sysfs_dirent *attr_sd = dentry->d_fsdata;
struct kobject *kobj = attr_sd->s_parent->s_dir.kobj;
- const struct sysfs_ops *ops = buffer->ops;
+ const struct sysfs_ops *ops;
int rc;
/* need attr_sd for attr and ops, its parent for kobj */
if (!sysfs_get_active(attr_sd))
return -ENODEV;
+ ops = sysfs_file_ops(attr_sd);
rc = ops->store(kobj, attr_sd->s_attr.attr, buffer->page, count);
sysfs_put_active(attr_sd);
@@ -205,7 +219,6 @@ static int flush_write_buffer(struct dentry *dentry,
return rc;
}
-
/**
* sysfs_write_file - write an attribute.
* @file: file pointer
@@ -334,14 +347,11 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
return -ENODEV;
/* every kobject with an attribute needs a ktype assigned */
- if (kobj->ktype && kobj->ktype->sysfs_ops)
- ops = kobj->ktype->sysfs_ops;
- else {
- WARN(1, KERN_ERR
- "missing sysfs attribute operations for kobject: %s\n",
- kobject_name(kobj));
+ ops = sysfs_file_ops(attr_sd);
+ if (WARN(!ops, KERN_ERR
+ "missing sysfs attribute operations for kobject: %s\n",
+ kobject_name(kobj)))
goto err_out;
- }
/* File needs write support.
* The inode's perms must say it's ok,
@@ -370,7 +380,6 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
goto err_out;
mutex_init(&buffer->mutex);
- buffer->ops = ops;
file->private_data = buffer;
/* make sure we have open dirent struct */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 04/14] sysfs: add sysfs_open_file_mutex
2013-09-28 21:49 [PATCHSET] sysfs: use seq_file and unify regular and bin file handling Tejun Heo
` (2 preceding siblings ...)
2013-09-28 21:49 ` [PATCH 03/14] sysfs: remove sysfs_buffer->ops Tejun Heo
@ 2013-09-28 21:49 ` Tejun Heo
2013-09-28 21:49 ` [PATCH 05/14] sysfs: rename sysfs_buffer to sysfs_open_file Tejun Heo
` (10 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2013-09-28 21:49 UTC (permalink / raw)
To: gregkh; +Cc: kay, linux-kernel, ebiederm, Tejun Heo
Add a separate mutex to protect sysfs_open_dirent->buffers list. This
will allow performing sleepable operations while traversing
sysfs_buffers, which will be renamed to sysfs_open_file.
Note that currently sysfs_open_dirent->buffers list isn't being used
for anything and this patch doesn't make any functional difference.
It will be used to merge regular and bin file supports.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
fs/sysfs/file.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 7dfcc33..499cff8 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -25,15 +25,17 @@
#include "sysfs.h"
/*
- * There's one sysfs_buffer for each open file and one
- * sysfs_open_dirent for each sysfs_dirent with one or more open
- * files.
+ * There's one sysfs_buffer for each open file and one sysfs_open_dirent
+ * for each sysfs_dirent with one or more open files.
*
- * filp->private_data points to sysfs_buffer and
- * sysfs_dirent->s_attr.open points to sysfs_open_dirent. s_attr.open
- * is protected by sysfs_open_dirent_lock.
+ * sysfs_dirent->s_attr.open points to sysfs_open_dirent. s_attr.open is
+ * protected by sysfs_open_dirent_lock.
+ *
+ * filp->private_data points to sysfs_buffer which is chained at
+ * sysfs_open_dirent->buffers, which is protected by sysfs_open_file_mutex.
*/
static DEFINE_SPINLOCK(sysfs_open_dirent_lock);
+static DEFINE_MUTEX(sysfs_open_file_mutex);
struct sysfs_open_dirent {
atomic_t refcnt;
@@ -272,6 +274,7 @@ static int sysfs_get_open_dirent(struct sysfs_dirent *sd,
struct sysfs_open_dirent *od, *new_od = NULL;
retry:
+ mutex_lock(&sysfs_open_file_mutex);
spin_lock_irq(&sysfs_open_dirent_lock);
if (!sd->s_attr.open && new_od) {
@@ -286,6 +289,7 @@ static int sysfs_get_open_dirent(struct sysfs_dirent *sd,
}
spin_unlock_irq(&sysfs_open_dirent_lock);
+ mutex_unlock(&sysfs_open_file_mutex);
if (od) {
kfree(new_od);
@@ -321,6 +325,7 @@ static void sysfs_put_open_dirent(struct sysfs_dirent *sd,
struct sysfs_open_dirent *od = sd->s_attr.open;
unsigned long flags;
+ mutex_lock(&sysfs_open_file_mutex);
spin_lock_irqsave(&sysfs_open_dirent_lock, flags);
list_del(&buffer->list);
@@ -330,6 +335,7 @@ static void sysfs_put_open_dirent(struct sysfs_dirent *sd,
od = NULL;
spin_unlock_irqrestore(&sysfs_open_dirent_lock, flags);
+ mutex_unlock(&sysfs_open_file_mutex);
kfree(od);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 05/14] sysfs: rename sysfs_buffer to sysfs_open_file
2013-09-28 21:49 [PATCHSET] sysfs: use seq_file and unify regular and bin file handling Tejun Heo
` (3 preceding siblings ...)
2013-09-28 21:49 ` [PATCH 04/14] sysfs: add sysfs_open_file_mutex Tejun Heo
@ 2013-09-28 21:49 ` Tejun Heo
2013-09-28 21:49 ` [PATCH 06/14] sysfs: add sysfs_open_file->sd and ->file Tejun Heo
` (9 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2013-09-28 21:49 UTC (permalink / raw)
To: gregkh; +Cc: kay, linux-kernel, ebiederm, Tejun Heo
sysfs read path will be converted to use seq_file which will handle
buffering making sysfs_buffer a misnomer. Rename sysfs_buffer to
sysfs_open_file, and sysfs_open_dirent->buffers to ->files.
This path is pure rename.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
fs/sysfs/file.c | 127 ++++++++++++++++++++++++++++----------------------------
1 file changed, 63 insertions(+), 64 deletions(-)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 499cff8..4b55bcf 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -25,14 +25,14 @@
#include "sysfs.h"
/*
- * There's one sysfs_buffer for each open file and one sysfs_open_dirent
+ * There's one sysfs_open_file for each open file and one sysfs_open_dirent
* for each sysfs_dirent with one or more open files.
*
* sysfs_dirent->s_attr.open points to sysfs_open_dirent. s_attr.open is
* protected by sysfs_open_dirent_lock.
*
- * filp->private_data points to sysfs_buffer which is chained at
- * sysfs_open_dirent->buffers, which is protected by sysfs_open_file_mutex.
+ * filp->private_data points to sysfs_open_file which is chained at
+ * sysfs_open_dirent->files, which is protected by sysfs_open_file_mutex.
*/
static DEFINE_SPINLOCK(sysfs_open_dirent_lock);
static DEFINE_MUTEX(sysfs_open_file_mutex);
@@ -41,10 +41,10 @@ struct sysfs_open_dirent {
atomic_t refcnt;
atomic_t event;
wait_queue_head_t poll;
- struct list_head buffers; /* goes through sysfs_buffer.list */
+ struct list_head files; /* goes through sysfs_open_file.list */
};
-struct sysfs_buffer {
+struct sysfs_open_file {
size_t count;
char *page;
struct mutex mutex;
@@ -75,7 +75,7 @@ static const struct sysfs_ops *sysfs_file_ops(struct sysfs_dirent *sd)
* This is called only once, on the file's first read unless an error
* is returned.
*/
-static int fill_read_buffer(struct dentry *dentry, struct sysfs_buffer *buffer)
+static int fill_read_buffer(struct dentry *dentry, struct sysfs_open_file *of)
{
struct sysfs_dirent *attr_sd = dentry->d_fsdata;
struct kobject *kobj = attr_sd->s_parent->s_dir.kobj;
@@ -83,19 +83,19 @@ static int fill_read_buffer(struct dentry *dentry, struct sysfs_buffer *buffer)
int ret = 0;
ssize_t count;
- if (!buffer->page)
- buffer->page = (char *) get_zeroed_page(GFP_KERNEL);
- if (!buffer->page)
+ if (!of->page)
+ of->page = (char *) get_zeroed_page(GFP_KERNEL);
+ if (!of->page)
return -ENOMEM;
/* need attr_sd for attr and ops, its parent for kobj */
if (!sysfs_get_active(attr_sd))
return -ENODEV;
- buffer->event = atomic_read(&attr_sd->s_attr.open->event);
+ of->event = atomic_read(&attr_sd->s_attr.open->event);
ops = sysfs_file_ops(attr_sd);
- count = ops->show(kobj, attr_sd->s_attr.attr, buffer->page);
+ count = ops->show(kobj, attr_sd->s_attr.attr, of->page);
sysfs_put_active(attr_sd);
@@ -110,7 +110,7 @@ static int fill_read_buffer(struct dentry *dentry, struct sysfs_buffer *buffer)
count = PAGE_SIZE - 1;
}
if (count >= 0)
- buffer->count = count;
+ of->count = count;
else
ret = count;
return ret;
@@ -138,63 +138,62 @@ static int fill_read_buffer(struct dentry *dentry, struct sysfs_buffer *buffer)
static ssize_t
sysfs_read_file(struct file *file, char __user *buf, size_t count, loff_t *ppos)
{
- struct sysfs_buffer *buffer = file->private_data;
+ struct sysfs_open_file *of = file->private_data;
ssize_t retval = 0;
- mutex_lock(&buffer->mutex);
+ mutex_lock(&of->mutex);
/*
* 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 (*ppos == 0 || !of->page) {
+ retval = fill_read_buffer(file->f_path.dentry, of);
if (retval)
goto out;
}
pr_debug("%s: count = %zd, ppos = %lld, buf = %s\n",
- __func__, count, *ppos, buffer->page);
- retval = simple_read_from_buffer(buf, count, ppos, buffer->page,
- buffer->count);
+ __func__, count, *ppos, of->page);
+ retval = simple_read_from_buffer(buf, count, ppos, of->page, of->count);
out:
- mutex_unlock(&buffer->mutex);
+ mutex_unlock(&of->mutex);
return retval;
}
/**
* fill_write_buffer - copy buffer from userspace.
- * @buffer: data buffer for file.
+ * @of: open file struct.
* @buf: 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.
+ * Allocate @of->page if it hasn't been already, then copy the
+ * user-supplied buffer into it.
*/
-static int fill_write_buffer(struct sysfs_buffer *buffer,
+static int fill_write_buffer(struct sysfs_open_file *of,
const char __user *buf, size_t count)
{
int error;
- if (!buffer->page)
- buffer->page = (char *)get_zeroed_page(GFP_KERNEL);
- if (!buffer->page)
+ if (!of->page)
+ of->page = (char *)get_zeroed_page(GFP_KERNEL);
+ if (!of->page)
return -ENOMEM;
if (count >= PAGE_SIZE)
count = PAGE_SIZE - 1;
- error = copy_from_user(buffer->page, buf, count);
+ error = copy_from_user(of->page, buf, count);
/*
* 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;
+ of->page[count] = 0;
return error ? -EFAULT : count;
}
/**
* flush_write_buffer - push buffer to kobject.
* @dentry: dentry to the attribute
- * @buffer: data buffer for file.
+ * @of: open file
* @count: number of bytes
*
* Get the correct pointers for the kobject and the attribute we're
@@ -202,7 +201,7 @@ static int fill_write_buffer(struct sysfs_buffer *buffer,
* passing the buffer that we acquired in fill_write_buffer().
*/
static int flush_write_buffer(struct dentry *dentry,
- struct sysfs_buffer *buffer, size_t count)
+ struct sysfs_open_file *of, size_t count)
{
struct sysfs_dirent *attr_sd = dentry->d_fsdata;
struct kobject *kobj = attr_sd->s_parent->s_dir.kobj;
@@ -214,7 +213,7 @@ static int flush_write_buffer(struct dentry *dentry,
return -ENODEV;
ops = sysfs_file_ops(attr_sd);
- rc = ops->store(kobj, attr_sd->s_attr.attr, buffer->page, count);
+ rc = ops->store(kobj, attr_sd->s_attr.attr, of->page, count);
sysfs_put_active(attr_sd);
@@ -240,27 +239,26 @@ static int flush_write_buffer(struct dentry *dentry,
static ssize_t sysfs_write_file(struct file *file, const char __user *buf,
size_t count, loff_t *ppos)
{
- struct sysfs_buffer *buffer = file->private_data;
+ struct sysfs_open_file *of = file->private_data;
ssize_t len;
- mutex_lock(&buffer->mutex);
- len = fill_write_buffer(buffer, buf, count);
+ mutex_lock(&of->mutex);
+ len = fill_write_buffer(of, buf, count);
if (len > 0)
- len = flush_write_buffer(file->f_path.dentry, buffer, len);
+ len = flush_write_buffer(file->f_path.dentry, of, len);
if (len > 0)
*ppos += len;
- mutex_unlock(&buffer->mutex);
+ mutex_unlock(&of->mutex);
return len;
}
/**
* sysfs_get_open_dirent - get or create sysfs_open_dirent
* @sd: target sysfs_dirent
- * @buffer: sysfs_buffer for this instance of open
+ * @of: sysfs_open_file for this instance of open
*
* If @sd->s_attr.open exists, increment its reference count;
- * otherwise, create one. @buffer is chained to the buffers
- * list.
+ * otherwise, create one. @of is chained to the files list.
*
* LOCKING:
* Kernel thread context (may sleep).
@@ -269,7 +267,7 @@ static ssize_t sysfs_write_file(struct file *file, const char __user *buf,
* 0 on success, -errno on failure.
*/
static int sysfs_get_open_dirent(struct sysfs_dirent *sd,
- struct sysfs_buffer *buffer)
+ struct sysfs_open_file *of)
{
struct sysfs_open_dirent *od, *new_od = NULL;
@@ -285,7 +283,7 @@ static int sysfs_get_open_dirent(struct sysfs_dirent *sd,
od = sd->s_attr.open;
if (od) {
atomic_inc(&od->refcnt);
- list_add_tail(&buffer->list, &od->buffers);
+ list_add_tail(&of->list, &od->files);
}
spin_unlock_irq(&sysfs_open_dirent_lock);
@@ -304,23 +302,23 @@ static int sysfs_get_open_dirent(struct sysfs_dirent *sd,
atomic_set(&new_od->refcnt, 0);
atomic_set(&new_od->event, 1);
init_waitqueue_head(&new_od->poll);
- INIT_LIST_HEAD(&new_od->buffers);
+ INIT_LIST_HEAD(&new_od->files);
goto retry;
}
/**
* sysfs_put_open_dirent - put sysfs_open_dirent
* @sd: target sysfs_dirent
- * @buffer: associated sysfs_buffer
+ * @of: associated sysfs_open_file
*
- * Put @sd->s_attr.open and unlink @buffer from the buffers list.
- * If reference count reaches zero, disassociate and free it.
+ * Put @sd->s_attr.open and unlink @of from the files list. If
+ * reference count reaches zero, disassociate and free it.
*
* LOCKING:
* None.
*/
static void sysfs_put_open_dirent(struct sysfs_dirent *sd,
- struct sysfs_buffer *buffer)
+ struct sysfs_open_file *of)
{
struct sysfs_open_dirent *od = sd->s_attr.open;
unsigned long flags;
@@ -328,7 +326,7 @@ static void sysfs_put_open_dirent(struct sysfs_dirent *sd,
mutex_lock(&sysfs_open_file_mutex);
spin_lock_irqsave(&sysfs_open_dirent_lock, flags);
- list_del(&buffer->list);
+ list_del(&of->list);
if (atomic_dec_and_test(&od->refcnt))
sd->s_attr.open = NULL;
else
@@ -344,7 +342,7 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
{
struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
struct kobject *kobj = attr_sd->s_parent->s_dir.kobj;
- struct sysfs_buffer *buffer;
+ struct sysfs_open_file *of;
const struct sysfs_ops *ops;
int error = -EACCES;
@@ -377,19 +375,20 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
goto err_out;
}
- /* No error? Great, allocate a buffer for the file, and store it
- * it in file->private_data for easy access.
+ /*
+ * No error? Great, allocate a sysfs_open_file for the file, and
+ * store it it in file->private_data for easy access.
*/
error = -ENOMEM;
- buffer = kzalloc(sizeof(struct sysfs_buffer), GFP_KERNEL);
- if (!buffer)
+ of = kzalloc(sizeof(struct sysfs_open_file), GFP_KERNEL);
+ if (!of)
goto err_out;
- mutex_init(&buffer->mutex);
- file->private_data = buffer;
+ mutex_init(&of->mutex);
+ file->private_data = of;
/* make sure we have open dirent struct */
- error = sysfs_get_open_dirent(attr_sd, buffer);
+ error = sysfs_get_open_dirent(attr_sd, of);
if (error)
goto err_free;
@@ -398,7 +397,7 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
return 0;
err_free:
- kfree(buffer);
+ kfree(of);
err_out:
sysfs_put_active(attr_sd);
return error;
@@ -407,13 +406,13 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
static int sysfs_release(struct inode *inode, struct file *filp)
{
struct sysfs_dirent *sd = filp->f_path.dentry->d_fsdata;
- struct sysfs_buffer *buffer = filp->private_data;
+ struct sysfs_open_file *of = filp->private_data;
- sysfs_put_open_dirent(sd, buffer);
+ sysfs_put_open_dirent(sd, of);
- if (buffer->page)
- free_page((unsigned long)buffer->page);
- kfree(buffer);
+ if (of->page)
+ free_page((unsigned long)of->page);
+ kfree(of);
return 0;
}
@@ -433,7 +432,7 @@ static int sysfs_release(struct inode *inode, struct file *filp)
*/
static unsigned int sysfs_poll(struct file *filp, poll_table *wait)
{
- struct sysfs_buffer *buffer = filp->private_data;
+ struct sysfs_open_file *of = filp->private_data;
struct sysfs_dirent *attr_sd = filp->f_path.dentry->d_fsdata;
struct sysfs_open_dirent *od = attr_sd->s_attr.open;
@@ -445,7 +444,7 @@ static unsigned int sysfs_poll(struct file *filp, poll_table *wait)
sysfs_put_active(attr_sd);
- if (buffer->event != atomic_read(&od->event))
+ if (of->event != atomic_read(&od->event))
goto trigger;
return DEFAULT_POLLMASK;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 06/14] sysfs: add sysfs_open_file->sd and ->file
2013-09-28 21:49 [PATCHSET] sysfs: use seq_file and unify regular and bin file handling Tejun Heo
` (4 preceding siblings ...)
2013-09-28 21:49 ` [PATCH 05/14] sysfs: rename sysfs_buffer to sysfs_open_file Tejun Heo
@ 2013-09-28 21:49 ` Tejun Heo
2013-09-28 21:49 ` [PATCH 07/14] sysfs: use transient write buffer Tejun Heo
` (8 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2013-09-28 21:49 UTC (permalink / raw)
To: gregkh; +Cc: kay, linux-kernel, ebiederm, Tejun Heo
sysfs will be converted to use seq_file for read path, which will make
it difficult to pass around multiple pointers directly. This patch
adds sysfs_open_file->sd and ->file so that we can reach all the
necessary data structures from sysfs_open_file.
flush_write_buffer() is updated to drop @dentry which was used to
discover the sysfs_dirent as it's now available through
sysfs_open_file->sd.
This patch doesn't cause any behavior difference.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
fs/sysfs/file.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 4b55bcf..af6e909 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -45,6 +45,8 @@ struct sysfs_open_dirent {
};
struct sysfs_open_file {
+ struct sysfs_dirent *sd;
+ struct file *file;
size_t count;
char *page;
struct mutex mutex;
@@ -192,7 +194,6 @@ static int fill_write_buffer(struct sysfs_open_file *of,
/**
* flush_write_buffer - push buffer to kobject.
- * @dentry: dentry to the attribute
* @of: open file
* @count: number of bytes
*
@@ -200,22 +201,20 @@ static int fill_write_buffer(struct sysfs_open_file *of,
* dealing with, then call the store() method for the attribute,
* passing the buffer that we acquired in fill_write_buffer().
*/
-static int flush_write_buffer(struct dentry *dentry,
- struct sysfs_open_file *of, size_t count)
+static int flush_write_buffer(struct sysfs_open_file *of, size_t count)
{
- struct sysfs_dirent *attr_sd = dentry->d_fsdata;
- struct kobject *kobj = attr_sd->s_parent->s_dir.kobj;
+ struct kobject *kobj = of->sd->s_parent->s_dir.kobj;
const struct sysfs_ops *ops;
int rc;
- /* need attr_sd for attr and ops, its parent for kobj */
- if (!sysfs_get_active(attr_sd))
+ /* need @of->sd for attr and ops, its parent for kobj */
+ if (!sysfs_get_active(of->sd))
return -ENODEV;
- ops = sysfs_file_ops(attr_sd);
- rc = ops->store(kobj, attr_sd->s_attr.attr, of->page, count);
+ ops = sysfs_file_ops(of->sd);
+ rc = ops->store(kobj, of->sd->s_attr.attr, of->page, count);
- sysfs_put_active(attr_sd);
+ sysfs_put_active(of->sd);
return rc;
}
@@ -245,7 +244,7 @@ static ssize_t sysfs_write_file(struct file *file, const char __user *buf,
mutex_lock(&of->mutex);
len = fill_write_buffer(of, buf, count);
if (len > 0)
- len = flush_write_buffer(file->f_path.dentry, of, len);
+ len = flush_write_buffer(of, len);
if (len > 0)
*ppos += len;
mutex_unlock(&of->mutex);
@@ -385,6 +384,8 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
goto err_out;
mutex_init(&of->mutex);
+ of->sd = attr_sd;
+ of->file = file;
file->private_data = of;
/* make sure we have open dirent struct */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 07/14] sysfs: use transient write buffer
2013-09-28 21:49 [PATCHSET] sysfs: use seq_file and unify regular and bin file handling Tejun Heo
` (5 preceding siblings ...)
2013-09-28 21:49 ` [PATCH 06/14] sysfs: add sysfs_open_file->sd and ->file Tejun Heo
@ 2013-09-28 21:49 ` Tejun Heo
2013-09-28 21:49 ` [PATCH 08/14] sysfs: use seq_file when reading regular files Tejun Heo
` (7 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2013-09-28 21:49 UTC (permalink / raw)
To: gregkh; +Cc: kay, linux-kernel, ebiederm, Tejun Heo
There isn't much to be gained by keeping around kernel buffer while a
file is open especially as the read path planned to be converted to
use seq_file and won't use the buffer. This patch makes
sysfs_write_file() use per-write transient buffer instead of
sysfs_open_file->page.
This simplifies the write path, enables removing sysfs_open_file->page
once read path is updated and will help merging bin file write path
which already requires the use of a transient buffer due to a locking
order issue.
As the function comments of flush_write_buffer() and
sysfs_write_buffer() are being updated anyway, reformat them so that
they're more conventional.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
fs/sysfs/file.c | 114 ++++++++++++++++++++++++++------------------------------
1 file changed, 52 insertions(+), 62 deletions(-)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index af6e909..642dbcc 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -162,92 +162,82 @@ out:
}
/**
- * fill_write_buffer - copy buffer from userspace.
- * @of: open file struct.
- * @buf: data from user.
- * @count: number of bytes in @userbuf.
+ * flush_write_buffer - push buffer to kobject
+ * @of: open file
+ * @buf: data buffer for file
+ * @count: number of bytes
*
- * Allocate @of->page if it hasn't been already, then copy the
- * user-supplied buffer into it.
+ * Get the correct pointers for the kobject and the attribute we're dealing
+ * with, then call the store() method for it with @buf.
*/
-static int fill_write_buffer(struct sysfs_open_file *of,
- const char __user *buf, size_t count)
-{
- int error;
-
- if (!of->page)
- of->page = (char *)get_zeroed_page(GFP_KERNEL);
- if (!of->page)
- return -ENOMEM;
-
- if (count >= PAGE_SIZE)
- count = PAGE_SIZE - 1;
- error = copy_from_user(of->page, buf, count);
-
- /*
- * If buf is assumed to contain a string, terminate it by \0, so
- * e.g. sscanf() can scan the string easily.
- */
- of->page[count] = 0;
- return error ? -EFAULT : count;
-}
-
-/**
- * flush_write_buffer - push buffer to kobject.
- * @of: open file
- * @count: number of bytes
- *
- * 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().
- */
-static int flush_write_buffer(struct sysfs_open_file *of, size_t count)
+static int flush_write_buffer(struct sysfs_open_file *of, char *buf,
+ size_t count)
{
struct kobject *kobj = of->sd->s_parent->s_dir.kobj;
const struct sysfs_ops *ops;
- int rc;
+ int rc = 0;
- /* need @of->sd for attr and ops, its parent for kobj */
- if (!sysfs_get_active(of->sd))
+ /*
+ * Need @of->sd for attr and ops, its parent for kobj. @of->mutex
+ * nests outside active ref and is just to ensure that the ops
+ * aren't called concurrently for the same open file.
+ */
+ mutex_lock(&of->mutex);
+ if (!sysfs_get_active(of->sd)) {
+ mutex_unlock(&of->mutex);
return -ENODEV;
+ }
ops = sysfs_file_ops(of->sd);
- rc = ops->store(kobj, of->sd->s_attr.attr, of->page, count);
+ rc = ops->store(kobj, of->sd->s_attr.attr, buf, count);
sysfs_put_active(of->sd);
+ mutex_unlock(&of->mutex);
return rc;
}
/**
- * sysfs_write_file - write an attribute.
- * @file: file pointer
- * @buf: data to write
- * @count: number of bytes
- * @ppos: starting offset
+ * sysfs_write_file - write an attribute
+ * @file: file pointer
+ * @user_buf: data to write
+ * @count: number of bytes
+ * @ppos: starting offset
*
- * 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.
+ * Copy data in from userland and pass it to the matching
+ * sysfs_ops->store() by invoking 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 __user *buf,
+static ssize_t sysfs_write_file(struct file *file, const char __user *user_buf,
size_t count, loff_t *ppos)
{
struct sysfs_open_file *of = file->private_data;
- ssize_t len;
+ ssize_t len = min(count, PAGE_SIZE - 1);
+ char *buf;
- mutex_lock(&of->mutex);
- len = fill_write_buffer(of, buf, count);
- if (len > 0)
- len = flush_write_buffer(of, len);
+ if (!len)
+ return 0;
+
+ buf = kmalloc(len + 1, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ if (copy_from_user(buf, user_buf, len)) {
+ len = -EFAULT;
+ goto out_free;
+ }
+ buf[len] = '\0'; /* guarantee string termination */
+
+ len = flush_write_buffer(of, buf, len);
if (len > 0)
*ppos += len;
- mutex_unlock(&of->mutex);
+out_free:
+ kfree(buf);
return len;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 08/14] sysfs: use seq_file when reading regular files
2013-09-28 21:49 [PATCHSET] sysfs: use seq_file and unify regular and bin file handling Tejun Heo
` (6 preceding siblings ...)
2013-09-28 21:49 ` [PATCH 07/14] sysfs: use transient write buffer Tejun Heo
@ 2013-09-28 21:49 ` Tejun Heo
2013-09-28 21:49 ` [PATCH 09/14] sysfs: prepare llseek path for unified regular / bin file handling Tejun Heo
` (6 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2013-09-28 21:49 UTC (permalink / raw)
To: gregkh; +Cc: kay, linux-kernel, ebiederm, Tejun Heo
sysfs read path implements its own buffering scheme between userland
and kernel callbacks, which essentially is a degenerate duplicate of
seq_file. This patch replaces the custom read buffering
implementation in sysfs with seq_file.
While the amount of code reduction is small, this reduces low level
hairiness and enables future development of a new versatile API based
on seq_file so that sysfs features can be shared with other
subsystems.
As write path was already converted to not use sysfs_open_file->page,
this patch makes ->page and ->count unused and removes them.
Userland behavior remains the same except for some extreme corner
cases - e.g. sysfs will now regenerate the content each time a file is
read after a non-contiguous seek whereas the original code would keep
using the same content. While this is a userland visible behavior
change, it is extremely unlikely to be noticeable and brings sysfs
behavior closer to that of procfs.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Kay Sievers <kay@vrfy.org>
---
fs/sysfs/file.c | 164 +++++++++++++++++++++++++-------------------------------
1 file changed, 73 insertions(+), 91 deletions(-)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 642dbcc..6211dd7 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -21,6 +21,7 @@
#include <linux/mutex.h>
#include <linux/limits.h>
#include <linux/uaccess.h>
+#include <linux/seq_file.h>
#include "sysfs.h"
@@ -31,7 +32,8 @@
* sysfs_dirent->s_attr.open points to sysfs_open_dirent. s_attr.open is
* protected by sysfs_open_dirent_lock.
*
- * filp->private_data points to sysfs_open_file which is chained at
+ * filp->private_data points to seq_file whose ->private points to
+ * sysfs_open_file. sysfs_open_files are chained at
* sysfs_open_dirent->files, which is protected by sysfs_open_file_mutex.
*/
static DEFINE_SPINLOCK(sysfs_open_dirent_lock);
@@ -47,13 +49,16 @@ struct sysfs_open_dirent {
struct sysfs_open_file {
struct sysfs_dirent *sd;
struct file *file;
- size_t count;
- char *page;
struct mutex mutex;
int event;
struct list_head list;
};
+static struct sysfs_open_file *sysfs_of(struct file *file)
+{
+ return ((struct seq_file *)file->private_data)->private;
+}
+
/*
* Determine ktype->sysfs_ops for the given sysfs_dirent. This function
* must be called while holding an active reference.
@@ -66,40 +71,54 @@ static const struct sysfs_ops *sysfs_file_ops(struct sysfs_dirent *sd)
return kobj->ktype ? kobj->ktype->sysfs_ops : NULL;
}
-/**
- * fill_read_buffer - allocate and fill buffer from object.
- * @dentry: dentry 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 unless an error
- * is returned.
+/*
+ * Reads on sysfs are handled through seq_file, which takes care of hairy
+ * details like buffering and seeking. The following function pipes
+ * sysfs_ops->show() result through seq_file.
*/
-static int fill_read_buffer(struct dentry *dentry, struct sysfs_open_file *of)
+static int sysfs_seq_show(struct seq_file *sf, void *v)
{
- struct sysfs_dirent *attr_sd = dentry->d_fsdata;
- struct kobject *kobj = attr_sd->s_parent->s_dir.kobj;
+ struct sysfs_open_file *of = sf->private;
+ struct kobject *kobj = of->sd->s_parent->s_dir.kobj;
const struct sysfs_ops *ops;
- int ret = 0;
+ char *buf;
ssize_t count;
- if (!of->page)
- of->page = (char *) get_zeroed_page(GFP_KERNEL);
- if (!of->page)
- return -ENOMEM;
+ /* acquire buffer and ensure that it's >= PAGE_SIZE */
+ count = seq_get_buf(sf, &buf);
+ if (count < PAGE_SIZE) {
+ seq_commit(sf, -1);
+ return 0;
+ }
- /* need attr_sd for attr and ops, its parent for kobj */
- if (!sysfs_get_active(attr_sd))
+ /*
+ * Need @of->sd for attr and ops, its parent for kobj. @of->mutex
+ * nests outside active ref and is just to ensure that the ops
+ * aren't called concurrently for the same open file.
+ */
+ mutex_lock(&of->mutex);
+ if (!sysfs_get_active(of->sd)) {
+ mutex_unlock(&of->mutex);
return -ENODEV;
+ }
- of->event = atomic_read(&attr_sd->s_attr.open->event);
+ of->event = atomic_read(&of->sd->s_attr.open->event);
- ops = sysfs_file_ops(attr_sd);
- count = ops->show(kobj, attr_sd->s_attr.attr, of->page);
+ /*
+ * Lookup @ops and invoke show(). Control may reach here via seq
+ * file lseek even if @ops->show() isn't implemented.
+ */
+ ops = sysfs_file_ops(of->sd);
+ if (ops->show)
+ count = ops->show(kobj, of->sd->s_attr.attr, buf);
+ else
+ count = 0;
- sysfs_put_active(attr_sd);
+ sysfs_put_active(of->sd);
+ mutex_unlock(&of->mutex);
+
+ if (count < 0)
+ return count;
/*
* The code works fine with PAGE_SIZE return but it's likely to
@@ -111,54 +130,8 @@ static int fill_read_buffer(struct dentry *dentry, struct sysfs_open_file *of)
/* Try to struggle along */
count = PAGE_SIZE - 1;
}
- if (count >= 0)
- of->count = count;
- else
- ret = count;
- return ret;
-}
-
-/**
- * sysfs_read_file - read an attribute.
- * @file: file pointer.
- * @buf: buffer to fill.
- * @count: number of bytes to read.
- * @ppos: starting offset in file.
- *
- * 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.
- *
- * 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 __user *buf, size_t count, loff_t *ppos)
-{
- struct sysfs_open_file *of = file->private_data;
- ssize_t retval = 0;
-
- mutex_lock(&of->mutex);
- /*
- * 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 || !of->page) {
- retval = fill_read_buffer(file->f_path.dentry, of);
- if (retval)
- goto out;
- }
- pr_debug("%s: count = %zd, ppos = %lld, buf = %s\n",
- __func__, count, *ppos, of->page);
- retval = simple_read_from_buffer(buf, count, ppos, of->page, of->count);
-out:
- mutex_unlock(&of->mutex);
- return retval;
+ seq_commit(sf, count);
+ return 0;
}
/**
@@ -216,7 +189,7 @@ static int flush_write_buffer(struct sysfs_open_file *of, char *buf,
static ssize_t sysfs_write_file(struct file *file, const char __user *user_buf,
size_t count, loff_t *ppos)
{
- struct sysfs_open_file *of = file->private_data;
+ struct sysfs_open_file *of = sysfs_of(file);
ssize_t len = min(count, PAGE_SIZE - 1);
char *buf;
@@ -364,10 +337,7 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
goto err_out;
}
- /*
- * No error? Great, allocate a sysfs_open_file for the file, and
- * store it it in file->private_data for easy access.
- */
+ /* allocate a sysfs_open_file for the file */
error = -ENOMEM;
of = kzalloc(sizeof(struct sysfs_open_file), GFP_KERNEL);
if (!of)
@@ -376,20 +346,34 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
mutex_init(&of->mutex);
of->sd = attr_sd;
of->file = file;
- file->private_data = of;
+
+ /*
+ * Always instantiate seq_file even if read access is not
+ * implemented or requested. This unifies private data access and
+ * most files are readable anyway.
+ */
+ error = single_open(file, sysfs_seq_show, of);
+ if (error)
+ goto err_free;
+
+ /* seq_file clears PWRITE unconditionally, restore it if WRITE */
+ if (file->f_mode & FMODE_WRITE)
+ file->f_mode |= FMODE_PWRITE;
/* make sure we have open dirent struct */
error = sysfs_get_open_dirent(attr_sd, of);
if (error)
- goto err_free;
+ goto err_close;
/* open succeeded, put active references */
sysfs_put_active(attr_sd);
return 0;
- err_free:
+err_close:
+ single_release(inode, file);
+err_free:
kfree(of);
- err_out:
+err_out:
sysfs_put_active(attr_sd);
return error;
}
@@ -397,12 +381,10 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
static int sysfs_release(struct inode *inode, struct file *filp)
{
struct sysfs_dirent *sd = filp->f_path.dentry->d_fsdata;
- struct sysfs_open_file *of = filp->private_data;
+ struct sysfs_open_file *of = sysfs_of(filp);
sysfs_put_open_dirent(sd, of);
-
- if (of->page)
- free_page((unsigned long)of->page);
+ single_release(inode, filp);
kfree(of);
return 0;
@@ -423,7 +405,7 @@ static int sysfs_release(struct inode *inode, struct file *filp)
*/
static unsigned int sysfs_poll(struct file *filp, poll_table *wait)
{
- struct sysfs_open_file *of = filp->private_data;
+ struct sysfs_open_file *of = sysfs_of(filp);
struct sysfs_dirent *attr_sd = filp->f_path.dentry->d_fsdata;
struct sysfs_open_dirent *od = attr_sd->s_attr.open;
@@ -481,9 +463,9 @@ void sysfs_notify(struct kobject *k, const char *dir, const char *attr)
EXPORT_SYMBOL_GPL(sysfs_notify);
const struct file_operations sysfs_file_operations = {
- .read = sysfs_read_file,
+ .read = seq_read,
.write = sysfs_write_file,
- .llseek = generic_file_llseek,
+ .llseek = seq_lseek,
.open = sysfs_open_file,
.release = sysfs_release,
.poll = sysfs_poll,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 09/14] sysfs: prepare llseek path for unified regular / bin file handling
2013-09-28 21:49 [PATCHSET] sysfs: use seq_file and unify regular and bin file handling Tejun Heo
` (7 preceding siblings ...)
2013-09-28 21:49 ` [PATCH 08/14] sysfs: use seq_file when reading regular files Tejun Heo
@ 2013-09-28 21:49 ` Tejun Heo
2013-09-28 21:49 ` [PATCH 10/14] sysfs: prepare path write " Tejun Heo
` (5 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2013-09-28 21:49 UTC (permalink / raw)
To: gregkh; +Cc: kay, linux-kernel, ebiederm, Tejun Heo
sysfs bin file handling will be merged into the regular file support.
This patch prepares the llseek path.
sysfs currently unconditionally uses seq_lseek() whether the file
supports read or not, which means that sysfs_seq_show() may be used
purely for seeking even if the file doesn't implement read.
sysfs_seq_show() simply doesn't produce any data if sysfs_ops->show()
is not available. This is good enough for write-only files as open()
doesn't allow FMODE_READ if sysfs_ops->show() is not implemented and
seq_lseek() sets f_pos to the requested offset as long as show()
doesn't fail.
However, bin files allow FMODE_READ when ->mmap() is implemented even
if ->read() is not, which means that sysfs_seq_show() would need to
fail if ->read() is not implemented, which is fine for read(2) but
would break lseek(2).
This patch implements sysfs_llseek() which uses seq_lseek() iff read
is implemented. If not, generic_file_llseek() is used instead. This
removes the case where sysfs_seq_show() is used purely for seeking
thus solving the above issue. Plus, it's weird to use seq_seek() when
seq_file isn't being used anyway.
Note that sysfs_llseek() handles both regular and bin files. While
this isn't used yet, it'll allow unifying handling of both types.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
fs/sysfs/file.c | 44 +++++++++++++++++++++++++++++++++++---------
1 file changed, 35 insertions(+), 9 deletions(-)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 6211dd7..d9109d3 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -54,6 +54,11 @@ struct sysfs_open_file {
struct list_head list;
};
+static bool sysfs_is_bin(struct sysfs_dirent *sd)
+{
+ return sysfs_type(sd) == SYSFS_KOBJ_BIN_ATTR;
+}
+
static struct sysfs_open_file *sysfs_of(struct file *file)
{
return ((struct seq_file *)file->private_data)->private;
@@ -72,6 +77,33 @@ static const struct sysfs_ops *sysfs_file_ops(struct sysfs_dirent *sd)
}
/*
+ * llseek for sysfs. Use seq_lseek() if read operation is implemented;
+ * otherwise, fall back to generic_file_llseek(). This ensures that
+ * sysfs_seq_show() isn't invoked to seek in a file which doesn't
+ * implemented read.
+ */
+static loff_t sysfs_llseek(struct file *file, loff_t offset, int whence)
+{
+ struct sysfs_open_file *of = sysfs_of(file);
+ bool has_read;
+
+ if (!sysfs_get_active(of->sd))
+ return -ENODEV;
+
+ if (sysfs_is_bin(of->sd))
+ has_read = of->sd->s_bin_attr.bin_attr->read;
+ else
+ has_read = sysfs_file_ops(of->sd)->show;
+
+ sysfs_put_active(of->sd);
+
+ if (has_read)
+ return seq_lseek(file, offset, whence);
+ else
+ return generic_file_llseek(file, offset, whence);
+}
+
+/*
* Reads on sysfs are handled through seq_file, which takes care of hairy
* details like buffering and seeking. The following function pipes
* sysfs_ops->show() result through seq_file.
@@ -104,15 +136,9 @@ static int sysfs_seq_show(struct seq_file *sf, void *v)
of->event = atomic_read(&of->sd->s_attr.open->event);
- /*
- * Lookup @ops and invoke show(). Control may reach here via seq
- * file lseek even if @ops->show() isn't implemented.
- */
+ /* lookup @ops and invoke show() */
ops = sysfs_file_ops(of->sd);
- if (ops->show)
- count = ops->show(kobj, of->sd->s_attr.attr, buf);
- else
- count = 0;
+ count = ops->show(kobj, of->sd->s_attr.attr, buf);
sysfs_put_active(of->sd);
mutex_unlock(&of->mutex);
@@ -465,7 +491,7 @@ EXPORT_SYMBOL_GPL(sysfs_notify);
const struct file_operations sysfs_file_operations = {
.read = seq_read,
.write = sysfs_write_file,
- .llseek = seq_lseek,
+ .llseek = sysfs_llseek,
.open = sysfs_open_file,
.release = sysfs_release,
.poll = sysfs_poll,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 10/14] sysfs: prepare path write for unified regular / bin file handling
2013-09-28 21:49 [PATCHSET] sysfs: use seq_file and unify regular and bin file handling Tejun Heo
` (8 preceding siblings ...)
2013-09-28 21:49 ` [PATCH 09/14] sysfs: prepare llseek path for unified regular / bin file handling Tejun Heo
@ 2013-09-28 21:49 ` Tejun Heo
2013-09-28 21:49 ` [PATCH 11/14] sysfs: prepare read path " Tejun Heo
` (4 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2013-09-28 21:49 UTC (permalink / raw)
To: gregkh; +Cc: kay, linux-kernel, ebiederm, Tejun Heo
sysfs bin file handling will be merged into the regular file support.
This patch prepares the write path.
bin file write is almost identical to regular file write except that
the write length is capped by the inode size and @off is passed to the
write method. This patch adds bin file handling to sysfs_write_file()
so that it can handle both regular and bin files.
This is a preparation and the new bin file path isn't used yet.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
fs/sysfs/file.c | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index d9109d3..5380009 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -164,16 +164,16 @@ static int sysfs_seq_show(struct seq_file *sf, void *v)
* flush_write_buffer - push buffer to kobject
* @of: open file
* @buf: data buffer for file
+ * @off: file offset to write to
* @count: number of bytes
*
* Get the correct pointers for the kobject and the attribute we're dealing
* with, then call the store() method for it with @buf.
*/
-static int flush_write_buffer(struct sysfs_open_file *of, char *buf,
+static int flush_write_buffer(struct sysfs_open_file *of, char *buf, loff_t off,
size_t count)
{
struct kobject *kobj = of->sd->s_parent->s_dir.kobj;
- const struct sysfs_ops *ops;
int rc = 0;
/*
@@ -187,8 +187,18 @@ static int flush_write_buffer(struct sysfs_open_file *of, char *buf,
return -ENODEV;
}
- ops = sysfs_file_ops(of->sd);
- rc = ops->store(kobj, of->sd->s_attr.attr, buf, count);
+ if (sysfs_is_bin(of->sd)) {
+ struct bin_attribute *battr = of->sd->s_bin_attr.bin_attr;
+
+ rc = -EIO;
+ if (battr->write)
+ rc = battr->write(of->file, kobj, battr, buf, off,
+ count);
+ } else {
+ const struct sysfs_ops *ops = sysfs_file_ops(of->sd);
+
+ rc = ops->store(kobj, of->sd->s_attr.attr, buf, count);
+ }
sysfs_put_active(of->sd);
mutex_unlock(&of->mutex);
@@ -216,9 +226,17 @@ static ssize_t sysfs_write_file(struct file *file, const char __user *user_buf,
size_t count, loff_t *ppos)
{
struct sysfs_open_file *of = sysfs_of(file);
- ssize_t len = min(count, PAGE_SIZE - 1);
+ ssize_t len = min(count, PAGE_SIZE);
char *buf;
+ if (sysfs_is_bin(of->sd)) {
+ loff_t size = file_inode(file)->i_size;
+
+ if (size <= *ppos)
+ return 0;
+ len = min_t(ssize_t, len, size - *ppos);
+ }
+
if (!len)
return 0;
@@ -232,7 +250,7 @@ static ssize_t sysfs_write_file(struct file *file, const char __user *user_buf,
}
buf[len] = '\0'; /* guarantee string termination */
- len = flush_write_buffer(of, buf, len);
+ len = flush_write_buffer(of, buf, *ppos, len);
if (len > 0)
*ppos += len;
out_free:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 11/14] sysfs: prepare read path for unified regular / bin file handling
2013-09-28 21:49 [PATCHSET] sysfs: use seq_file and unify regular and bin file handling Tejun Heo
` (9 preceding siblings ...)
2013-09-28 21:49 ` [PATCH 10/14] sysfs: prepare path write " Tejun Heo
@ 2013-09-28 21:49 ` Tejun Heo
2013-09-28 21:49 ` [PATCH 12/14] sysfs: copy bin mmap support from fs/sysfs/bin.c to fs/sysfs/file.c Tejun Heo
` (3 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2013-09-28 21:49 UTC (permalink / raw)
To: gregkh; +Cc: kay, linux-kernel, ebiederm, Tejun Heo
sysfs bin file handling will be merged into the regular file support.
This patch prepares the read path.
This is a bit tricky as read support is quite different between
regular and bin files. bin file supports arbitrarily large file size
and passes the read offset and size directly to the callback as long
as the size is <= PAGE_SIZE. While it's doable to preserve the offset
and size parameters from userland and pass them to
bin_attribute->read() callback, none of the current users seems to
depend on the behavior and it's a lot simpler and more efficient to
implement pagniated behavior. After all, it is an extremely bad idea
to make reads of sysfs files to have side-effects.
sysfs_bin_start/next/stop() are implemented so that seq_file iterator
pointer is 1-based page index and sysfs_seq_show() is updated to
transfer data from bin_attribute->read() to seq_file buffer
page-by-page. A comment clarifying that ->read() must not have
side-effects is added to bin_attribute definition.
This is a preparation and the new bin file path isn't used yet.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Kay Sievers <kay@vrfy.org>
---
fs/sysfs/file.c | 96 +++++++++++++++++++++++++++++++++++++++++++--------
include/linux/sysfs.h | 7 ++++
2 files changed, 89 insertions(+), 14 deletions(-)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 5380009..46f7d59 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -52,6 +52,8 @@ struct sysfs_open_file {
struct mutex mutex;
int event;
struct list_head list;
+
+ void *private_data;
};
static bool sysfs_is_bin(struct sysfs_dirent *sd)
@@ -112,7 +114,6 @@ static int sysfs_seq_show(struct seq_file *sf, void *v)
{
struct sysfs_open_file *of = sf->private;
struct kobject *kobj = of->sd->s_parent->s_dir.kobj;
- const struct sysfs_ops *ops;
char *buf;
ssize_t count;
@@ -136,9 +137,48 @@ static int sysfs_seq_show(struct seq_file *sf, void *v)
of->event = atomic_read(&of->sd->s_attr.open->event);
- /* lookup @ops and invoke show() */
- ops = sysfs_file_ops(of->sd);
- count = ops->show(kobj, of->sd->s_attr.attr, buf);
+ /* lookup ops and invoke read/show() */
+ if (sysfs_is_bin(of->sd)) {
+ struct bin_attribute *battr = of->sd->s_bin_attr.bin_attr;
+ size_t idx = (unsigned long)v - 1;
+ loff_t off = idx << PAGE_SHIFT;
+ size_t size = file_inode(of->file)->i_size;
+
+ if (size)
+ size = min_t(size_t, size - off, PAGE_SIZE);
+ else
+ size = PAGE_SIZE;
+
+ /* @battr may be implementing only ->mmap() */
+ count = -EIO;
+ if (battr->read) {
+ count = battr->read(of->file, kobj, battr, buf, off,
+ size);
+ /*
+ * If read() returned zero, it is the end of the
+ * file. Record it so that ->next() terminates on
+ * the next invocation.
+ */
+ if (!count)
+ of->private_data = (void *)(unsigned long)idx;
+ }
+ } else {
+ const struct sysfs_ops *ops = sysfs_file_ops(of->sd);
+
+ count = ops->show(kobj, of->sd->s_attr.attr, buf);
+
+ /*
+ * The code works fine with PAGE_SIZE return but it's
+ * likely to indicate truncated result or overflow in
+ * normal use cases.
+ */
+ if (unlikely(count >= (ssize_t)PAGE_SIZE)) {
+ print_symbol("fill_read_buffer: %s returned bad count\n",
+ (unsigned long)ops->show);
+ /* Try to struggle along */
+ count = PAGE_SIZE - 1;
+ }
+ }
sysfs_put_active(of->sd);
mutex_unlock(&of->mutex);
@@ -146,20 +186,48 @@ static int sysfs_seq_show(struct seq_file *sf, void *v)
if (count < 0)
return count;
- /*
- * The code works fine with PAGE_SIZE return but it's likely to
- * indicate truncated result or overflow in normal use cases.
- */
- if (count >= (ssize_t)PAGE_SIZE) {
- print_symbol("fill_read_buffer: %s returned bad count\n",
- (unsigned long)ops->show);
- /* Try to struggle along */
- count = PAGE_SIZE - 1;
- }
seq_commit(sf, count);
return 0;
}
+static void *sysfs_bin_seq_start(struct seq_file *sf, loff_t *pidx)
+{
+ struct sysfs_open_file *of = sf->private;
+ loff_t size = file_inode(of->file)->i_size;
+ size_t nr_pages = DIV_ROUND_UP(size, PAGE_SIZE);
+
+ /* record number of pages in private_data */
+ of->private_data = (void *)(unsigned long)nr_pages;
+
+ if (nr_pages && *pidx >= nr_pages)
+ return NULL;
+
+ /* 1 based page index as NULL is term condition */
+ return (void *)(unsigned long)(1 + *pidx);
+}
+
+static void *sysfs_bin_seq_next(struct seq_file *sf, void *v, loff_t *pidx)
+{
+ struct sysfs_open_file *of = sf->private;
+ size_t nr_pages = (unsigned long)of->private_data;
+
+ if (nr_pages && *pidx >= nr_pages)
+ return NULL;
+
+ return (void *)(unsigned long)(1 + ++(*pidx));
+}
+
+static void sysfs_bin_seq_stop(struct seq_file *sf, void *v)
+{
+}
+
+static struct seq_operations sysfs_bin_seq_ops = {
+ .start = sysfs_bin_seq_start,
+ .next = sysfs_bin_seq_next,
+ .stop = sysfs_bin_seq_stop,
+ .show = sysfs_seq_show,
+};
+
/**
* flush_write_buffer - push buffer to kobject
* @of: open file
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 6695040..f99a4c2 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -120,6 +120,13 @@ struct bin_attribute {
struct attribute attr;
size_t size;
void *private;
+
+ /*
+ * Reads are buffered by seq_file and offset and length passed to
+ * ->read() may not match the parameters specified in read(2).
+ * ->read() may also be invoked while seeking. As such, ->read()
+ * must not have side-effects.
+ */
ssize_t (*read)(struct file *, struct kobject *, struct bin_attribute *,
char *, loff_t, size_t);
ssize_t (*write)(struct file *, struct kobject *, struct bin_attribute *,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 12/14] sysfs: copy bin mmap support from fs/sysfs/bin.c to fs/sysfs/file.c
2013-09-28 21:49 [PATCHSET] sysfs: use seq_file and unify regular and bin file handling Tejun Heo
` (10 preceding siblings ...)
2013-09-28 21:49 ` [PATCH 11/14] sysfs: prepare read path " Tejun Heo
@ 2013-09-28 21:49 ` Tejun Heo
2013-09-28 21:49 ` [PATCH 13/14] sysfs: prepare open path for unified regular / bin file handling Tejun Heo
` (2 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2013-09-28 21:49 UTC (permalink / raw)
To: gregkh; +Cc: kay, linux-kernel, ebiederm, Tejun Heo
sysfs bin file handling will be merged into the regular file support.
This patch copies mmap support from bin so that fs/sysfs/file.c can
handle mmapping bin files.
The code is copied mostly verbatim with the following updates.
* ->mmapped and ->vm_ops are added to sysfs_open_file and bin_buffer
references are replaced with sysfs_open_file ones.
* Symbols are prefixed with sysfs_.
* sysfs_bin_mmap() explicitly checks whether the file is a bin file
and returns -ENODEV if not.
* sysfs_unmap_bin_file() grabs sysfs_open_dirent and traverses
->files. Invocation of this function is added to
sysfs_addrm_finish().
This is a preparation and the new mmap path isn't used yet.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
fs/sysfs/dir.c | 1 +
fs/sysfs/file.c | 250 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
fs/sysfs/sysfs.h | 2 +
3 files changed, 252 insertions(+), 1 deletion(-)
diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index b518afd..c4040dd 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -595,6 +595,7 @@ void sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt)
acxt->removed = sd->u.removed_list;
sysfs_deactivate(sd);
+ sysfs_unmap_bin_file(sd);
unmap_bin_file(sd);
sysfs_put(sd);
}
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 46f7d59..fe5c440 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -22,6 +22,7 @@
#include <linux/limits.h>
#include <linux/uaccess.h>
#include <linux/seq_file.h>
+#include <linux/mm.h>
#include "sysfs.h"
@@ -53,6 +54,9 @@ struct sysfs_open_file {
int event;
struct list_head list;
+ bool mmapped;
+ const struct vm_operations_struct *vm_ops;
+
void *private_data;
};
@@ -326,6 +330,221 @@ out_free:
return len;
}
+static void sysfs_bin_vma_open(struct vm_area_struct *vma)
+{
+ struct file *file = vma->vm_file;
+ struct sysfs_open_file *of = sysfs_of(file);
+
+ if (!of->vm_ops)
+ return;
+
+ if (!sysfs_get_active(of->sd))
+ return;
+
+ if (of->vm_ops->open)
+ of->vm_ops->open(vma);
+
+ sysfs_put_active(of->sd);
+}
+
+static int sysfs_bin_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+ struct file *file = vma->vm_file;
+ struct sysfs_open_file *of = sysfs_of(file);
+ int ret;
+
+ if (!of->vm_ops)
+ return VM_FAULT_SIGBUS;
+
+ if (!sysfs_get_active(of->sd))
+ return VM_FAULT_SIGBUS;
+
+ ret = VM_FAULT_SIGBUS;
+ if (of->vm_ops->fault)
+ ret = of->vm_ops->fault(vma, vmf);
+
+ sysfs_put_active(of->sd);
+ return ret;
+}
+
+static int sysfs_bin_page_mkwrite(struct vm_area_struct *vma,
+ struct vm_fault *vmf)
+{
+ struct file *file = vma->vm_file;
+ struct sysfs_open_file *of = sysfs_of(file);
+ int ret;
+
+ if (!of->vm_ops)
+ return VM_FAULT_SIGBUS;
+
+ if (!sysfs_get_active(of->sd))
+ return VM_FAULT_SIGBUS;
+
+ ret = 0;
+ if (of->vm_ops->page_mkwrite)
+ ret = of->vm_ops->page_mkwrite(vma, vmf);
+ else
+ file_update_time(file);
+
+ sysfs_put_active(of->sd);
+ return ret;
+}
+
+static int sysfs_bin_access(struct vm_area_struct *vma, unsigned long addr,
+ void *buf, int len, int write)
+{
+ struct file *file = vma->vm_file;
+ struct sysfs_open_file *of = sysfs_of(file);
+ int ret;
+
+ if (!of->vm_ops)
+ return -EINVAL;
+
+ if (!sysfs_get_active(of->sd))
+ return -EINVAL;
+
+ ret = -EINVAL;
+ if (of->vm_ops->access)
+ ret = of->vm_ops->access(vma, addr, buf, len, write);
+
+ sysfs_put_active(of->sd);
+ return ret;
+}
+
+#ifdef CONFIG_NUMA
+static int sysfs_bin_set_policy(struct vm_area_struct *vma,
+ struct mempolicy *new)
+{
+ struct file *file = vma->vm_file;
+ struct sysfs_open_file *of = sysfs_of(file);
+ int ret;
+
+ if (!of->vm_ops)
+ return 0;
+
+ if (!sysfs_get_active(of->sd))
+ return -EINVAL;
+
+ ret = 0;
+ if (of->vm_ops->set_policy)
+ ret = of->vm_ops->set_policy(vma, new);
+
+ sysfs_put_active(of->sd);
+ return ret;
+}
+
+static struct mempolicy *sysfs_bin_get_policy(struct vm_area_struct *vma,
+ unsigned long addr)
+{
+ struct file *file = vma->vm_file;
+ struct sysfs_open_file *of = sysfs_of(file);
+ struct mempolicy *pol;
+
+ if (!of->vm_ops)
+ return vma->vm_policy;
+
+ if (!sysfs_get_active(of->sd))
+ return vma->vm_policy;
+
+ pol = vma->vm_policy;
+ if (of->vm_ops->get_policy)
+ pol = of->vm_ops->get_policy(vma, addr);
+
+ sysfs_put_active(of->sd);
+ return pol;
+}
+
+static int sysfs_bin_migrate(struct vm_area_struct *vma, const nodemask_t *from,
+ const nodemask_t *to, unsigned long flags)
+{
+ struct file *file = vma->vm_file;
+ struct sysfs_open_file *of = sysfs_of(file);
+ int ret;
+
+ if (!of->vm_ops)
+ return 0;
+
+ if (!sysfs_get_active(of->sd))
+ return 0;
+
+ ret = 0;
+ if (of->vm_ops->migrate)
+ ret = of->vm_ops->migrate(vma, from, to, flags);
+
+ sysfs_put_active(of->sd);
+ return ret;
+}
+#endif
+
+static const struct vm_operations_struct sysfs_bin_vm_ops = {
+ .open = sysfs_bin_vma_open,
+ .fault = sysfs_bin_fault,
+ .page_mkwrite = sysfs_bin_page_mkwrite,
+ .access = sysfs_bin_access,
+#ifdef CONFIG_NUMA
+ .set_policy = sysfs_bin_set_policy,
+ .get_policy = sysfs_bin_get_policy,
+ .migrate = sysfs_bin_migrate,
+#endif
+};
+
+static int sysfs_bin_mmap(struct file *file, struct vm_area_struct *vma)
+{
+ struct sysfs_open_file *of = sysfs_of(file);
+ struct bin_attribute *battr = of->sd->s_bin_attr.bin_attr;
+ struct kobject *kobj = of->sd->s_parent->s_dir.kobj;
+ int rc;
+
+ if (!sysfs_is_bin(of->sd))
+ return -ENODEV;
+
+ mutex_lock(&of->mutex);
+
+ /* need of->sd for battr, its parent for kobj */
+ rc = -ENODEV;
+ if (!sysfs_get_active(of->sd))
+ goto out_unlock;
+
+ rc = -EINVAL;
+ if (!battr->mmap)
+ goto out_put;
+
+ rc = battr->mmap(file, kobj, battr, vma);
+ if (rc)
+ goto out_put;
+
+ /*
+ * PowerPC's pci_mmap of legacy_mem uses shmem_zero_setup()
+ * to satisfy versions of X which crash if the mmap fails: that
+ * substitutes a new vm_file, and we don't then want bin_vm_ops.
+ */
+ if (vma->vm_file != file)
+ goto out_put;
+
+ rc = -EINVAL;
+ if (of->mmapped && of->vm_ops != vma->vm_ops)
+ goto out_put;
+
+ /*
+ * It is not possible to successfully wrap close.
+ * So error if someone is trying to use close.
+ */
+ rc = -EINVAL;
+ if (vma->vm_ops && vma->vm_ops->close)
+ goto out_put;
+
+ rc = 0;
+ of->mmapped = 1;
+ of->vm_ops = vma->vm_ops;
+ vma->vm_ops = &sysfs_bin_vm_ops;
+out_put:
+ sysfs_put_active(of->sd);
+out_unlock:
+ mutex_unlock(&of->mutex);
+
+ return rc;
+}
+
/**
* sysfs_get_open_dirent - get or create sysfs_open_dirent
* @sd: target sysfs_dirent
@@ -400,7 +619,9 @@ static void sysfs_put_open_dirent(struct sysfs_dirent *sd,
mutex_lock(&sysfs_open_file_mutex);
spin_lock_irqsave(&sysfs_open_dirent_lock, flags);
- list_del(&of->list);
+ if (of)
+ list_del(&of->list);
+
if (atomic_dec_and_test(&od->refcnt))
sd->s_attr.open = NULL;
else
@@ -502,6 +723,32 @@ static int sysfs_release(struct inode *inode, struct file *filp)
return 0;
}
+void sysfs_unmap_bin_file(struct sysfs_dirent *sd)
+{
+ struct sysfs_open_dirent *od;
+ struct sysfs_open_file *of;
+
+ if (!sysfs_is_bin(sd))
+ return;
+
+ spin_lock_irq(&sysfs_open_dirent_lock);
+ od = sd->s_attr.open;
+ if (od)
+ atomic_inc(&od->refcnt);
+ spin_unlock_irq(&sysfs_open_dirent_lock);
+ if (!od)
+ return;
+
+ mutex_lock(&sysfs_open_file_mutex);
+ list_for_each_entry(of, &od->files, list) {
+ struct inode *inode = file_inode(of->file);
+ unmap_mapping_range(inode->i_mapping, 0, 0, 1);
+ }
+ mutex_unlock(&sysfs_open_file_mutex);
+
+ sysfs_put_open_dirent(sd, NULL);
+}
+
/* Sysfs attribute files are pollable. The idea is that you read
* the content and then you use 'poll' or 'select' to wait for
* the content to change. When the content changes (assuming the
@@ -581,6 +828,7 @@ const struct file_operations sysfs_file_operations = {
.open = sysfs_open_file,
.release = sysfs_release,
.poll = sysfs_poll,
+ .mmap = sysfs_bin_mmap,
};
int sysfs_add_file_mode_ns(struct sysfs_dirent *dir_sd,
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index 4b1d825..b6444ec 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -219,6 +219,8 @@ int sysfs_add_file(struct sysfs_dirent *dir_sd,
int sysfs_add_file_mode_ns(struct sysfs_dirent *dir_sd,
const struct attribute *attr, int type,
umode_t amode, const void *ns);
+void sysfs_unmap_bin_file(struct sysfs_dirent *sd);
+
/*
* bin.c
*/
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 13/14] sysfs: prepare open path for unified regular / bin file handling
2013-09-28 21:49 [PATCHSET] sysfs: use seq_file and unify regular and bin file handling Tejun Heo
` (11 preceding siblings ...)
2013-09-28 21:49 ` [PATCH 12/14] sysfs: copy bin mmap support from fs/sysfs/bin.c to fs/sysfs/file.c Tejun Heo
@ 2013-09-28 21:49 ` Tejun Heo
2013-09-28 21:49 ` [PATCH 14/14] sysfs: merge regular and " Tejun Heo
2013-09-28 22:15 ` [PATCHSET] sysfs: use seq_file and unify " Tejun Heo
14 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2013-09-28 21:49 UTC (permalink / raw)
To: gregkh; +Cc: kay, linux-kernel, ebiederm, Tejun Heo
sysfs bin file handling will be merged into the regular file support.
All three access paths - read, write and mmap - can now handle both
regular and bin files. This patch updates sysfs_open_file() and
sysfs_release() such that they can handle both regular and bin files.
This is a preparation and the new bin file path isn't used yet.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
fs/sysfs/file.c | 61 ++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 37 insertions(+), 24 deletions(-)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index fe5c440..723f78c 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -638,38 +638,40 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
struct kobject *kobj = attr_sd->s_parent->s_dir.kobj;
struct sysfs_open_file *of;
- const struct sysfs_ops *ops;
+ bool has_read, has_write;
int error = -EACCES;
/* need attr_sd for attr and ops, its parent for kobj */
if (!sysfs_get_active(attr_sd))
return -ENODEV;
- /* every kobject with an attribute needs a ktype assigned */
- ops = sysfs_file_ops(attr_sd);
- if (WARN(!ops, KERN_ERR
- "missing sysfs attribute operations for kobject: %s\n",
- kobject_name(kobj)))
- goto err_out;
+ if (sysfs_is_bin(attr_sd)) {
+ struct bin_attribute *battr = attr_sd->s_bin_attr.bin_attr;
- /* File needs write support.
- * The inode's perms must say it's ok,
- * and we must have a store method.
- */
- if (file->f_mode & FMODE_WRITE) {
- if (!(inode->i_mode & S_IWUGO) || !ops->store)
- goto err_out;
- }
+ has_read = battr->read || battr->mmap;
+ has_write = battr->write || battr->mmap;
+ } else {
+ const struct sysfs_ops *ops = sysfs_file_ops(attr_sd);
- /* File needs read support.
- * The inode's perms must say it's ok, and we there
- * must be a show method for it.
- */
- if (file->f_mode & FMODE_READ) {
- if (!(inode->i_mode & S_IRUGO) || !ops->show)
+ /* every kobject with an attribute needs a ktype assigned */
+ if (WARN(!ops, KERN_ERR
+ "missing sysfs attribute operations for kobject: %s\n",
+ kobject_name(kobj)))
goto err_out;
+
+ has_read = ops->show;
+ has_write = ops->store;
}
+ /* check perms and supported operations */
+ if ((file->f_mode & FMODE_WRITE) &&
+ (!(inode->i_mode & S_IWUGO) || !has_write))
+ goto err_out;
+
+ if ((file->f_mode & FMODE_READ) &&
+ (!(inode->i_mode & S_IRUGO) || !has_read))
+ goto err_out;
+
/* allocate a sysfs_open_file for the file */
error = -ENOMEM;
of = kzalloc(sizeof(struct sysfs_open_file), GFP_KERNEL);
@@ -685,10 +687,15 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
* implemented or requested. This unifies private data access and
* most files are readable anyway.
*/
- error = single_open(file, sysfs_seq_show, of);
+ if (sysfs_is_bin(attr_sd))
+ error = seq_open(file, &sysfs_bin_seq_ops);
+ else
+ error = single_open(file, sysfs_seq_show, NULL);
if (error)
goto err_free;
+ ((struct seq_file *)file->private_data)->private = of;
+
/* seq_file clears PWRITE unconditionally, restore it if WRITE */
if (file->f_mode & FMODE_WRITE)
file->f_mode |= FMODE_PWRITE;
@@ -703,7 +710,10 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
return 0;
err_close:
- single_release(inode, file);
+ if (sysfs_is_bin(attr_sd))
+ seq_release(inode, file);
+ else
+ single_release(inode, file);
err_free:
kfree(of);
err_out:
@@ -717,7 +727,10 @@ static int sysfs_release(struct inode *inode, struct file *filp)
struct sysfs_open_file *of = sysfs_of(filp);
sysfs_put_open_dirent(sd, of);
- single_release(inode, filp);
+ if (sysfs_is_bin(sd))
+ seq_release(inode, filp);
+ else
+ single_release(inode, filp);
kfree(of);
return 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 14/14] sysfs: merge regular and bin file handling
2013-09-28 21:49 [PATCHSET] sysfs: use seq_file and unify regular and bin file handling Tejun Heo
` (12 preceding siblings ...)
2013-09-28 21:49 ` [PATCH 13/14] sysfs: prepare open path for unified regular / bin file handling Tejun Heo
@ 2013-09-28 21:49 ` Tejun Heo
2013-09-28 22:15 ` [PATCHSET] sysfs: use seq_file and unify " Tejun Heo
14 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2013-09-28 21:49 UTC (permalink / raw)
To: gregkh; +Cc: kay, linux-kernel, ebiederm, Tejun Heo
With the previous changes, sysfs regular file code is ready to handle
bin files too. This patch makes bin files share the regular file
path.
* sysfs_create/remove_bin_file() are moved to fs/sysfs/file.c.
* sysfs_init_inode() is updated to use sysfs_file_operations for both
regular and bin files.
* fs/sysfs/bin.c and the related pieces are removed.
This patch introduces subtle behavior changes in extreme corner cases.
Please refer to the preceding preparation patches which added bin file
handling to regular file paths for details.
Overall, this unification reduces the amount of duplicate logic, makes
behaviors more consistent and paves the road for building simpler and
more versatile interface which will allow other subsystems to make use
of sysfs for their pseudo filesystems.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Kay Sievers <kay@vrfy.org>
---
fs/sysfs/Makefile | 3 +-
fs/sysfs/bin.c | 502 ------------------------------------------------------
fs/sysfs/dir.c | 1 -
fs/sysfs/file.c | 26 +++
fs/sysfs/inode.c | 2 +-
fs/sysfs/sysfs.h | 6 -
6 files changed, 28 insertions(+), 512 deletions(-)
delete mode 100644 fs/sysfs/bin.c
diff --git a/fs/sysfs/Makefile b/fs/sysfs/Makefile
index 7a1ceb9..8876ac1 100644
--- a/fs/sysfs/Makefile
+++ b/fs/sysfs/Makefile
@@ -2,5 +2,4 @@
# Makefile for the sysfs virtual filesystem
#
-obj-y := inode.o file.o dir.o symlink.o mount.o bin.o \
- group.o
+obj-y := inode.o file.o dir.o symlink.o mount.o group.o
diff --git a/fs/sysfs/bin.c b/fs/sysfs/bin.c
deleted file mode 100644
index d49e6ca..0000000
--- a/fs/sysfs/bin.c
+++ /dev/null
@@ -1,502 +0,0 @@
-/*
- * fs/sysfs/bin.c - sysfs binary file implementation
- *
- * Copyright (c) 2003 Patrick Mochel
- * Copyright (c) 2003 Matthew Wilcox
- * Copyright (c) 2004 Silicon Graphics, Inc.
- * Copyright (c) 2007 SUSE Linux Products GmbH
- * Copyright (c) 2007 Tejun Heo <teheo@suse.de>
- *
- * This file is released under the GPLv2.
- *
- * Please see Documentation/filesystems/sysfs.txt for more information.
- */
-
-#undef DEBUG
-
-#include <linux/errno.h>
-#include <linux/fs.h>
-#include <linux/kernel.h>
-#include <linux/kobject.h>
-#include <linux/module.h>
-#include <linux/slab.h>
-#include <linux/mutex.h>
-#include <linux/mm.h>
-#include <linux/uaccess.h>
-
-#include "sysfs.h"
-
-/*
- * There's one bin_buffer for each open file.
- *
- * filp->private_data points to bin_buffer and
- * sysfs_dirent->s_bin_attr.buffers points to a the bin_buffer s
- * sysfs_dirent->s_bin_attr.buffers is protected by sysfs_bin_lock
- */
-static DEFINE_MUTEX(sysfs_bin_lock);
-
-struct bin_buffer {
- struct mutex mutex;
- void *buffer;
- int mmapped;
- const struct vm_operations_struct *vm_ops;
- struct file *file;
- struct hlist_node list;
-};
-
-static int
-fill_read(struct file *file, char *buffer, loff_t off, size_t count)
-{
- struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
- struct bin_attribute *attr = attr_sd->s_bin_attr.bin_attr;
- struct kobject *kobj = attr_sd->s_parent->s_dir.kobj;
- int rc;
-
- /* need attr_sd for attr, its parent for kobj */
- if (!sysfs_get_active(attr_sd))
- return -ENODEV;
-
- rc = -EIO;
- if (attr->read)
- rc = attr->read(file, kobj, attr, buffer, off, count);
-
- sysfs_put_active(attr_sd);
-
- return rc;
-}
-
-static ssize_t
-read(struct file *file, char __user *userbuf, size_t bytes, loff_t *off)
-{
- struct bin_buffer *bb = file->private_data;
- int size = file_inode(file)->i_size;
- loff_t offs = *off;
- int count = min_t(size_t, bytes, PAGE_SIZE);
- char *temp;
-
- if (!bytes)
- return 0;
-
- if (size) {
- if (offs > size)
- return 0;
- if (offs + count > size)
- count = size - offs;
- }
-
- temp = kmalloc(count, GFP_KERNEL);
- if (!temp)
- return -ENOMEM;
-
- mutex_lock(&bb->mutex);
-
- count = fill_read(file, bb->buffer, offs, count);
- if (count < 0) {
- mutex_unlock(&bb->mutex);
- goto out_free;
- }
-
- memcpy(temp, bb->buffer, count);
-
- mutex_unlock(&bb->mutex);
-
- if (copy_to_user(userbuf, temp, count)) {
- count = -EFAULT;
- goto out_free;
- }
-
- pr_debug("offs = %lld, *off = %lld, count = %d\n", offs, *off, count);
-
- *off = offs + count;
-
- out_free:
- kfree(temp);
- return count;
-}
-
-static int
-flush_write(struct file *file, char *buffer, loff_t offset, size_t count)
-{
- struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
- struct bin_attribute *attr = attr_sd->s_bin_attr.bin_attr;
- struct kobject *kobj = attr_sd->s_parent->s_dir.kobj;
- int rc;
-
- /* need attr_sd for attr, its parent for kobj */
- if (!sysfs_get_active(attr_sd))
- return -ENODEV;
-
- rc = -EIO;
- if (attr->write)
- rc = attr->write(file, kobj, attr, buffer, offset, count);
-
- sysfs_put_active(attr_sd);
-
- return rc;
-}
-
-static ssize_t write(struct file *file, const char __user *userbuf,
- size_t bytes, loff_t *off)
-{
- struct bin_buffer *bb = file->private_data;
- int size = file_inode(file)->i_size;
- loff_t offs = *off;
- int count = min_t(size_t, bytes, PAGE_SIZE);
- char *temp;
-
- if (!bytes)
- return 0;
-
- if (size) {
- if (offs > size)
- return 0;
- if (offs + count > size)
- count = size - offs;
- }
-
- temp = memdup_user(userbuf, count);
- if (IS_ERR(temp))
- return PTR_ERR(temp);
-
- mutex_lock(&bb->mutex);
-
- memcpy(bb->buffer, temp, count);
-
- count = flush_write(file, bb->buffer, offs, count);
- mutex_unlock(&bb->mutex);
-
- if (count > 0)
- *off = offs + count;
-
- kfree(temp);
- return count;
-}
-
-static void bin_vma_open(struct vm_area_struct *vma)
-{
- struct file *file = vma->vm_file;
- struct bin_buffer *bb = file->private_data;
- struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
-
- if (!bb->vm_ops)
- return;
-
- if (!sysfs_get_active(attr_sd))
- return;
-
- if (bb->vm_ops->open)
- bb->vm_ops->open(vma);
-
- sysfs_put_active(attr_sd);
-}
-
-static int bin_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
-{
- struct file *file = vma->vm_file;
- struct bin_buffer *bb = file->private_data;
- struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
- int ret;
-
- if (!bb->vm_ops)
- return VM_FAULT_SIGBUS;
-
- if (!sysfs_get_active(attr_sd))
- return VM_FAULT_SIGBUS;
-
- ret = VM_FAULT_SIGBUS;
- if (bb->vm_ops->fault)
- ret = bb->vm_ops->fault(vma, vmf);
-
- sysfs_put_active(attr_sd);
- return ret;
-}
-
-static int bin_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
-{
- struct file *file = vma->vm_file;
- struct bin_buffer *bb = file->private_data;
- struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
- int ret;
-
- if (!bb->vm_ops)
- return VM_FAULT_SIGBUS;
-
- if (!sysfs_get_active(attr_sd))
- return VM_FAULT_SIGBUS;
-
- ret = 0;
- if (bb->vm_ops->page_mkwrite)
- ret = bb->vm_ops->page_mkwrite(vma, vmf);
- else
- file_update_time(file);
-
- sysfs_put_active(attr_sd);
- return ret;
-}
-
-static int bin_access(struct vm_area_struct *vma, unsigned long addr,
- void *buf, int len, int write)
-{
- struct file *file = vma->vm_file;
- struct bin_buffer *bb = file->private_data;
- struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
- int ret;
-
- if (!bb->vm_ops)
- return -EINVAL;
-
- if (!sysfs_get_active(attr_sd))
- return -EINVAL;
-
- ret = -EINVAL;
- if (bb->vm_ops->access)
- ret = bb->vm_ops->access(vma, addr, buf, len, write);
-
- sysfs_put_active(attr_sd);
- return ret;
-}
-
-#ifdef CONFIG_NUMA
-static int bin_set_policy(struct vm_area_struct *vma, struct mempolicy *new)
-{
- struct file *file = vma->vm_file;
- struct bin_buffer *bb = file->private_data;
- struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
- int ret;
-
- if (!bb->vm_ops)
- return 0;
-
- if (!sysfs_get_active(attr_sd))
- return -EINVAL;
-
- ret = 0;
- if (bb->vm_ops->set_policy)
- ret = bb->vm_ops->set_policy(vma, new);
-
- sysfs_put_active(attr_sd);
- return ret;
-}
-
-static struct mempolicy *bin_get_policy(struct vm_area_struct *vma,
- unsigned long addr)
-{
- struct file *file = vma->vm_file;
- struct bin_buffer *bb = file->private_data;
- struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
- struct mempolicy *pol;
-
- if (!bb->vm_ops)
- return vma->vm_policy;
-
- if (!sysfs_get_active(attr_sd))
- return vma->vm_policy;
-
- pol = vma->vm_policy;
- if (bb->vm_ops->get_policy)
- pol = bb->vm_ops->get_policy(vma, addr);
-
- sysfs_put_active(attr_sd);
- return pol;
-}
-
-static int bin_migrate(struct vm_area_struct *vma, const nodemask_t *from,
- const nodemask_t *to, unsigned long flags)
-{
- struct file *file = vma->vm_file;
- struct bin_buffer *bb = file->private_data;
- struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
- int ret;
-
- if (!bb->vm_ops)
- return 0;
-
- if (!sysfs_get_active(attr_sd))
- return 0;
-
- ret = 0;
- if (bb->vm_ops->migrate)
- ret = bb->vm_ops->migrate(vma, from, to, flags);
-
- sysfs_put_active(attr_sd);
- return ret;
-}
-#endif
-
-static const struct vm_operations_struct bin_vm_ops = {
- .open = bin_vma_open,
- .fault = bin_fault,
- .page_mkwrite = bin_page_mkwrite,
- .access = bin_access,
-#ifdef CONFIG_NUMA
- .set_policy = bin_set_policy,
- .get_policy = bin_get_policy,
- .migrate = bin_migrate,
-#endif
-};
-
-static int mmap(struct file *file, struct vm_area_struct *vma)
-{
- struct bin_buffer *bb = file->private_data;
- struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
- struct bin_attribute *attr = attr_sd->s_bin_attr.bin_attr;
- struct kobject *kobj = attr_sd->s_parent->s_dir.kobj;
- int rc;
-
- mutex_lock(&bb->mutex);
-
- /* need attr_sd for attr, its parent for kobj */
- rc = -ENODEV;
- if (!sysfs_get_active(attr_sd))
- goto out_unlock;
-
- rc = -EINVAL;
- if (!attr->mmap)
- goto out_put;
-
- rc = attr->mmap(file, kobj, attr, vma);
- if (rc)
- goto out_put;
-
- /*
- * PowerPC's pci_mmap of legacy_mem uses shmem_zero_setup()
- * to satisfy versions of X which crash if the mmap fails: that
- * substitutes a new vm_file, and we don't then want bin_vm_ops.
- */
- if (vma->vm_file != file)
- goto out_put;
-
- rc = -EINVAL;
- if (bb->mmapped && bb->vm_ops != vma->vm_ops)
- goto out_put;
-
- /*
- * It is not possible to successfully wrap close.
- * So error if someone is trying to use close.
- */
- rc = -EINVAL;
- if (vma->vm_ops && vma->vm_ops->close)
- goto out_put;
-
- rc = 0;
- bb->mmapped = 1;
- bb->vm_ops = vma->vm_ops;
- vma->vm_ops = &bin_vm_ops;
-out_put:
- sysfs_put_active(attr_sd);
-out_unlock:
- mutex_unlock(&bb->mutex);
-
- return rc;
-}
-
-static int open(struct inode *inode, struct file *file)
-{
- struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
- struct bin_attribute *attr = attr_sd->s_bin_attr.bin_attr;
- struct bin_buffer *bb = NULL;
- int error;
-
- /* binary file operations requires both @sd and its parent */
- if (!sysfs_get_active(attr_sd))
- return -ENODEV;
-
- error = -EACCES;
- if ((file->f_mode & FMODE_WRITE) && !(attr->write || attr->mmap))
- goto err_out;
- if ((file->f_mode & FMODE_READ) && !(attr->read || attr->mmap))
- goto err_out;
-
- error = -ENOMEM;
- bb = kzalloc(sizeof(*bb), GFP_KERNEL);
- if (!bb)
- goto err_out;
-
- bb->buffer = kmalloc(PAGE_SIZE, GFP_KERNEL);
- if (!bb->buffer)
- goto err_out;
-
- mutex_init(&bb->mutex);
- bb->file = file;
- file->private_data = bb;
-
- mutex_lock(&sysfs_bin_lock);
- hlist_add_head(&bb->list, &attr_sd->s_bin_attr.buffers);
- mutex_unlock(&sysfs_bin_lock);
-
- /* open succeeded, put active references */
- sysfs_put_active(attr_sd);
- return 0;
-
- err_out:
- sysfs_put_active(attr_sd);
- kfree(bb);
- return error;
-}
-
-static int release(struct inode *inode, struct file *file)
-{
- struct bin_buffer *bb = file->private_data;
-
- mutex_lock(&sysfs_bin_lock);
- hlist_del(&bb->list);
- mutex_unlock(&sysfs_bin_lock);
-
- kfree(bb->buffer);
- kfree(bb);
- return 0;
-}
-
-const struct file_operations bin_fops = {
- .read = read,
- .write = write,
- .mmap = mmap,
- .llseek = generic_file_llseek,
- .open = open,
- .release = release,
-};
-
-
-void unmap_bin_file(struct sysfs_dirent *attr_sd)
-{
- struct bin_buffer *bb;
-
- if (sysfs_type(attr_sd) != SYSFS_KOBJ_BIN_ATTR)
- return;
-
- mutex_lock(&sysfs_bin_lock);
-
- hlist_for_each_entry(bb, &attr_sd->s_bin_attr.buffers, list) {
- struct inode *inode = file_inode(bb->file);
-
- unmap_mapping_range(inode->i_mapping, 0, 0, 1);
- }
-
- mutex_unlock(&sysfs_bin_lock);
-}
-
-/**
- * sysfs_create_bin_file - create binary file for object.
- * @kobj: object.
- * @attr: attribute descriptor.
- */
-int sysfs_create_bin_file(struct kobject *kobj,
- const struct bin_attribute *attr)
-{
- BUG_ON(!kobj || !kobj->sd || !attr);
-
- return sysfs_add_file(kobj->sd, &attr->attr, SYSFS_KOBJ_BIN_ATTR);
-}
-EXPORT_SYMBOL_GPL(sysfs_create_bin_file);
-
-/**
- * sysfs_remove_bin_file - remove binary file for object.
- * @kobj: object.
- * @attr: attribute descriptor.
- */
-void sysfs_remove_bin_file(struct kobject *kobj,
- const struct bin_attribute *attr)
-{
- sysfs_hash_and_remove(kobj->sd, attr->attr.name, NULL);
-}
-EXPORT_SYMBOL_GPL(sysfs_remove_bin_file);
diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index c4040dd..f6025c8 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -596,7 +596,6 @@ void sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt)
sysfs_deactivate(sd);
sysfs_unmap_bin_file(sd);
- unmap_bin_file(sd);
sysfs_put(sd);
}
}
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 723f78c..f3cbdbd 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -1014,6 +1014,32 @@ void sysfs_remove_file_from_group(struct kobject *kobj,
}
EXPORT_SYMBOL_GPL(sysfs_remove_file_from_group);
+/**
+ * sysfs_create_bin_file - create binary file for object.
+ * @kobj: object.
+ * @attr: attribute descriptor.
+ */
+int sysfs_create_bin_file(struct kobject *kobj,
+ const struct bin_attribute *attr)
+{
+ BUG_ON(!kobj || !kobj->sd || !attr);
+
+ return sysfs_add_file(kobj->sd, &attr->attr, SYSFS_KOBJ_BIN_ATTR);
+}
+EXPORT_SYMBOL_GPL(sysfs_create_bin_file);
+
+/**
+ * sysfs_remove_bin_file - remove binary file for object.
+ * @kobj: object.
+ * @attr: attribute descriptor.
+ */
+void sysfs_remove_bin_file(struct kobject *kobj,
+ const struct bin_attribute *attr)
+{
+ sysfs_hash_and_remove(kobj->sd, attr->attr.name, NULL);
+}
+EXPORT_SYMBOL_GPL(sysfs_remove_bin_file);
+
struct sysfs_schedule_callback_struct {
struct list_head workq_list;
struct kobject *kobj;
diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index 63f755e..fe274a3 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -260,7 +260,7 @@ static void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode)
case SYSFS_KOBJ_BIN_ATTR:
bin_attr = sd->s_bin_attr.bin_attr;
inode->i_size = bin_attr->size;
- inode->i_fop = &bin_fops;
+ inode->i_fop = &sysfs_file_operations;
break;
case SYSFS_KOBJ_LINK:
inode->i_op = &sysfs_symlink_inode_operations;
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index b6444ec..94857dc 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -222,12 +222,6 @@ int sysfs_add_file_mode_ns(struct sysfs_dirent *dir_sd,
void sysfs_unmap_bin_file(struct sysfs_dirent *sd);
/*
- * bin.c
- */
-extern const struct file_operations bin_fops;
-void unmap_bin_file(struct sysfs_dirent *attr_sd);
-
-/*
* symlink.c
*/
extern const struct inode_operations sysfs_symlink_inode_operations;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCHSET] sysfs: use seq_file and unify regular and bin file handling
2013-09-28 21:49 [PATCHSET] sysfs: use seq_file and unify regular and bin file handling Tejun Heo
` (13 preceding siblings ...)
2013-09-28 21:49 ` [PATCH 14/14] sysfs: merge regular and " Tejun Heo
@ 2013-09-28 22:15 ` Tejun Heo
2013-09-30 19:54 ` Tejun Heo
2013-10-01 5:03 ` Bjorn Helgaas
14 siblings, 2 replies; 20+ messages in thread
From: Tejun Heo @ 2013-09-28 22:15 UTC (permalink / raw)
To: gregkh; +Cc: kay, linux-kernel, ebiederm
Hey, again.
On Sat, Sep 28, 2013 at 05:49:30PM -0400, Tejun Heo wrote:
> 0001-sysfs-remove-unused-sysfs_buffer-pos.patch
> 0002-sysfs-remove-sysfs_buffer-needs_read_fill.patch
> 0003-sysfs-remove-sysfs_buffer-ops.patch
> 0004-sysfs-add-sysfs_open_file_mutex.patch
> 0005-sysfs-rename-sysfs_buffer-to-sysfs_open_file.patch
> 0006-sysfs-add-sysfs_open_file-sd-and-file.patch
> 0007-sysfs-use-transient-write-buffer.patch
> 0008-sysfs-use-seq_file-when-reading-regular-files.patch
> 0009-sysfs-prepare-llseek-path-for-unified-regular-bin-fi.patch
> 0010-sysfs-prepare-path-write-for-unified-regular-bin-fil.patch
> 0011-sysfs-prepare-read-path-for-unified-regular-bin-file.patch
> 0012-sysfs-copy-bin-mmap-support-from-fs-sysfs-bin.c-to-f.patch
> 0013-sysfs-prepare-open-path-for-unified-regular-bin-file.patch
> 0014-sysfs-merge-regular-and-bin-file-handling.patch
On the second thought, 0011 seems too dangerous, especially for pci IO
BAR regions. Grumble, looks like I'll have to break out the bin read
path. Please ignore patches >= 0009. I'll update them and repost.
Thanks!
--
tejun
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCHSET] sysfs: use seq_file and unify regular and bin file handling
2013-09-28 22:15 ` [PATCHSET] sysfs: use seq_file and unify " Tejun Heo
@ 2013-09-30 19:54 ` Tejun Heo
2013-09-30 20:14 ` Greg KH
2013-10-01 5:03 ` Bjorn Helgaas
1 sibling, 1 reply; 20+ messages in thread
From: Tejun Heo @ 2013-09-30 19:54 UTC (permalink / raw)
To: gregkh; +Cc: kay, linux-kernel, ebiederm
Hello,
On Sat, Sep 28, 2013 at 06:15:13PM -0400, Tejun Heo wrote:
> On the second thought, 0011 seems too dangerous, especially for pci IO
> BAR regions. Grumble, looks like I'll have to break out the bin read
> path. Please ignore patches >= 0009. I'll update them and repost.
Please ignore the whole series for now. It seems like we'll need a
separate read path for the existing sysfs interface anyway and
seq_file interface should be added separately. Will post updated
patches soon.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCHSET] sysfs: use seq_file and unify regular and bin file handling
2013-09-30 19:54 ` Tejun Heo
@ 2013-09-30 20:14 ` Greg KH
0 siblings, 0 replies; 20+ messages in thread
From: Greg KH @ 2013-09-30 20:14 UTC (permalink / raw)
To: Tejun Heo; +Cc: kay, linux-kernel, ebiederm
On Mon, Sep 30, 2013 at 03:54:03PM -0400, Tejun Heo wrote:
> Hello,
>
> On Sat, Sep 28, 2013 at 06:15:13PM -0400, Tejun Heo wrote:
> > On the second thought, 0011 seems too dangerous, especially for pci IO
> > BAR regions. Grumble, looks like I'll have to break out the bin read
> > path. Please ignore patches >= 0009. I'll update them and repost.
>
> Please ignore the whole series for now. It seems like we'll need a
> separate read path for the existing sysfs interface anyway and
> seq_file interface should be added separately. Will post updated
> patches soon.
Ok, consider it ignored :)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCHSET] sysfs: use seq_file and unify regular and bin file handling
2013-09-28 22:15 ` [PATCHSET] sysfs: use seq_file and unify " Tejun Heo
2013-09-30 19:54 ` Tejun Heo
@ 2013-10-01 5:03 ` Bjorn Helgaas
2013-10-01 14:23 ` Tejun Heo
1 sibling, 1 reply; 20+ messages in thread
From: Bjorn Helgaas @ 2013-10-01 5:03 UTC (permalink / raw)
To: Tejun Heo
Cc: Greg Kroah-Hartman, Kay Sievers, linux-kernel@vger.kernel.org,
Eric W. Biederman
On Sat, Sep 28, 2013 at 4:15 PM, Tejun Heo <tj@kernel.org> wrote:
> Hey, again.
>
> On Sat, Sep 28, 2013 at 05:49:30PM -0400, Tejun Heo wrote:
>> 0001-sysfs-remove-unused-sysfs_buffer-pos.patch
>> 0002-sysfs-remove-sysfs_buffer-needs_read_fill.patch
>> 0003-sysfs-remove-sysfs_buffer-ops.patch
>> 0004-sysfs-add-sysfs_open_file_mutex.patch
>> 0005-sysfs-rename-sysfs_buffer-to-sysfs_open_file.patch
>> 0006-sysfs-add-sysfs_open_file-sd-and-file.patch
>> 0007-sysfs-use-transient-write-buffer.patch
>> 0008-sysfs-use-seq_file-when-reading-regular-files.patch
>> 0009-sysfs-prepare-llseek-path-for-unified-regular-bin-fi.patch
>> 0010-sysfs-prepare-path-write-for-unified-regular-bin-fil.patch
>> 0011-sysfs-prepare-read-path-for-unified-regular-bin-file.patch
>> 0012-sysfs-copy-bin-mmap-support-from-fs-sysfs-bin.c-to-f.patch
>> 0013-sysfs-prepare-open-path-for-unified-regular-bin-file.patch
>> 0014-sysfs-merge-regular-and-bin-file-handling.patch
>
> On the second thought, 0011 seems too dangerous, especially for pci IO
> BAR regions. Grumble, looks like I'll have to break out the bin read
> path. Please ignore patches >= 0009. I'll update them and repost.
I don't pretend to understand sysfs or the issue you tripped over with
PCI I/O BAR regions. But we had a long discussion about those files
[1] last spring, and I'm pretty convinced that it was a mistake to add
them in their current form, and I would support an attempt to rework
them. We had some ideas about how to do that, but I think everybody
lost interest before anything happened.
Bjorn
[1] http://lkml.kernel.org/r/20130316213519.2974.38954.stgit@amt.stowe
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCHSET] sysfs: use seq_file and unify regular and bin file handling
2013-10-01 5:03 ` Bjorn Helgaas
@ 2013-10-01 14:23 ` Tejun Heo
0 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2013-10-01 14:23 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Greg Kroah-Hartman, Kay Sievers, linux-kernel@vger.kernel.org,
Eric W. Biederman
Hello,
On Mon, Sep 30, 2013 at 11:03:50PM -0600, Bjorn Helgaas wrote:
> I don't pretend to understand sysfs or the issue you tripped over with
> PCI I/O BAR regions. But we had a long discussion about those files
The issue is rather simple. Let's say I do "dd if=SOME_IO_BAR skip=12
bs=4 count=1", it should result in exactly 4 byte read from the ioport
at BAR + 12 as io reads may have side effects; however, seq_file
breaks that with buffering. Pretty similar to using stdio on ioports.
> [1] last spring, and I'm pretty convinced that it was a mistake to add
> them in their current form, and I would support an attempt to rework
> them. We had some ideas about how to do that, but I think everybody
> lost interest before anything happened.
Yeah, I was pretty weirded out after finding out that the BARs are
directly accessible through sysfs. People run all sorts of scripts
over the sysfs hierarchy after all. That said, I don't think sysfs
can simply pull out bin file support at this point. I'll keep the
code path separate so that it can be easily separated out if we don't
need it later.
Thanks!
--
tejun
^ permalink raw reply [flat|nested] 20+ messages in thread