[PATCH] configfs: Implement binary attributes

[PATCH] configfs: Implement binary attributes

[Pantelis Antoniou]

ConfigFS lacked binary attributes up until now. This patch
introduces support for binary attributes in a somewhat similar
manner of sysfs binary attributes albeit with changes that
fit the configfs usage model.

Problems that configfs binary attributes fix are everything that
requires a binary blob as part of the configuration of a resource,
such as bitstream loading for FPGAs, DTBs for dynamically created
devices etc.

Look at Documentation/filesystems/configfs/configfs.txt for internals
and howto use them.

This patch is against linux-next as of today that contains
Christoph's configfs rework.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
---
 Documentation/filesystems/configfs/configfs.txt |  57 +++++-
 fs/configfs/configfs_internal.h                 |  14 +-
 fs/configfs/dir.c                               |  18 +-
 fs/configfs/file.c                              | 256 +++++++++++++++++++++++-
 fs/configfs/inode.c                             |   2 +-
 include/linux/configfs.h                        |  50 +++++
 6 files changed, 375 insertions(+), 22 deletions(-)

diff --git a/Documentation/filesystems/configfs/configfs.txt b/Documentation/filesystems/configfs/configfs.txt
index af68efd..e5fe521 100644
--- a/Documentation/filesystems/configfs/configfs.txt
+++ b/Documentation/filesystems/configfs/configfs.txt
@@ -51,15 +51,27 @@ configfs tree is always there, whether mounted on /config or not.
 An item is created via mkdir(2).  The item's attributes will also
 appear at this time.  readdir(3) can determine what the attributes are,
 read(2) can query their default values, and write(2) can store new
-values.  Like sysfs, attributes should be ASCII text files, preferably
-with only one value per file.  The same efficiency caveats from sysfs
-apply.  Don't mix more than one attribute in one attribute file.
-
-Like sysfs, configfs expects write(2) to store the entire buffer at
-once.  When writing to configfs attributes, userspace processes should
-first read the entire file, modify the portions they wish to change, and
-then write the entire buffer back.  Attribute files have a maximum size
-of one page (PAGE_SIZE, 4096 on i386).
+values.  Don't mix more than one attribute in one attribute file.
+
+There are two types of configfs attributes:
+
+* Normal attributes, which similar to sysfs attributes, are small ASCII text
+files, with a maximum size of one page (PAGE_SIZE, 4096 on i386).  Preferably
+only one value per file should be used, and the same caveats from sysfs apply.
+Configfs expects write(2) to store the entire buffer at once.  When writing to
+normal configfs attributes, userspace processes should first read the entire
+file, modify the portions they wish to change, and then write the entire
+buffer back.
+
+* Binary attributes, which are somewhat similar to sysfs binary attributes,
+but with a few slight changes to semantics.  The PAGE_SIZE limitation does not
+apply, but the whole binary item must fit in single kernel vmalloc'ed buffer.
+The write(2) calls from user space are buffered, and the attributes'
+write_bin_attribute method will be invoked on the final close, therefore it is
+imperative for user-space to check the return code of close(2) in order to
+verify that the operation finished successfully.
+To avoid a malicious user OOMing the kernel, there's a per-binary attribute
+maximum buffer value.
 
 When an item needs to be destroyed, remove it with rmdir(2).  An
 item cannot be destroyed if any other item has a link to it (via
@@ -171,6 +183,7 @@ among other things.  For that, it needs a type.
 		struct configfs_item_operations         *ct_item_ops;
 		struct configfs_group_operations        *ct_group_ops;
 		struct configfs_attribute               **ct_attrs;
+		struct configfs_bin_attribute		**ct_bin_attrs;
 	};
 
 The most basic function of a config_item_type is to define what
@@ -201,6 +214,32 @@ be called whenever userspace asks for a read(2) on the attribute.  If an
 attribute is writable and provides a ->store  method, that method will be
 be called whenever userspace asks for a write(2) on the attribute.
 
+[struct configfs_bin_attribute]
+
+	struct configfs_attribute {
+		struct configfs_attribute	cb_attr;
+		void				*cb_private;
+		size_t				cb_max_size;
+	};
+
+The binary attribute is used when the one needs to use binary blob to
+appear as the contents of a file in the item's configfs directory.
+To do so add the binary attribute to the NULL-terminated array
+config_item_type->ct_bin_attrs, and the item appears in configfs, the
+attribute file will appear with the configfs_bin_attribute->cb_attr.ca_name
+filename.  configfs_bin_attribute->cb_attr.ca_mode specifies the file
+permissions.
+The cb_private member is provided for use by the driver, while the
+cb_max_size member specifies the maximum amount of vmalloc buffer
+to be used.
+
+If binary attribute is readable and the config_item provides a
+ct_item_ops->read_bin_attribute() method, that method will be called
+whenever userspace asks for a read(2) on the attribute.  The converse
+will happen for write(2). The reads/writes are bufferred so only a
+single read/write will occur; the attributes' need not concern itself
+with it.
+
 [struct config_group]
 
 A config_item cannot live in a vacuum.  The only way one can be created
diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
index b65d1ef..ccc31fa 100644
--- a/fs/configfs/configfs_internal.h
+++ b/fs/configfs/configfs_internal.h
@@ -53,13 +53,14 @@ struct configfs_dirent {
 #define CONFIGFS_ROOT		0x0001
 #define CONFIGFS_DIR		0x0002
 #define CONFIGFS_ITEM_ATTR	0x0004
+#define CONFIGFS_ITEM_BIN_ATTR	0x0008
 #define CONFIGFS_ITEM_LINK	0x0020
 #define CONFIGFS_USET_DIR	0x0040
 #define CONFIGFS_USET_DEFAULT	0x0080
 #define CONFIGFS_USET_DROPPING	0x0100
 #define CONFIGFS_USET_IN_MKDIR	0x0200
 #define CONFIGFS_USET_CREATING	0x0400
-#define CONFIGFS_NOT_PINNED	(CONFIGFS_ITEM_ATTR)
+#define CONFIGFS_NOT_PINNED	(CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)
 
 extern struct mutex configfs_symlink_mutex;
 extern spinlock_t configfs_dirent_lock;
@@ -72,6 +73,8 @@ extern struct inode * configfs_new_inode(umode_t mode, struct configfs_dirent *,
 extern int configfs_create(struct dentry *, umode_t mode, void (*init)(struct inode *));
 
 extern int configfs_create_file(struct config_item *, const struct configfs_attribute *);
+extern int configfs_create_bin_file(struct config_item *,
+				    const struct configfs_bin_attribute *);
 extern int configfs_make_dirent(struct configfs_dirent *,
 				struct dentry *, void *, umode_t, int);
 extern int configfs_dirent_is_ready(struct configfs_dirent *);
@@ -88,7 +91,7 @@ extern void configfs_release_fs(void);
 extern struct rw_semaphore configfs_rename_sem;
 extern const struct file_operations configfs_dir_operations;
 extern const struct file_operations configfs_file_operations;
-extern const struct file_operations bin_fops;
+extern const struct file_operations configfs_bin_file_operations;
 extern const struct inode_operations configfs_dir_inode_operations;
 extern const struct inode_operations configfs_root_inode_operations;
 extern const struct inode_operations configfs_symlink_inode_operations;
@@ -119,6 +122,13 @@ static inline struct configfs_attribute * to_attr(struct dentry * dentry)
 	return ((struct configfs_attribute *) sd->s_element);
 }
 
+static inline struct configfs_bin_attribute *to_bin_attr(struct dentry *dentry)
+{
+	struct configfs_attribute *attr = to_attr(dentry);
+
+	return container_of(attr, struct configfs_bin_attribute, cb_attr);
+}
+
 static inline struct config_item *configfs_get_config_item(struct dentry *dentry)
 {
 	struct config_item * item = NULL;
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index c81ce7f..8951466 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -255,6 +255,12 @@ static void configfs_init_file(struct inode * inode)
 	inode->i_fop = &configfs_file_operations;
 }
 
+static void configfs_init_bin_file(struct inode *inode)
+{
+	inode->i_size = 0;
+	inode->i_fop = &configfs_bin_file_operations;
+}
+
 static void init_symlink(struct inode * inode)
 {
 	inode->i_op = &configfs_symlink_inode_operations;
@@ -423,7 +429,9 @@ static int configfs_attach_attr(struct configfs_dirent * sd, struct dentry * den
 	spin_unlock(&configfs_dirent_lock);
 
 	error = configfs_create(dentry, (attr->ca_mode & S_IALLUGO) | S_IFREG,
-				configfs_init_file);
+				(sd->s_type & CONFIGFS_ITEM_ATTR) ?
+					configfs_init_file :
+					configfs_init_bin_file);
 	if (error) {
 		configfs_put(sd);
 		return error;
@@ -583,6 +591,7 @@ static int populate_attrs(struct config_item *item)
 {
 	struct config_item_type *t = item->ci_type;
 	struct configfs_attribute *attr;
+	struct configfs_bin_attribute *bin_attr;
 	int error = 0;
 	int i;
 
@@ -594,6 +603,13 @@ static int populate_attrs(struct config_item *item)
 				break;
 		}
 	}
+	if (t->ct_bin_attrs) {
+		for (i = 0; (bin_attr = t->ct_bin_attrs[i]) != NULL; i++) {
+			error = configfs_create_bin_file(item, bin_attr);
+			if (error)
+				break;
+		}
+	}
 
 	if (error)
 		detach_attrs(item);
diff --git a/fs/configfs/file.c b/fs/configfs/file.c
index d39099e..a3a3c66 100644
--- a/fs/configfs/file.c
+++ b/fs/configfs/file.c
@@ -28,6 +28,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/mutex.h>
+#include <linux/vmalloc.h>
 #include <asm/uaccess.h>
 
 #include <linux/configfs.h>
@@ -48,6 +49,10 @@ struct configfs_buffer {
 	struct configfs_item_operations	* ops;
 	struct mutex		mutex;
 	int			needs_read_fill;
+	int			read_in_progress;
+	int			write_in_progress;
+	char			*bin_buffer;
+	int			bin_buffer_size;
 };
 
 
@@ -123,6 +128,88 @@ out:
 	return retval;
 }
 
+/**
+ *	configfs_read_bin_file - read a binary attribute.
+ *	@file:	file pointer.
+ *	@buf:	buffer to fill.
+ *	@count:	number of bytes to read.
+ *	@ppos:	starting offset in file.
+ *
+ *	Userspace wants to read a binary attribute file. The attribute
+ *	descriptor is in the file's ->d_fsdata. The target item is in the
+ *	directory's ->d_fsdata.
+ *
+ *	We check whether we need to refill the buffer. If so we will
+ *	call the attributes' attr->read() twice. The first time we
+ *	will pass a NULL as a buffer pointer, which the attributes' method
+ *	will use to return the size of the buffer required. If no error
+ *	occurs we will allocate the buffer using vmalloc and call
+ *	attr->read() again passing that buffer as an argument.
+ *	Then we just copy to user-space using simple_read_from_buffer.
+ */
+
+static ssize_t
+configfs_read_bin_file(struct file *file, char __user *buf,
+		       size_t count, loff_t *ppos)
+{
+	struct configfs_buffer *buffer = file->private_data;
+	struct dentry *dentry = file->f_path.dentry;
+	struct config_item *item = to_item(dentry->d_parent);
+	struct configfs_bin_attribute *bin_attr = to_bin_attr(dentry);
+	ssize_t retval = 0;
+	ssize_t len = min_t(size_t, count, PAGE_SIZE);
+
+	mutex_lock(&buffer->mutex);
+
+	/* we don't support switching read/write modes */
+	if (buffer->write_in_progress) {
+		retval = -EINVAL;
+		goto out;
+	}
+	buffer->read_in_progress = 1;
+
+	if (buffer->needs_read_fill) {
+
+		/* perform first read with buf == NULL to get extent */
+		len = bin_attr->read(item, NULL, 0);
+		if (len < 0) {
+			retval = len;
+			goto out;
+		}
+
+		/* do not exceed the maximum value */
+		if (bin_attr->cb_max_size && len > bin_attr->cb_max_size) {
+			retval = -EFBIG;
+			goto out;
+		}
+
+		buffer->bin_buffer = vmalloc(len);
+		if (buffer->bin_buffer == NULL) {
+			retval = -ENOMEM;
+			goto out;
+		}
+		buffer->bin_buffer_size = len;
+
+		/* perform second read to fill buffer */
+		len = bin_attr->read(item, buffer->bin_buffer, len);
+		if (len < 0) {
+			retval = len;
+			vfree(buffer->bin_buffer);
+			buffer->bin_buffer_size = 0;
+			buffer->bin_buffer = NULL;
+			goto out;
+		}
+
+		buffer->needs_read_fill = 0;
+	}
+
+	retval = simple_read_from_buffer(buf, count, ppos, buffer->bin_buffer,
+					buffer->bin_buffer_size);
+out:
+	mutex_unlock(&buffer->mutex);
+	return retval;
+}
+
 
 /**
  *	fill_write_buffer - copy buffer from userspace.
@@ -209,10 +296,80 @@ configfs_write_file(struct file *file, const char __user *buf, size_t count, lof
 	return len;
 }
 
-static int check_perm(struct inode * inode, struct file * file)
+/**
+ *	configfs_write_bin_file - write a binary attribute.
+ *	@file:	file pointer
+ *	@buf:	data to write
+ *	@count:	number of bytes
+ *	@ppos:	starting offset
+ *
+ *	Writing to a binary attribute file is similar to a normal read.
+ *	We buffer the consecutive writes (binary attribute files do not
+ *	support lseek) in a continuously growing buffer, but we don't
+ *	commit until the close of the file.
+ */
+
+static ssize_t
+configfs_write_bin_file(struct file *file, const char __user *buf,
+			size_t count, loff_t *ppos)
+{
+	struct configfs_buffer *buffer = file->private_data;
+	struct dentry *dentry = file->f_path.dentry;
+	struct configfs_bin_attribute *bin_attr = to_bin_attr(dentry);
+	void *tbuf = NULL;
+	ssize_t len;
+
+	mutex_lock(&buffer->mutex);
+
+	/* we don't support switching read/write modes */
+	if (buffer->read_in_progress) {
+		len = -EINVAL;
+		goto out;
+	}
+	buffer->write_in_progress = 1;
+
+	/* buffer grows? */
+	if (*ppos + count > buffer->bin_buffer_size) {
+
+		if (bin_attr->cb_max_size &&
+			*ppos + count > bin_attr->cb_max_size) {
+			len = -EFBIG;
+		}
+
+		tbuf = vmalloc(*ppos + count);
+		if (tbuf == NULL) {
+			len = -ENOMEM;
+			goto out;
+		}
+
+		/* copy old contents */
+		if (buffer->bin_buffer) {
+			memcpy(tbuf, buffer->bin_buffer,
+				buffer->bin_buffer_size);
+			vfree(buffer->bin_buffer);
+		}
+
+		/* clear the new area */
+		memset(tbuf + buffer->bin_buffer_size, 0,
+			*ppos + count - buffer->bin_buffer_size);
+		buffer->bin_buffer = tbuf;
+		buffer->bin_buffer_size = *ppos + count;
+	}
+
+	len = simple_write_to_buffer(buffer->bin_buffer,
+			buffer->bin_buffer_size, ppos, buf, count);
+	if (len > 0)
+		*ppos += len;
+out:
+	mutex_unlock(&buffer->mutex);
+	return len;
+}
+
+static int check_perm(struct inode * inode, struct file * file, int type)
 {
 	struct config_item *item = configfs_get_config_item(file->f_path.dentry->d_parent);
 	struct configfs_attribute * attr = to_attr(file->f_path.dentry);
+	struct configfs_bin_attribute *bin_attr = NULL;
 	struct configfs_buffer * buffer;
 	struct configfs_item_operations * ops = NULL;
 	int error = 0;
@@ -220,6 +377,9 @@ static int check_perm(struct inode * inode, struct file * file)
 	if (!item || !attr)
 		goto Einval;
 
+	if (type & CONFIGFS_ITEM_BIN_ATTR)
+		bin_attr = to_bin_attr(file->f_path.dentry);
+
 	/* Grab the module reference for this attribute if we have one */
 	if (!try_module_get(attr->ca_owner)) {
 		error = -ENODEV;
@@ -236,9 +396,14 @@ static int check_perm(struct inode * inode, struct file * file)
 	 * and we must have a store method.
 	 */
 	if (file->f_mode & FMODE_WRITE) {
-		if (!(inode->i_mode & S_IWUGO) || !attr->store)
+		if (!(inode->i_mode & S_IWUGO))
 			goto Eaccess;
 
+		if ((type & CONFIGFS_ITEM_ATTR) && !attr->store)
+			goto Eaccess;
+
+		if ((type & CONFIGFS_ITEM_BIN_ATTR) && !bin_attr->write)
+			goto Eaccess;
 	}
 
 	/* File needs read support.
@@ -246,7 +411,13 @@ static int check_perm(struct inode * inode, struct file * file)
 	 * must be a show method for it.
 	 */
 	if (file->f_mode & FMODE_READ) {
-		if (!(inode->i_mode & S_IRUGO) || !attr->show)
+		if (!(inode->i_mode & S_IRUGO))
+			goto Eaccess;
+
+		if ((type & CONFIGFS_ITEM_ATTR) && !attr->show)
+			goto Eaccess;
+
+		if ((type & CONFIGFS_ITEM_BIN_ATTR) && !bin_attr->read)
 			goto Eaccess;
 	}
 
@@ -260,6 +431,8 @@ static int check_perm(struct inode * inode, struct file * file)
 	}
 	mutex_init(&buffer->mutex);
 	buffer->needs_read_fill = 1;
+	buffer->read_in_progress = 0;
+	buffer->write_in_progress = 0;
 	buffer->ops = ops;
 	file->private_data = buffer;
 	goto Done;
@@ -277,12 +450,7 @@ static int check_perm(struct inode * inode, struct file * file)
 	return error;
 }
 
-static int configfs_open_file(struct inode * inode, struct file * filp)
-{
-	return check_perm(inode,filp);
-}
-
-static int configfs_release(struct inode * inode, struct file * filp)
+static int configfs_release(struct inode *inode, struct file *filp)
 {
 	struct config_item * item = to_item(filp->f_path.dentry->d_parent);
 	struct configfs_attribute * attr = to_attr(filp->f_path.dentry);
@@ -303,6 +471,47 @@ static int configfs_release(struct inode * inode, struct file * filp)
 	return 0;
 }
 
+static int configfs_open_file(struct inode *inode, struct file *filp)
+{
+	return check_perm(inode, filp, CONFIGFS_ITEM_ATTR);
+}
+
+static int configfs_open_bin_file(struct inode *inode, struct file *filp)
+{
+	return check_perm(inode, filp, CONFIGFS_ITEM_BIN_ATTR);
+}
+
+static int configfs_release_bin_file(struct inode *inode, struct file *filp)
+{
+	struct configfs_buffer *buffer = filp->private_data;
+	struct dentry *dentry = filp->f_path.dentry;
+	struct config_item *item = to_item(dentry->d_parent);
+	struct configfs_bin_attribute *bin_attr = to_bin_attr(dentry);
+	ssize_t len = 0;
+	int ret;
+
+	buffer->read_in_progress = 0;
+
+	if (buffer->write_in_progress) {
+		buffer->write_in_progress = 0;
+
+		len = bin_attr->write(item, buffer->bin_buffer,
+				buffer->bin_buffer_size);
+
+		/* vfree on NULL is safe */
+		vfree(buffer->bin_buffer);
+		buffer->bin_buffer = NULL;
+		buffer->bin_buffer_size = 0;
+		buffer->needs_read_fill = 1;
+	}
+
+	ret = configfs_release(inode, filp);
+	if (len < 0)
+		return len;
+	return ret;
+}
+
+
 const struct file_operations configfs_file_operations = {
 	.read		= configfs_read_file,
 	.write		= configfs_write_file,
@@ -311,6 +520,14 @@ const struct file_operations configfs_file_operations = {
 	.release	= configfs_release,
 };
 
+const struct file_operations configfs_bin_file_operations = {
+	.read		= configfs_read_bin_file,
+	.write		= configfs_write_bin_file,
+	.llseek		= NULL,		/* bin file is not seekable */
+	.open		= configfs_open_bin_file,
+	.release	= configfs_release_bin_file,
+};
+
 /**
  *	configfs_create_file - create an attribute file for an item.
  *	@item:	item we're creating for.
@@ -332,3 +549,24 @@ int configfs_create_file(struct config_item * item, const struct configfs_attrib
 	return error;
 }
 
+/**
+ *	configfs_create_bin_file - create a binary attribute file for an item.
+ *	@item:	item we're creating for.
+ *	@attr:	atrribute descriptor.
+ */
+
+int configfs_create_bin_file(struct config_item *item,
+		const struct configfs_bin_attribute *bin_attr)
+{
+	struct dentry *dir = item->ci_dentry;
+	struct configfs_dirent *parent_sd = dir->d_fsdata;
+	umode_t mode = (bin_attr->cb_attr.ca_mode & S_IALLUGO) | S_IFREG;
+	int error = 0;
+
+	mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_NORMAL);
+	error = configfs_make_dirent(parent_sd, NULL, (void *) bin_attr, mode,
+				     CONFIGFS_ITEM_BIN_ATTR);
+	mutex_unlock(&dir->d_inode->i_mutex);
+
+	return error;
+}
diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
index eae8757..0cc810e 100644
--- a/fs/configfs/inode.c
+++ b/fs/configfs/inode.c
@@ -218,7 +218,7 @@ const unsigned char * configfs_get_name(struct configfs_dirent *sd)
 	if (sd->s_type & (CONFIGFS_DIR | CONFIGFS_ITEM_LINK))
 		return sd->s_dentry->d_name.name;
 
-	if (sd->s_type & CONFIGFS_ITEM_ATTR) {
+	if (sd->s_type & (CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)) {
 		attr = sd->s_element;
 		return attr->ca_name;
 	}
diff --git a/include/linux/configfs.h b/include/linux/configfs.h
index a8a335b..7a529fa 100644
--- a/include/linux/configfs.h
+++ b/include/linux/configfs.h
@@ -51,6 +51,7 @@ struct module;
 struct configfs_item_operations;
 struct configfs_group_operations;
 struct configfs_attribute;
+struct configfs_bin_attribute;
 struct configfs_subsystem;
 
 struct config_item {
@@ -84,6 +85,7 @@ struct config_item_type {
 	struct configfs_item_operations		*ct_item_ops;
 	struct configfs_group_operations	*ct_group_ops;
 	struct configfs_attribute		**ct_attrs;
+	struct configfs_bin_attribute		**ct_bin_attrs;
 };
 
 /**
@@ -154,6 +156,54 @@ static struct configfs_attribute _pfx##attr_##_name = {	\
 	.store		= _pfx##_name##_store,		\
 }
 
+struct file;
+struct vm_area_struct;
+
+struct configfs_bin_attribute {
+	struct configfs_attribute cb_attr;	/* std. attribute */
+	void *cb_private;			/* for user       */
+	size_t cb_max_size;			/* max core size  */
+	ssize_t (*read)(struct config_item *, void *, size_t);
+	ssize_t (*write)(struct config_item *, const void *, size_t);
+};
+
+#define CONFIGFS_BIN_ATTR(_pfx, _name, _priv, _maxsz)		\
+static struct configfs_bin_attribute _pfx##attr_##_name = {	\
+	.cb_attr = {						\
+		.ca_name	= __stringify(_name),		\
+		.ca_mode	= S_IRUGO | S_IWUSR,		\
+		.ca_owner	= THIS_MODULE,			\
+	},							\
+	.cb_private	= _priv,				\
+	.cb_max_size	= _maxsz,				\
+	.read		= _pfx##_name##_read,			\
+	.write		= _pfx##_name##_write,			\
+}
+
+#define CONFIGFS_BIN_ATTR_RO(_pfx, _name, _priv, _maxsz)	\
+static struct configfs_attribute _pfx##attr_##_name = {		\
+	.cb_attr = {						\
+		.ca_name	= __stringify(_name),		\
+		.ca_mode	= S_IRUGO,			\
+		.ca_owner	= THIS_MODULE,			\
+	},							\
+	.cb_private	= _priv,				\
+	.cb_max_size	= _maxsz,				\
+	.read		= _pfx##_name##_read,			\
+}
+
+#define CONFIGFS_BIN_ATTR_WO(_pfx, _name, _priv, _maxsz)	\
+static struct configfs_attribute _pfx##attr_##_name = {		\
+	.cb_attr = {						\
+		.ca_name	= __stringify(_name),		\
+		.ca_mode	= S_IWUSR,			\
+		.ca_owner	= THIS_MODULE,			\
+	},							\
+	.cb_private	= _priv,				\
+	.cb_max_size	= _maxsz,				\
+	.write		= _pfx##_name##_write,			\
+}
+
 /*
  * If allow_link() exists, the item can symlink(2) out to other
  * items.  If the item is a group, it may support mkdir(2).
-- 
1.7.12

Re: [PATCH] configfs: Implement binary attributes

[Christoph Hellwig]

Hi Pantelis,

what's the state of this patch?  I'll need it for some work I plan to
merge for 4.5 as well, so getting it into a stable repository we could
both base off would be nice.

Talking about that, I'd be happy to volunteer maintaining a configfs
tree as the current split over -mm and the target tree seem a bit
painful now that development activity for configfs has picked up.

Re: [PATCH] configfs: Implement binary attributes

[Christoph Hellwig]

Joel, Andrew,

On Tue, Dec 01, 2015 at 10:21:24AM -0800, Christoph Hellwig wrote:
> what's the state of this patch?  I'll need it for some work I plan to
> merge for 4.5 as well, so getting it into a stable repository we could
> both base off would be nice.
> 
> Talking about that, I'd be happy to volunteer maintaining a configfs
> tree as the current split over -mm and the target tree seem a bit
> painful now that development activity for configfs has picked up.

Any progress on having a stable non-reabse branch for this?  My offer to
just do it myself still stands, btw!

Re: [PATCH] configfs: Implement binary attributes

[Pantelis Antoniou]

Hi Christoph,

> On Dec 18, 2015, at 13:20 , Christoph Hellwig <hch@infradead.org> wrote:
> 
> Joel, Andrew,
> 
> On Tue, Dec 01, 2015 at 10:21:24AM -0800, Christoph Hellwig wrote:
>> what's the state of this patch?  I'll need it for some work I plan to
>> merge for 4.5 as well, so getting it into a stable repository we could
>> both base off would be nice.
>> 
>> Talking about that, I'd be happy to volunteer maintaining a configfs
>> tree as the current split over -mm and the target tree seem a bit
>> painful now that development activity for configfs has picked up.
> 
> Any progress on having a stable non-reabse branch for this?  My offer to
> just do it myself still stands, btw!

It’s on my TODO list for the holiday break.

It is not dropped, just /me swamped.

Regards

— Pantelis

Re: [PATCH] configfs: Implement binary attributes

[Christoph Hellwig]

On Fri, Dec 18, 2015 at 01:21:39PM +0200, Pantelis Antoniou wrote:
> It???s on my TODO list for the holiday break.
> 
> It is not dropped, just /me swamped.

I'm happy to take over the patch, but the real important part is
to have in a shared repository where you and I can use it.  For that
we'll need a real configfs git tree.

Re: [PATCH] configfs: Implement binary attributes

[Pantelis Antoniou]

Hi Christoph,

> On Dec 18, 2015, at 13:26 , Christoph Hellwig <hch@infradead.org> wrote:
> 
> On Fri, Dec 18, 2015 at 01:21:39PM +0200, Pantelis Antoniou wrote:
>> It???s on my TODO list for the holiday break.
>> 
>> It is not dropped, just /me swamped.
> 
> I'm happy to take over the patch, but the real important part is
> to have in a shared repository where you and I can use it.  For that
> we'll need a real configfs git tree.

You’ll get it.

— Pantelis

Re: [PATCH] configfs: Implement binary attributes

[Geert Uytterhoeven]

Hi  Christoph,

On Fri, Dec 18, 2015 at 12:27 PM, Pantelis Antoniou
<pantelis.antoniou@konsulko.com> wrote:
>> On Dec 18, 2015, at 13:26 , Christoph Hellwig <hch@infradead.org> wrote:
>>
>> On Fri, Dec 18, 2015 at 01:21:39PM +0200, Pantelis Antoniou wrote:
>>> It???s on my TODO list for the holiday break.
>>>
>>> It is not dropped, just /me swamped.
>>
>> I'm happy to take over the patch, but the real important part is
>> to have in a shared repository where you and I can use it.  For that
>> we'll need a real configfs git tree.
>
> You’ll get it.

I have two fixes in the topic/overlays branch of renesas-drivers.git:

configfs: Fix reading empty binary attributes
of: configfs: Use %zu to format size_t

https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/overlays&id=c90f9ac60eefa6f888fe38553ceff1777deacdf2
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/overlays&id=05d5dc4a335d209b4b7338e4afaaf97d496b7a98

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] configfs: Implement binary attributes

[Pantelis Antoniou]

Hi Geert,

> On Dec 18, 2015, at 16:31 , Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> 
> Hi  Christoph,
> 
> On Fri, Dec 18, 2015 at 12:27 PM, Pantelis Antoniou
> <pantelis.antoniou@konsulko.com> wrote:
>>> On Dec 18, 2015, at 13:26 , Christoph Hellwig <hch@infradead.org> wrote:
>>> 
>>> On Fri, Dec 18, 2015 at 01:21:39PM +0200, Pantelis Antoniou wrote:
>>>> It???s on my TODO list for the holiday break.
>>>> 
>>>> It is not dropped, just /me swamped.
>>> 
>>> I'm happy to take over the patch, but the real important part is
>>> to have in a shared repository where you and I can use it.  For that
>>> we'll need a real configfs git tree.
>> 
>> You’ll get it.
> 
> I have two fixes in the topic/overlays branch of renesas-drivers.git:
> 
> configfs: Fix reading empty binary attributes
> of: configfs: Use %zu to format size_t
> 
> https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/overlays&id=c90f9ac60eefa6f888fe38553ceff1777deacdf2
> https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/overlays&id=05d5dc4a335d209b4b7338e4afaaf97d496b7a98
> 

Duly noted.

> Gr{oetje,eeting}s,
> 
>                        Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                -- Linus Torvalds

Regards

— Pantelis

Re: [PATCH] configfs: Implement binary attributes

[Christoph Hellwig]

On Fri, Dec 18, 2015 at 03:31:42PM +0100, Geert Uytterhoeven wrote:
> I have two fixes in the topic/overlays branch of renesas-drivers.git:

Does thus mean you already have the patch queued up for Linus?

Re: [PATCH] configfs: Implement binary attributes

[Geert Uytterhoeven]

Hi Christoph,

On Tue, Dec 22, 2015 at 4:53 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Fri, Dec 18, 2015 at 03:31:42PM +0100, Geert Uytterhoeven wrote:
>> I have two fixes in the topic/overlays branch of renesas-drivers.git:
>
> Does thus mean you already have the patch queued up for Linus?

No, this is just my local branch containing overlay support, published for
other people who want to use DT overlays.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] configfs: implement binary attributes

[Pantelis Antoniou]

Hi Joel,

> On Dec 30, 2015, at 01:00 , Joel Becker <jlbec@evilplan.org> wrote:
> 
> On Thu, Dec 24, 2015 at 03:51:10PM +0100, Christoph Hellwig wrote:
>> From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>> 
>> ConfigFS lacked binary attributes up until now. This patch
>> introduces support for binary attributes in a somewhat similar
>> manner of sysfs binary attributes albeit with changes that
>> fit the configfs usage model.
>> 
>> Problems that configfs binary attributes fix are everything that
>> requires a binary blob as part of the configuration of a resource,
>> such as bitstream loading for FPGAs, DTBs for dynamically created
>> devices etc.
> 
> Overall, I really like this addition.
> 

That’s good :)

>> @@ -423,7 +429,9 @@ static int configfs_attach_attr(struct configfs_dirent * sd, struct dentry * den
>> 	spin_unlock(&configfs_dirent_lock);
>> 
>> 	error = configfs_create(dentry, (attr->ca_mode & S_IALLUGO) | S_IFREG,
>> -				configfs_init_file);
>> +				(sd->s_type & CONFIGFS_ITEM_ATTR) ?
>> +					configfs_init_file :
>> +					configfs_init_bin_file);
> 
> BIN_ATTRs are the more uncommon type, at least for now.  I would think
> this code should check for special cases and fall back to ITEM_ATTR.  So
> reverse it.
> 
> 	(sd->s_type & CONFIGFS_ITEM_BIN_ATTR) ?
>  		configfs_init_bin_file :
> 		configfs_init_file
> 

OK.

>> +static ssize_t
>> +configfs_read_bin_file(struct file *file, char __user *buf,
>> +		       size_t count, loff_t *ppos)
>> +{
>> +	struct configfs_buffer *buffer = file->private_data;
>> +	struct dentry *dentry = file->f_path.dentry;
>> +	struct config_item *item = to_item(dentry->d_parent);
>> +	struct configfs_bin_attribute *bin_attr = to_bin_attr(dentry);
>> +	ssize_t retval = 0;
>> +	ssize_t len = min_t(size_t, count, PAGE_SIZE);
>> +
>> +	mutex_lock(&buffer->mutex);
>> +
>> +	/* we don't support switching read/write modes */
>> +	if (buffer->write_in_progress) {
>> +		retval = -EINVAL;
>> +		goto out;
>> +	}
> 
> These are valid arguments, it's just competing with another operation.
> Wouldn't something like EINPROGRESS or ETXTBSY make more sense and be
> more informative?  The same for configfs_write_bin_file().
> 

Yep, will be done.

> Joel 
> 

Regards

— Pantelis

PS. A big thanks to Christoph for keeping this going. I will shortly post a new version with the
requested changes incorporated.


> -- 
> 
> "Soap and education are not as sudden as a massacre, but they are more
> deadly in the long run."
> 	- Mark Twain
> 
> 
> 			http://www.jlbec.org/
> 			jlbec@evilplan.org

Re: [PATCH] configfs: implement binary attributes

[Krzysztof Opasiak]

W dniu 2015-12-24 o 15:51, Christoph Hellwig pisze:
> From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>
> ConfigFS lacked binary attributes up until now. This patch
> introduces support for binary attributes in a somewhat similar
> manner of sysfs binary attributes albeit with changes that
> fit the configfs usage model.
>
> Problems that configfs binary attributes fix are everything that
> requires a binary blob as part of the configuration of a resource,
> such as bitstream loading for FPGAs, DTBs for dynamically created
> devices etc.
>
> Look at Documentation/filesystems/configfs/configfs.txt for internals
> and howto use them.
>
> This patch is against linux-next as of today that contains
> Christoph's configfs rework.
>
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> [hch: folded a fix from Geert Uytterhoeven <geert+renesas@glider.be>]
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

(...)

>
>   #include <linux/configfs.h>
> @@ -48,6 +49,10 @@ struct configfs_buffer {
>   	struct configfs_item_operations	* ops;
>   	struct mutex		mutex;
>   	int			needs_read_fill;
> +	int			read_in_progress;
> +	int			write_in_progress;

Those 2 should be probably bool instead of int.

> +	char			*bin_buffer;
> +	int			bin_buffer_size;
>   };

(...)

> diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
> index eae8757..0cc810e 100644
> --- a/fs/configfs/inode.c
> +++ b/fs/configfs/inode.c
> @@ -218,7 +218,7 @@ const unsigned char * configfs_get_name(struct configfs_dirent *sd)
>   	if (sd->s_type & (CONFIGFS_DIR | CONFIGFS_ITEM_LINK))
>   		return sd->s_dentry->d_name.name;
>
> -	if (sd->s_type & CONFIGFS_ITEM_ATTR) {
> +	if (sd->s_type & (CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)) {

I would also recomend a static inline helper here instead of adding this 
or as this may simplify the code in a few places.


Best regards,
-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH] configfs: implement binary attributes

[Krzysztof Opasiak]

W dniu 2015-12-30 o 00:00, Joel Becker pisze:
 > On Thu, Dec 24, 2015 at 03:51:10PM +0100, Christoph Hellwig wrote:
 >> From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
 >>
 >> ConfigFS lacked binary attributes up until now. This patch
 >> introduces support for binary attributes in a somewhat similar
 >> manner of sysfs binary attributes albeit with changes that
 >> fit the configfs usage model.
 >>
 >> Problems that configfs binary attributes fix are everything that
 >> requires a binary blob as part of the configuration of a resource,
 >> such as bitstream loading for FPGAs, DTBs for dynamically created
 >> devices etc.
 >
 > Overall, I really like this addition.
 >
 >> @@ -423,7 +429,9 @@ static int configfs_attach_attr(struct 
configfs_dirent * sd, struct dentry * den
 >>   	spin_unlock(&configfs_dirent_lock);
 >>
 >>   	error = configfs_create(dentry, (attr->ca_mode & S_IALLUGO) | 
S_IFREG,
 >> -				configfs_init_file);
 >> +				(sd->s_type & CONFIGFS_ITEM_ATTR) ?
 >> +					configfs_init_file :
 >> +					configfs_init_bin_file);
 >
 > BIN_ATTRs are the more uncommon type, at least for now.  I would think
 > this code should check for special cases and fall back to ITEM_ATTR.  So
 > reverse it.
 >
 > 	(sd->s_type & CONFIGFS_ITEM_BIN_ATTR) ?
 >    		configfs_init_bin_file :
 > 		configfs_init_file
 >

I think that static inline helper can be usefull here to improve readability

 >> +static ssize_t
 >> +configfs_read_bin_file(struct file *file, char __user *buf,
 >> +		       size_t count, loff_t *ppos)
 >> +{
 >> +	struct configfs_buffer *buffer = file->private_data;
 >> +	struct dentry *dentry = file->f_path.dentry;
 >> +	struct config_item *item = to_item(dentry->d_parent);
 >> +	struct configfs_bin_attribute *bin_attr = to_bin_attr(dentry);
 >> +	ssize_t retval = 0;
 >> +	ssize_t len = min_t(size_t, count, PAGE_SIZE);
 >> +
 >> +	mutex_lock(&buffer->mutex);
 >> +
 >> +	/* we don't support switching read/write modes */
 >> +	if (buffer->write_in_progress) {
 >> +		retval = -EINVAL;
 >> +		goto out;
 >> +	}
 >
 > These are valid arguments, it's just competing with another operation.
 > Wouldn't something like EINPROGRESS or ETXTBSY make more sense and be
 > more informative?  The same for configfs_write_bin_file().
 >
 > Joel
 >

-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH] configfs: implement binary attributes

[Joel Becker]

On Thu, Dec 24, 2015 at 03:51:10PM +0100, Christoph Hellwig wrote:
> From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> 
> ConfigFS lacked binary attributes up until now. This patch
> introduces support for binary attributes in a somewhat similar
> manner of sysfs binary attributes albeit with changes that
> fit the configfs usage model.
> 
> Problems that configfs binary attributes fix are everything that
> requires a binary blob as part of the configuration of a resource,
> such as bitstream loading for FPGAs, DTBs for dynamically created
> devices etc.

Overall, I really like this addition.

> @@ -423,7 +429,9 @@ static int configfs_attach_attr(struct configfs_dirent * sd, struct dentry * den
>  	spin_unlock(&configfs_dirent_lock);
>  
>  	error = configfs_create(dentry, (attr->ca_mode & S_IALLUGO) | S_IFREG,
> -				configfs_init_file);
> +				(sd->s_type & CONFIGFS_ITEM_ATTR) ?
> +					configfs_init_file :
> +					configfs_init_bin_file);

BIN_ATTRs are the more uncommon type, at least for now.  I would think
this code should check for special cases and fall back to ITEM_ATTR.  So
reverse it.

	(sd->s_type & CONFIGFS_ITEM_BIN_ATTR) ?
  		configfs_init_bin_file :
		configfs_init_file

> +static ssize_t
> +configfs_read_bin_file(struct file *file, char __user *buf,
> +		       size_t count, loff_t *ppos)
> +{
> +	struct configfs_buffer *buffer = file->private_data;
> +	struct dentry *dentry = file->f_path.dentry;
> +	struct config_item *item = to_item(dentry->d_parent);
> +	struct configfs_bin_attribute *bin_attr = to_bin_attr(dentry);
> +	ssize_t retval = 0;
> +	ssize_t len = min_t(size_t, count, PAGE_SIZE);
> +
> +	mutex_lock(&buffer->mutex);
> +
> +	/* we don't support switching read/write modes */
> +	if (buffer->write_in_progress) {
> +		retval = -EINVAL;
> +		goto out;
> +	}

These are valid arguments, it's just competing with another operation.
Wouldn't something like EINPROGRESS or ETXTBSY make more sense and be
more informative?  The same for configfs_write_bin_file().

Joel 

-- 

"Soap and education are not as sudden as a massacre, but they are more
 deadly in the long run."
	- Mark Twain


			http://www.jlbec.org/
			jlbec@evilplan.org

[PATCH] configfs: implement binary attributes

[Christoph Hellwig]

From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>

ConfigFS lacked binary attributes up until now. This patch
introduces support for binary attributes in a somewhat similar
manner of sysfs binary attributes albeit with changes that
fit the configfs usage model.

Problems that configfs binary attributes fix are everything that
requires a binary blob as part of the configuration of a resource,
such as bitstream loading for FPGAs, DTBs for dynamically created
devices etc.

Look at Documentation/filesystems/configfs/configfs.txt for internals
and howto use them.

This patch is against linux-next as of today that contains
Christoph's configfs rework.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
[hch: folded a fix from Geert Uytterhoeven <geert+renesas@glider.be>]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 Documentation/filesystems/configfs/configfs.txt |  57 +++++-
 fs/configfs/configfs_internal.h                 |  14 +-
 fs/configfs/dir.c                               |  18 +-
 fs/configfs/file.c                              | 255 +++++++++++++++++++++++-
 fs/configfs/inode.c                             |   2 +-
 include/linux/configfs.h                        |  50 +++++
 6 files changed, 374 insertions(+), 22 deletions(-)

diff --git a/Documentation/filesystems/configfs/configfs.txt b/Documentation/filesystems/configfs/configfs.txt
index af68efd..e5fe521 100644
--- a/Documentation/filesystems/configfs/configfs.txt
+++ b/Documentation/filesystems/configfs/configfs.txt
@@ -51,15 +51,27 @@ configfs tree is always there, whether mounted on /config or not.
 An item is created via mkdir(2).  The item's attributes will also
 appear at this time.  readdir(3) can determine what the attributes are,
 read(2) can query their default values, and write(2) can store new
-values.  Like sysfs, attributes should be ASCII text files, preferably
-with only one value per file.  The same efficiency caveats from sysfs
-apply.  Don't mix more than one attribute in one attribute file.
-
-Like sysfs, configfs expects write(2) to store the entire buffer at
-once.  When writing to configfs attributes, userspace processes should
-first read the entire file, modify the portions they wish to change, and
-then write the entire buffer back.  Attribute files have a maximum size
-of one page (PAGE_SIZE, 4096 on i386).
+values.  Don't mix more than one attribute in one attribute file.
+
+There are two types of configfs attributes:
+
+* Normal attributes, which similar to sysfs attributes, are small ASCII text
+files, with a maximum size of one page (PAGE_SIZE, 4096 on i386).  Preferably
+only one value per file should be used, and the same caveats from sysfs apply.
+Configfs expects write(2) to store the entire buffer at once.  When writing to
+normal configfs attributes, userspace processes should first read the entire
+file, modify the portions they wish to change, and then write the entire
+buffer back.
+
+* Binary attributes, which are somewhat similar to sysfs binary attributes,
+but with a few slight changes to semantics.  The PAGE_SIZE limitation does not
+apply, but the whole binary item must fit in single kernel vmalloc'ed buffer.
+The write(2) calls from user space are buffered, and the attributes'
+write_bin_attribute method will be invoked on the final close, therefore it is
+imperative for user-space to check the return code of close(2) in order to
+verify that the operation finished successfully.
+To avoid a malicious user OOMing the kernel, there's a per-binary attribute
+maximum buffer value.
 
 When an item needs to be destroyed, remove it with rmdir(2).  An
 item cannot be destroyed if any other item has a link to it (via
@@ -171,6 +183,7 @@ among other things.  For that, it needs a type.
 		struct configfs_item_operations         *ct_item_ops;
 		struct configfs_group_operations        *ct_group_ops;
 		struct configfs_attribute               **ct_attrs;
+		struct configfs_bin_attribute		**ct_bin_attrs;
 	};
 
 The most basic function of a config_item_type is to define what
@@ -201,6 +214,32 @@ be called whenever userspace asks for a read(2) on the attribute.  If an
 attribute is writable and provides a ->store  method, that method will be
 be called whenever userspace asks for a write(2) on the attribute.
 
+[struct configfs_bin_attribute]
+
+	struct configfs_attribute {
+		struct configfs_attribute	cb_attr;
+		void				*cb_private;
+		size_t				cb_max_size;
+	};
+
+The binary attribute is used when the one needs to use binary blob to
+appear as the contents of a file in the item's configfs directory.
+To do so add the binary attribute to the NULL-terminated array
+config_item_type->ct_bin_attrs, and the item appears in configfs, the
+attribute file will appear with the configfs_bin_attribute->cb_attr.ca_name
+filename.  configfs_bin_attribute->cb_attr.ca_mode specifies the file
+permissions.
+The cb_private member is provided for use by the driver, while the
+cb_max_size member specifies the maximum amount of vmalloc buffer
+to be used.
+
+If binary attribute is readable and the config_item provides a
+ct_item_ops->read_bin_attribute() method, that method will be called
+whenever userspace asks for a read(2) on the attribute.  The converse
+will happen for write(2). The reads/writes are bufferred so only a
+single read/write will occur; the attributes' need not concern itself
+with it.
+
 [struct config_group]
 
 A config_item cannot live in a vacuum.  The only way one can be created
diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
index b65d1ef..ccc31fa 100644
--- a/fs/configfs/configfs_internal.h
+++ b/fs/configfs/configfs_internal.h
@@ -53,13 +53,14 @@ struct configfs_dirent {
 #define CONFIGFS_ROOT		0x0001
 #define CONFIGFS_DIR		0x0002
 #define CONFIGFS_ITEM_ATTR	0x0004
+#define CONFIGFS_ITEM_BIN_ATTR	0x0008
 #define CONFIGFS_ITEM_LINK	0x0020
 #define CONFIGFS_USET_DIR	0x0040
 #define CONFIGFS_USET_DEFAULT	0x0080
 #define CONFIGFS_USET_DROPPING	0x0100
 #define CONFIGFS_USET_IN_MKDIR	0x0200
 #define CONFIGFS_USET_CREATING	0x0400
-#define CONFIGFS_NOT_PINNED	(CONFIGFS_ITEM_ATTR)
+#define CONFIGFS_NOT_PINNED	(CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)
 
 extern struct mutex configfs_symlink_mutex;
 extern spinlock_t configfs_dirent_lock;
@@ -72,6 +73,8 @@ extern struct inode * configfs_new_inode(umode_t mode, struct configfs_dirent *,
 extern int configfs_create(struct dentry *, umode_t mode, void (*init)(struct inode *));
 
 extern int configfs_create_file(struct config_item *, const struct configfs_attribute *);
+extern int configfs_create_bin_file(struct config_item *,
+				    const struct configfs_bin_attribute *);
 extern int configfs_make_dirent(struct configfs_dirent *,
 				struct dentry *, void *, umode_t, int);
 extern int configfs_dirent_is_ready(struct configfs_dirent *);
@@ -88,7 +91,7 @@ extern void configfs_release_fs(void);
 extern struct rw_semaphore configfs_rename_sem;
 extern const struct file_operations configfs_dir_operations;
 extern const struct file_operations configfs_file_operations;
-extern const struct file_operations bin_fops;
+extern const struct file_operations configfs_bin_file_operations;
 extern const struct inode_operations configfs_dir_inode_operations;
 extern const struct inode_operations configfs_root_inode_operations;
 extern const struct inode_operations configfs_symlink_inode_operations;
@@ -119,6 +122,13 @@ static inline struct configfs_attribute * to_attr(struct dentry * dentry)
 	return ((struct configfs_attribute *) sd->s_element);
 }
 
+static inline struct configfs_bin_attribute *to_bin_attr(struct dentry *dentry)
+{
+	struct configfs_attribute *attr = to_attr(dentry);
+
+	return container_of(attr, struct configfs_bin_attribute, cb_attr);
+}
+
 static inline struct config_item *configfs_get_config_item(struct dentry *dentry)
 {
 	struct config_item * item = NULL;
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index a7a1b21..8f6c37a 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -255,6 +255,12 @@ static void configfs_init_file(struct inode * inode)
 	inode->i_fop = &configfs_file_operations;
 }
 
+static void configfs_init_bin_file(struct inode *inode)
+{
+	inode->i_size = 0;
+	inode->i_fop = &configfs_bin_file_operations;
+}
+
 static void init_symlink(struct inode * inode)
 {
 	inode->i_op = &configfs_symlink_inode_operations;
@@ -423,7 +429,9 @@ static int configfs_attach_attr(struct configfs_dirent * sd, struct dentry * den
 	spin_unlock(&configfs_dirent_lock);
 
 	error = configfs_create(dentry, (attr->ca_mode & S_IALLUGO) | S_IFREG,
-				configfs_init_file);
+				(sd->s_type & CONFIGFS_ITEM_ATTR) ?
+					configfs_init_file :
+					configfs_init_bin_file);
 	if (error) {
 		configfs_put(sd);
 		return error;
@@ -583,6 +591,7 @@ static int populate_attrs(struct config_item *item)
 {
 	struct config_item_type *t = item->ci_type;
 	struct configfs_attribute *attr;
+	struct configfs_bin_attribute *bin_attr;
 	int error = 0;
 	int i;
 
@@ -594,6 +603,13 @@ static int populate_attrs(struct config_item *item)
 				break;
 		}
 	}
+	if (t->ct_bin_attrs) {
+		for (i = 0; (bin_attr = t->ct_bin_attrs[i]) != NULL; i++) {
+			error = configfs_create_bin_file(item, bin_attr);
+			if (error)
+				break;
+		}
+	}
 
 	if (error)
 		detach_attrs(item);
diff --git a/fs/configfs/file.c b/fs/configfs/file.c
index d39099e..235f653 100644
--- a/fs/configfs/file.c
+++ b/fs/configfs/file.c
@@ -28,6 +28,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/mutex.h>
+#include <linux/vmalloc.h>
 #include <asm/uaccess.h>
 
 #include <linux/configfs.h>
@@ -48,6 +49,10 @@ struct configfs_buffer {
 	struct configfs_item_operations	* ops;
 	struct mutex		mutex;
 	int			needs_read_fill;
+	int			read_in_progress;
+	int			write_in_progress;
+	char			*bin_buffer;
+	int			bin_buffer_size;
 };
 
 
@@ -123,6 +128,87 @@ out:
 	return retval;
 }
 
+/**
+ *	configfs_read_bin_file - read a binary attribute.
+ *	@file:	file pointer.
+ *	@buf:	buffer to fill.
+ *	@count:	number of bytes to read.
+ *	@ppos:	starting offset in file.
+ *
+ *	Userspace wants to read a binary attribute file. The attribute
+ *	descriptor is in the file's ->d_fsdata. The target item is in the
+ *	directory's ->d_fsdata.
+ *
+ *	We check whether we need to refill the buffer. If so we will
+ *	call the attributes' attr->read() twice. The first time we
+ *	will pass a NULL as a buffer pointer, which the attributes' method
+ *	will use to return the size of the buffer required. If no error
+ *	occurs we will allocate the buffer using vmalloc and call
+ *	attr->read() again passing that buffer as an argument.
+ *	Then we just copy to user-space using simple_read_from_buffer.
+ */
+
+static ssize_t
+configfs_read_bin_file(struct file *file, char __user *buf,
+		       size_t count, loff_t *ppos)
+{
+	struct configfs_buffer *buffer = file->private_data;
+	struct dentry *dentry = file->f_path.dentry;
+	struct config_item *item = to_item(dentry->d_parent);
+	struct configfs_bin_attribute *bin_attr = to_bin_attr(dentry);
+	ssize_t retval = 0;
+	ssize_t len = min_t(size_t, count, PAGE_SIZE);
+
+	mutex_lock(&buffer->mutex);
+
+	/* we don't support switching read/write modes */
+	if (buffer->write_in_progress) {
+		retval = -EINVAL;
+		goto out;
+	}
+	buffer->read_in_progress = 1;
+
+	if (buffer->needs_read_fill) {
+		/* perform first read with buf == NULL to get extent */
+		len = bin_attr->read(item, NULL, 0);
+		if (len <= 0) {
+			retval = len;
+			goto out;
+		}
+
+		/* do not exceed the maximum value */
+		if (bin_attr->cb_max_size && len > bin_attr->cb_max_size) {
+			retval = -EFBIG;
+			goto out;
+		}
+
+		buffer->bin_buffer = vmalloc(len);
+		if (buffer->bin_buffer == NULL) {
+			retval = -ENOMEM;
+			goto out;
+		}
+		buffer->bin_buffer_size = len;
+
+		/* perform second read to fill buffer */
+		len = bin_attr->read(item, buffer->bin_buffer, len);
+		if (len < 0) {
+			retval = len;
+			vfree(buffer->bin_buffer);
+			buffer->bin_buffer_size = 0;
+			buffer->bin_buffer = NULL;
+			goto out;
+		}
+
+		buffer->needs_read_fill = 0;
+	}
+
+	retval = simple_read_from_buffer(buf, count, ppos, buffer->bin_buffer,
+					buffer->bin_buffer_size);
+out:
+	mutex_unlock(&buffer->mutex);
+	return retval;
+}
+
 
 /**
  *	fill_write_buffer - copy buffer from userspace.
@@ -209,10 +295,80 @@ configfs_write_file(struct file *file, const char __user *buf, size_t count, lof
 	return len;
 }
 
-static int check_perm(struct inode * inode, struct file * file)
+/**
+ *	configfs_write_bin_file - write a binary attribute.
+ *	@file:	file pointer
+ *	@buf:	data to write
+ *	@count:	number of bytes
+ *	@ppos:	starting offset
+ *
+ *	Writing to a binary attribute file is similar to a normal read.
+ *	We buffer the consecutive writes (binary attribute files do not
+ *	support lseek) in a continuously growing buffer, but we don't
+ *	commit until the close of the file.
+ */
+
+static ssize_t
+configfs_write_bin_file(struct file *file, const char __user *buf,
+			size_t count, loff_t *ppos)
+{
+	struct configfs_buffer *buffer = file->private_data;
+	struct dentry *dentry = file->f_path.dentry;
+	struct configfs_bin_attribute *bin_attr = to_bin_attr(dentry);
+	void *tbuf = NULL;
+	ssize_t len;
+
+	mutex_lock(&buffer->mutex);
+
+	/* we don't support switching read/write modes */
+	if (buffer->read_in_progress) {
+		len = -EINVAL;
+		goto out;
+	}
+	buffer->write_in_progress = 1;
+
+	/* buffer grows? */
+	if (*ppos + count > buffer->bin_buffer_size) {
+
+		if (bin_attr->cb_max_size &&
+			*ppos + count > bin_attr->cb_max_size) {
+			len = -EFBIG;
+		}
+
+		tbuf = vmalloc(*ppos + count);
+		if (tbuf == NULL) {
+			len = -ENOMEM;
+			goto out;
+		}
+
+		/* copy old contents */
+		if (buffer->bin_buffer) {
+			memcpy(tbuf, buffer->bin_buffer,
+				buffer->bin_buffer_size);
+			vfree(buffer->bin_buffer);
+		}
+
+		/* clear the new area */
+		memset(tbuf + buffer->bin_buffer_size, 0,
+			*ppos + count - buffer->bin_buffer_size);
+		buffer->bin_buffer = tbuf;
+		buffer->bin_buffer_size = *ppos + count;
+	}
+
+	len = simple_write_to_buffer(buffer->bin_buffer,
+			buffer->bin_buffer_size, ppos, buf, count);
+	if (len > 0)
+		*ppos += len;
+out:
+	mutex_unlock(&buffer->mutex);
+	return len;
+}
+
+static int check_perm(struct inode * inode, struct file * file, int type)
 {
 	struct config_item *item = configfs_get_config_item(file->f_path.dentry->d_parent);
 	struct configfs_attribute * attr = to_attr(file->f_path.dentry);
+	struct configfs_bin_attribute *bin_attr = NULL;
 	struct configfs_buffer * buffer;
 	struct configfs_item_operations * ops = NULL;
 	int error = 0;
@@ -220,6 +376,9 @@ static int check_perm(struct inode * inode, struct file * file)
 	if (!item || !attr)
 		goto Einval;
 
+	if (type & CONFIGFS_ITEM_BIN_ATTR)
+		bin_attr = to_bin_attr(file->f_path.dentry);
+
 	/* Grab the module reference for this attribute if we have one */
 	if (!try_module_get(attr->ca_owner)) {
 		error = -ENODEV;
@@ -236,9 +395,14 @@ static int check_perm(struct inode * inode, struct file * file)
 	 * and we must have a store method.
 	 */
 	if (file->f_mode & FMODE_WRITE) {
-		if (!(inode->i_mode & S_IWUGO) || !attr->store)
+		if (!(inode->i_mode & S_IWUGO))
+			goto Eaccess;
+
+		if ((type & CONFIGFS_ITEM_ATTR) && !attr->store)
 			goto Eaccess;
 
+		if ((type & CONFIGFS_ITEM_BIN_ATTR) && !bin_attr->write)
+			goto Eaccess;
 	}
 
 	/* File needs read support.
@@ -246,7 +410,13 @@ static int check_perm(struct inode * inode, struct file * file)
 	 * must be a show method for it.
 	 */
 	if (file->f_mode & FMODE_READ) {
-		if (!(inode->i_mode & S_IRUGO) || !attr->show)
+		if (!(inode->i_mode & S_IRUGO))
+			goto Eaccess;
+
+		if ((type & CONFIGFS_ITEM_ATTR) && !attr->show)
+			goto Eaccess;
+
+		if ((type & CONFIGFS_ITEM_BIN_ATTR) && !bin_attr->read)
 			goto Eaccess;
 	}
 
@@ -260,6 +430,8 @@ static int check_perm(struct inode * inode, struct file * file)
 	}
 	mutex_init(&buffer->mutex);
 	buffer->needs_read_fill = 1;
+	buffer->read_in_progress = 0;
+	buffer->write_in_progress = 0;
 	buffer->ops = ops;
 	file->private_data = buffer;
 	goto Done;
@@ -277,12 +449,7 @@ static int check_perm(struct inode * inode, struct file * file)
 	return error;
 }
 
-static int configfs_open_file(struct inode * inode, struct file * filp)
-{
-	return check_perm(inode,filp);
-}
-
-static int configfs_release(struct inode * inode, struct file * filp)
+static int configfs_release(struct inode *inode, struct file *filp)
 {
 	struct config_item * item = to_item(filp->f_path.dentry->d_parent);
 	struct configfs_attribute * attr = to_attr(filp->f_path.dentry);
@@ -303,6 +470,47 @@ static int configfs_release(struct inode * inode, struct file * filp)
 	return 0;
 }
 
+static int configfs_open_file(struct inode *inode, struct file *filp)
+{
+	return check_perm(inode, filp, CONFIGFS_ITEM_ATTR);
+}
+
+static int configfs_open_bin_file(struct inode *inode, struct file *filp)
+{
+	return check_perm(inode, filp, CONFIGFS_ITEM_BIN_ATTR);
+}
+
+static int configfs_release_bin_file(struct inode *inode, struct file *filp)
+{
+	struct configfs_buffer *buffer = filp->private_data;
+	struct dentry *dentry = filp->f_path.dentry;
+	struct config_item *item = to_item(dentry->d_parent);
+	struct configfs_bin_attribute *bin_attr = to_bin_attr(dentry);
+	ssize_t len = 0;
+	int ret;
+
+	buffer->read_in_progress = 0;
+
+	if (buffer->write_in_progress) {
+		buffer->write_in_progress = 0;
+
+		len = bin_attr->write(item, buffer->bin_buffer,
+				buffer->bin_buffer_size);
+
+		/* vfree on NULL is safe */
+		vfree(buffer->bin_buffer);
+		buffer->bin_buffer = NULL;
+		buffer->bin_buffer_size = 0;
+		buffer->needs_read_fill = 1;
+	}
+
+	ret = configfs_release(inode, filp);
+	if (len < 0)
+		return len;
+	return ret;
+}
+
+
 const struct file_operations configfs_file_operations = {
 	.read		= configfs_read_file,
 	.write		= configfs_write_file,
@@ -311,6 +519,14 @@ const struct file_operations configfs_file_operations = {
 	.release	= configfs_release,
 };
 
+const struct file_operations configfs_bin_file_operations = {
+	.read		= configfs_read_bin_file,
+	.write		= configfs_write_bin_file,
+	.llseek		= NULL,		/* bin file is not seekable */
+	.open		= configfs_open_bin_file,
+	.release	= configfs_release_bin_file,
+};
+
 /**
  *	configfs_create_file - create an attribute file for an item.
  *	@item:	item we're creating for.
@@ -332,3 +548,24 @@ int configfs_create_file(struct config_item * item, const struct configfs_attrib
 	return error;
 }
 
+/**
+ *	configfs_create_bin_file - create a binary attribute file for an item.
+ *	@item:	item we're creating for.
+ *	@attr:	atrribute descriptor.
+ */
+
+int configfs_create_bin_file(struct config_item *item,
+		const struct configfs_bin_attribute *bin_attr)
+{
+	struct dentry *dir = item->ci_dentry;
+	struct configfs_dirent *parent_sd = dir->d_fsdata;
+	umode_t mode = (bin_attr->cb_attr.ca_mode & S_IALLUGO) | S_IFREG;
+	int error = 0;
+
+	mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_NORMAL);
+	error = configfs_make_dirent(parent_sd, NULL, (void *) bin_attr, mode,
+				     CONFIGFS_ITEM_BIN_ATTR);
+	mutex_unlock(&dir->d_inode->i_mutex);
+
+	return error;
+}
diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
index eae8757..0cc810e 100644
--- a/fs/configfs/inode.c
+++ b/fs/configfs/inode.c
@@ -218,7 +218,7 @@ const unsigned char * configfs_get_name(struct configfs_dirent *sd)
 	if (sd->s_type & (CONFIGFS_DIR | CONFIGFS_ITEM_LINK))
 		return sd->s_dentry->d_name.name;
 
-	if (sd->s_type & CONFIGFS_ITEM_ATTR) {
+	if (sd->s_type & (CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)) {
 		attr = sd->s_element;
 		return attr->ca_name;
 	}
diff --git a/include/linux/configfs.h b/include/linux/configfs.h
index 758a029..f7300d0 100644
--- a/include/linux/configfs.h
+++ b/include/linux/configfs.h
@@ -51,6 +51,7 @@ struct module;
 struct configfs_item_operations;
 struct configfs_group_operations;
 struct configfs_attribute;
+struct configfs_bin_attribute;
 struct configfs_subsystem;
 
 struct config_item {
@@ -84,6 +85,7 @@ struct config_item_type {
 	struct configfs_item_operations		*ct_item_ops;
 	struct configfs_group_operations	*ct_group_ops;
 	struct configfs_attribute		**ct_attrs;
+	struct configfs_bin_attribute		**ct_bin_attrs;
 };
 
 /**
@@ -154,6 +156,54 @@ static struct configfs_attribute _pfx##attr_##_name = {	\
 	.store		= _pfx##_name##_store,		\
 }
 
+struct file;
+struct vm_area_struct;
+
+struct configfs_bin_attribute {
+	struct configfs_attribute cb_attr;	/* std. attribute */
+	void *cb_private;			/* for user       */
+	size_t cb_max_size;			/* max core size  */
+	ssize_t (*read)(struct config_item *, void *, size_t);
+	ssize_t (*write)(struct config_item *, const void *, size_t);
+};
+
+#define CONFIGFS_BIN_ATTR(_pfx, _name, _priv, _maxsz)		\
+static struct configfs_bin_attribute _pfx##attr_##_name = {	\
+	.cb_attr = {						\
+		.ca_name	= __stringify(_name),		\
+		.ca_mode	= S_IRUGO | S_IWUSR,		\
+		.ca_owner	= THIS_MODULE,			\
+	},							\
+	.cb_private	= _priv,				\
+	.cb_max_size	= _maxsz,				\
+	.read		= _pfx##_name##_read,			\
+	.write		= _pfx##_name##_write,			\
+}
+
+#define CONFIGFS_BIN_ATTR_RO(_pfx, _name, _priv, _maxsz)	\
+static struct configfs_attribute _pfx##attr_##_name = {		\
+	.cb_attr = {						\
+		.ca_name	= __stringify(_name),		\
+		.ca_mode	= S_IRUGO,			\
+		.ca_owner	= THIS_MODULE,			\
+	},							\
+	.cb_private	= _priv,				\
+	.cb_max_size	= _maxsz,				\
+	.read		= _pfx##_name##_read,			\
+}
+
+#define CONFIGFS_BIN_ATTR_WO(_pfx, _name, _priv, _maxsz)	\
+static struct configfs_attribute _pfx##attr_##_name = {		\
+	.cb_attr = {						\
+		.ca_name	= __stringify(_name),		\
+		.ca_mode	= S_IWUSR,			\
+		.ca_owner	= THIS_MODULE,			\
+	},							\
+	.cb_private	= _priv,				\
+	.cb_max_size	= _maxsz,				\
+	.write		= _pfx##_name##_write,			\
+}
+
 /*
  * If allow_link() exists, the item can symlink(2) out to other
  * items.  If the item is a group, it may support mkdir(2).
-- 
1.9.1

Re: [PATCH] configfs: Implement binary attributes

[Christoph Hellwig]

On Thu, Sep 17, 2015 at 09:29:35AM +0300, Pantelis Antoniou wrote:
> Yes, indeed this is interesting.
> 
> I could rebase on-top of your tree and try to make things simpler.
> Do you mind if you carry the binary attribute patch along when you post
> yours?

Sure, I can do that.

Re: [PATCH] configfs: Implement binary attributes

[Pantelis Antoniou]

Hi Christoph,

> On Sep 17, 2015, at 03:37 , Christoph Hellwig <hch@infradead.org> wrote:
> 
> Hi Pantelis,
> 
> can you check if your patches become simpler on top of the changes
> in this tree:
> 
> http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/configfs.2
> 
> I recently started using configfs and came up with this to avoid
> the gigantic amounts of boilerplate code required for configfs
> attributes at the moment.  I plan to send it out soon.

Yes, indeed this is interesting.

I could rebase on-top of your tree and try to make things simpler.
Do you mind if you carry the binary attribute patch along when you post
yours?

Regards

— Pantelis
 

Re: [PATCH] configfs: Implement binary attributes

[Christoph Hellwig]

Hi Pantelis,

can you check if your patches become simpler on top of the changes
in this tree:

http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/configfs.2

I recently started using configfs and came up with this to avoid
the gigantic amounts of boilerplate code required for configfs
attributes at the moment.  I plan to send it out soon.

[PATCH] configfs: Implement binary attributes

[Pantelis Antoniou]

ConfigFS lacked binary attributes up until now. This patch
introduces support for binary attributes in a somewhat similar
manner of sysfs binary attributes albeit with changes that
fit the configfs usage model.

Problems that configfs binary attributes fix are everything that
requires a binary blob as part of the configuration of a resource,
such as bitstream loading for FPGAs, DTBs for dynamically created
devices etc.

Look at Documentation/filesystems/configfs/configfs.txt for internals
and howto use them.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
---
 Documentation/filesystems/configfs/configfs.txt |  71 +++++-
 fs/configfs/configfs_internal.h                 |  14 +-
 fs/configfs/dir.c                               |  18 +-
 fs/configfs/file.c                              | 326 ++++++++++++++++++++++--
 fs/configfs/inode.c                             |   2 +-
 include/linux/configfs.h                        |  98 +++++++
 6 files changed, 496 insertions(+), 33 deletions(-)

diff --git a/Documentation/filesystems/configfs/configfs.txt b/Documentation/filesystems/configfs/configfs.txt
index b40fec9..eb4204c 100644
--- a/Documentation/filesystems/configfs/configfs.txt
+++ b/Documentation/filesystems/configfs/configfs.txt
@@ -51,15 +51,27 @@ configfs tree is always there, whether mounted on /config or not.
 An item is created via mkdir(2).  The item's attributes will also
 appear at this time.  readdir(3) can determine what the attributes are,
 read(2) can query their default values, and write(2) can store new
-values.  Like sysfs, attributes should be ASCII text files, preferably
-with only one value per file.  The same efficiency caveats from sysfs
-apply.  Don't mix more than one attribute in one attribute file.
-
-Like sysfs, configfs expects write(2) to store the entire buffer at
-once.  When writing to configfs attributes, userspace processes should
-first read the entire file, modify the portions they wish to change, and
-then write the entire buffer back.  Attribute files have a maximum size
-of one page (PAGE_SIZE, 4096 on i386).
+values.  Don't mix more than one attribute in one attribute file.
+
+There are two types of configfs attributes:
+
+* Normal attributes, which similar to sysfs attributes, are small ASCII text
+files, with a maximum size of one page (PAGE_SIZE, 4096 on i386).  Preferably
+only one value per file should be used, and the same caveats from sysfs apply.
+Configfs expects write(2) to store the entire buffer at once.  When writing to
+normal configfs attributes, userspace processes should first read the entire
+file, modify the portions they wish to change, and then write the entire
+buffer back.
+
+* Binary attributes, which are somewhat similar to sysfs binary attributes,
+but with a few slight changes to semantics.  The PAGE_SIZE limitation does not
+apply, but the whole binary item must fit in single kernel vmalloc'ed buffer.
+The write(2) calls from user space are buffered, and the attributes'
+write_bin_attribute method will be invoked on the final close, therefore it is
+imperative for user-space to check the return code of close(2) in order to
+verify that the operation finished successfully.
+To avoid a malicious user OOMing the kernel, there's a per-binary attribute
+maximum buffer value.
 
 When an item needs to be destroyed, remove it with rmdir(2).  An
 item cannot be destroyed if any other item has a link to it (via
@@ -166,6 +178,12 @@ among other things.  For that, it needs a type.
 		ssize_t (*store_attribute)(struct config_item *,
 					   struct configfs_attribute *,
 					   const char *, size_t);
+		ssize_t (*read_bin_attribute)(struct config_item *,
+					      struct configfs_bin_attribute *,
+					      void *, size_t);
+		ssize_t (*write_bin_attribute)(struct config_item *,
+					       struct configfs_bin_attribute *,
+					       const void *, size_t);
 		int (*allow_link)(struct config_item *src,
 				  struct config_item *target);
 		int (*drop_link)(struct config_item *src,
@@ -177,15 +195,18 @@ among other things.  For that, it needs a type.
 		struct configfs_item_operations         *ct_item_ops;
 		struct configfs_group_operations        *ct_group_ops;
 		struct configfs_attribute               **ct_attrs;
+		struct configfs_bin_attribute		**ct_bin_attrs;
 	};
 
 The most basic function of a config_item_type is to define what
 operations can be performed on a config_item.  All items that have been
 allocated dynamically will need to provide the ct_item_ops->release()
 method.  This method is called when the config_item's reference count
-reaches zero.  Items that wish to display an attribute need to provide
-the ct_item_ops->show_attribute() method.  Similarly, storing a new
-attribute value uses the store_attribute() method.
+reaches zero.  Items that wish to display an normal attribute need to provide
+the ct_item_ops->show_attribute() method, while binary attributes provide the
+ct_item_ops->read_bin_attribute(). Similarly, storing a new normal attribute
+value uses the store_attribute() method, while the binarys' attribute equivalent
+is the ct_item_ops->write_bin_attribute() method.
 
 [struct configfs_attribute]
 
@@ -207,6 +228,32 @@ ct_item_ops->show_attribute() method, that method will be called
 whenever userspace asks for a read(2) on the attribute.  The converse
 will happen for write(2).
 
+[struct configfs_bin_attribute]
+
+	struct configfs_attribute {
+		struct configfs_attribute	cb_attr;
+		void				*cb_private;
+		size_t				cb_max_size;
+	};
+
+The binary attribute is used when the one needs to use binary blob to
+appear as the contents of a file in the item's configfs directory.
+To do so add the binary attribute to the NULL-terminated array
+config_item_type->ct_bin_attrs, and the item appears in configfs, the
+attribute file will appear with the configfs_bin_attribute->cb_attr.ca_name
+filename.  configfs_bin_attribute->cb_attr.ca_mode specifies the file
+permissions.
+The cb_private member is provided for use by the driver, while the
+cb_max_size member specifies the maximum amount of vmalloc buffer
+to be used.
+
+If binary attribute is readable and the config_item provides a
+ct_item_ops->read_bin_attribute() method, that method will be called
+whenever userspace asks for a read(2) on the attribute.  The converse
+will happen for write(2). The reads/writes are bufferred so only a
+single read/write will occur; the attributes' need not concern itself
+with it.
+
 [struct config_group]
 
 A config_item cannot live in a vacuum.  The only way one can be created
diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
index b65d1ef..ccc31fa 100644
--- a/fs/configfs/configfs_internal.h
+++ b/fs/configfs/configfs_internal.h
@@ -53,13 +53,14 @@ struct configfs_dirent {
 #define CONFIGFS_ROOT		0x0001
 #define CONFIGFS_DIR		0x0002
 #define CONFIGFS_ITEM_ATTR	0x0004
+#define CONFIGFS_ITEM_BIN_ATTR	0x0008
 #define CONFIGFS_ITEM_LINK	0x0020
 #define CONFIGFS_USET_DIR	0x0040
 #define CONFIGFS_USET_DEFAULT	0x0080
 #define CONFIGFS_USET_DROPPING	0x0100
 #define CONFIGFS_USET_IN_MKDIR	0x0200
 #define CONFIGFS_USET_CREATING	0x0400
-#define CONFIGFS_NOT_PINNED	(CONFIGFS_ITEM_ATTR)
+#define CONFIGFS_NOT_PINNED	(CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)
 
 extern struct mutex configfs_symlink_mutex;
 extern spinlock_t configfs_dirent_lock;
@@ -72,6 +73,8 @@ extern struct inode * configfs_new_inode(umode_t mode, struct configfs_dirent *,
 extern int configfs_create(struct dentry *, umode_t mode, void (*init)(struct inode *));
 
 extern int configfs_create_file(struct config_item *, const struct configfs_attribute *);
+extern int configfs_create_bin_file(struct config_item *,
+				    const struct configfs_bin_attribute *);
 extern int configfs_make_dirent(struct configfs_dirent *,
 				struct dentry *, void *, umode_t, int);
 extern int configfs_dirent_is_ready(struct configfs_dirent *);
@@ -88,7 +91,7 @@ extern void configfs_release_fs(void);
 extern struct rw_semaphore configfs_rename_sem;
 extern const struct file_operations configfs_dir_operations;
 extern const struct file_operations configfs_file_operations;
-extern const struct file_operations bin_fops;
+extern const struct file_operations configfs_bin_file_operations;
 extern const struct inode_operations configfs_dir_inode_operations;
 extern const struct inode_operations configfs_root_inode_operations;
 extern const struct inode_operations configfs_symlink_inode_operations;
@@ -119,6 +122,13 @@ static inline struct configfs_attribute * to_attr(struct dentry * dentry)
 	return ((struct configfs_attribute *) sd->s_element);
 }
 
+static inline struct configfs_bin_attribute *to_bin_attr(struct dentry *dentry)
+{
+	struct configfs_attribute *attr = to_attr(dentry);
+
+	return container_of(attr, struct configfs_bin_attribute, cb_attr);
+}
+
 static inline struct config_item *configfs_get_config_item(struct dentry *dentry)
 {
 	struct config_item * item = NULL;
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index c81ce7f..8951466 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -255,6 +255,12 @@ static void configfs_init_file(struct inode * inode)
 	inode->i_fop = &configfs_file_operations;
 }
 
+static void configfs_init_bin_file(struct inode *inode)
+{
+	inode->i_size = 0;
+	inode->i_fop = &configfs_bin_file_operations;
+}
+
 static void init_symlink(struct inode * inode)
 {
 	inode->i_op = &configfs_symlink_inode_operations;
@@ -423,7 +429,9 @@ static int configfs_attach_attr(struct configfs_dirent * sd, struct dentry * den
 	spin_unlock(&configfs_dirent_lock);
 
 	error = configfs_create(dentry, (attr->ca_mode & S_IALLUGO) | S_IFREG,
-				configfs_init_file);
+				(sd->s_type & CONFIGFS_ITEM_ATTR) ?
+					configfs_init_file :
+					configfs_init_bin_file);
 	if (error) {
 		configfs_put(sd);
 		return error;
@@ -583,6 +591,7 @@ static int populate_attrs(struct config_item *item)
 {
 	struct config_item_type *t = item->ci_type;
 	struct configfs_attribute *attr;
+	struct configfs_bin_attribute *bin_attr;
 	int error = 0;
 	int i;
 
@@ -594,6 +603,13 @@ static int populate_attrs(struct config_item *item)
 				break;
 		}
 	}
+	if (t->ct_bin_attrs) {
+		for (i = 0; (bin_attr = t->ct_bin_attrs[i]) != NULL; i++) {
+			error = configfs_create_bin_file(item, bin_attr);
+			if (error)
+				break;
+		}
+	}
 
 	if (error)
 		detach_attrs(item);
diff --git a/fs/configfs/file.c b/fs/configfs/file.c
index 403269f..af6cb78 100644
--- a/fs/configfs/file.c
+++ b/fs/configfs/file.c
@@ -28,6 +28,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/mutex.h>
+#include <linux/vmalloc.h>
 #include <asm/uaccess.h>
 
 #include <linux/configfs.h>
@@ -48,6 +49,10 @@ struct configfs_buffer {
 	struct configfs_item_operations	* ops;
 	struct mutex		mutex;
 	int			needs_read_fill;
+	int			read_in_progress;
+	int			write_in_progress;
+	char			*bin_buffer;
+	int			bin_buffer_size;
 };
 
 
@@ -107,8 +112,16 @@ static ssize_t
 configfs_read_file(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 {
 	struct configfs_buffer * buffer = file->private_data;
+	struct configfs_dirent *sd;
 	ssize_t retval = 0;
 
+	sd = file->f_path.dentry->d_fsdata;
+	if (WARN_ON(sd == NULL))
+		return -EINVAL;
+
+	if (WARN_ON(!(sd->s_type & CONFIGFS_ITEM_ATTR)))
+		return -EINVAL;
+
 	mutex_lock(&buffer->mutex);
 	if (buffer->needs_read_fill) {
 		if ((retval = fill_read_buffer(file->f_path.dentry,buffer)))
@@ -123,6 +136,97 @@ out:
 	return retval;
 }
 
+/**
+ *	configfs_read_bin_file - read a binary attribute.
+ *	@file:	file pointer.
+ *	@buf:	buffer to fill.
+ *	@count:	number of bytes to read.
+ *	@ppos:	starting offset in file.
+ *
+ *	Userspace wants to read a binary attribute file. The attribute
+ *	descriptor is in the file's ->d_fsdata. The target item is in the
+ *	directory's ->d_fsdata.
+ *
+ *	We check whether we need to refill the buffer. If so we will
+ *	call the attributes' ops->read_bin_attribute() twice. The first time we
+ *	will pass a NULL as a buffer pointer, which the attributes' method
+ *	will use to return the size of the buffer required. If no error
+ *	occurs we will allocate the buffer using vmalloc and call
+ *	ops->read_bin_atribute() again passing that buffer as an argument.
+ *	Then we just copy to user-space using simple_read_from_buffer.
+ */
+
+static ssize_t
+configfs_read_bin_file(struct file *file, char __user *buf,
+		       size_t count, loff_t *ppos)
+{
+	struct configfs_buffer *buffer = file->private_data;
+	struct dentry *dentry = file->f_path.dentry;
+	struct configfs_dirent *sd = dentry->d_fsdata;
+	struct config_item *item = to_item(dentry->d_parent);
+	struct configfs_item_operations *ops = buffer->ops;
+	struct configfs_bin_attribute *bin_attr = to_bin_attr(dentry);
+	ssize_t retval = 0;
+	ssize_t len = min_t(size_t, count, PAGE_SIZE);
+
+	if (WARN_ON(sd == NULL))
+		return -EINVAL;
+
+	if (WARN_ON(!(sd->s_type & CONFIGFS_ITEM_BIN_ATTR)))
+		return -EINVAL;
+
+	mutex_lock(&buffer->mutex);
+
+	/* we don't support switching read/write modes */
+	if (buffer->write_in_progress) {
+		retval = -EINVAL;
+		goto out;
+	}
+	buffer->read_in_progress = 1;
+
+	if (buffer->needs_read_fill) {
+
+		/* perform first read with buf == NULL to get extent */
+		len = ops->read_bin_attribute(item, bin_attr, NULL, 0);
+		if (len < 0) {
+			retval = len;
+			goto out;
+		}
+
+		/* do not exceed the maximum value */
+		if (bin_attr->cb_max_size && len > bin_attr->cb_max_size) {
+			retval = -EFBIG;
+			goto out;
+		}
+
+		buffer->bin_buffer = vmalloc(len);
+		if (buffer->bin_buffer == NULL) {
+			retval = -ENOMEM;
+			goto out;
+		}
+		buffer->bin_buffer_size = len;
+
+		/* perform second read to fill buffer */
+		len = ops->read_bin_attribute(item, bin_attr,
+				buffer->bin_buffer, len);
+		if (len < 0) {
+			retval = len;
+			vfree(buffer->bin_buffer);
+			buffer->bin_buffer_size = 0;
+			buffer->bin_buffer = NULL;
+			goto out;
+		}
+
+		buffer->needs_read_fill = 0;
+	}
+
+	retval = simple_read_from_buffer(buf, count, ppos, buffer->bin_buffer,
+					buffer->bin_buffer_size);
+out:
+	mutex_unlock(&buffer->mutex);
+	return retval;
+}
+
 
 /**
  *	fill_write_buffer - copy buffer from userspace.
@@ -197,9 +301,16 @@ flush_write_buffer(struct dentry * dentry, struct configfs_buffer * buffer, size
 static ssize_t
 configfs_write_file(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 {
-	struct configfs_buffer * buffer = file->private_data;
+	struct configfs_buffer *buffer = file->private_data;
+	struct configfs_dirent *sd = file->f_path.dentry->d_fsdata;
 	ssize_t len;
 
+	if (WARN_ON(sd == NULL))
+		return -EINVAL;
+
+	if (WARN_ON(!(sd->s_type & CONFIGFS_ITEM_ATTR)))
+		return -EINVAL;
+
 	mutex_lock(&buffer->mutex);
 	len = fill_write_buffer(buffer, buf, count);
 	if (len > 0)
@@ -210,10 +321,86 @@ configfs_write_file(struct file *file, const char __user *buf, size_t count, lof
 	return len;
 }
 
-static int check_perm(struct inode * inode, struct file * file)
+/**
+ *	configfs_write_bin_file - write a binary attribute.
+ *	@file:	file pointer
+ *	@buf:	data to write
+ *	@count:	number of bytes
+ *	@ppos:	starting offset
+ *
+ *	Writing to a binary attribute file is similar to a normal read.
+ *	We buffer the consecutive writes (binary attribute files do not
+ *	support lseek) in a continuously growing buffer, but we don't
+ *	commit until the close of the file.
+ */
+
+static ssize_t
+configfs_write_bin_file(struct file *file, const char __user *buf,
+			size_t count, loff_t *ppos)
+{
+	struct configfs_buffer *buffer = file->private_data;
+	struct dentry *dentry = file->f_path.dentry;
+	struct configfs_dirent *sd = dentry->d_fsdata;
+	struct configfs_bin_attribute *bin_attr = to_bin_attr(dentry);
+	void *tbuf = NULL;
+	ssize_t len;
+
+	if (WARN_ON(sd == NULL))
+		return -EINVAL;
+
+	if (WARN_ON(!(sd->s_type & CONFIGFS_ITEM_BIN_ATTR)))
+		return -EINVAL;
+
+	mutex_lock(&buffer->mutex);
+
+	/* we don't support switching read/write modes */
+	if (buffer->read_in_progress) {
+		len = -EINVAL;
+		goto out;
+	}
+	buffer->write_in_progress = 1;
+
+	/* buffer grows? */
+	if (*ppos + count > buffer->bin_buffer_size) {
+
+		if (bin_attr->cb_max_size &&
+			*ppos + count > bin_attr->cb_max_size) {
+			len = -EFBIG;
+		}
+
+		tbuf = vmalloc(*ppos + count);
+		if (tbuf == NULL) {
+			len = -ENOMEM;
+			goto out;
+		}
+
+		/* copy old contents */
+		if (buffer->bin_buffer) {
+			memcpy(tbuf, buffer->bin_buffer,
+				buffer->bin_buffer_size);
+			vfree(buffer->bin_buffer);
+		}
+
+		/* clear the new area */
+		memset(tbuf + buffer->bin_buffer_size, 0,
+			*ppos + count - buffer->bin_buffer_size);
+		buffer->bin_buffer = tbuf;
+		buffer->bin_buffer_size = *ppos + count;
+	}
+
+	len = simple_write_to_buffer(buffer->bin_buffer,
+			buffer->bin_buffer_size, ppos, buf, count);
+	if (len > 0)
+		*ppos += len;
+out:
+	mutex_unlock(&buffer->mutex);
+	return len;
+}
+
+static int check_perm(struct inode *inode, struct file *file,
+		      struct config_item *item, struct configfs_attribute *attr,
+		      int type)
 {
-	struct config_item *item = configfs_get_config_item(file->f_path.dentry->d_parent);
-	struct configfs_attribute * attr = to_attr(file->f_path.dentry);
 	struct configfs_buffer * buffer;
 	struct configfs_item_operations * ops = NULL;
 	int error = 0;
@@ -238,9 +425,16 @@ static int check_perm(struct inode * inode, struct file * file)
 	 */
 	if (file->f_mode & FMODE_WRITE) {
 
-		if (!(inode->i_mode & S_IWUGO) || !ops->store_attribute)
+		if (!(inode->i_mode & S_IWUGO))
 			goto Eaccess;
 
+		if ((type & CONFIGFS_ITEM_ATTR) &&
+				!ops->store_attribute)
+			goto Eaccess;
+
+		if ((type & CONFIGFS_ITEM_BIN_ATTR) &&
+				!ops->write_bin_attribute)
+			goto Eaccess;
 	}
 
 	/* File needs read support.
@@ -248,7 +442,13 @@ static int check_perm(struct inode * inode, struct file * file)
 	 * must be a show method for it.
 	 */
 	if (file->f_mode & FMODE_READ) {
-		if (!(inode->i_mode & S_IRUGO) || !ops->show_attribute)
+		if (!(inode->i_mode & S_IRUGO))
+			goto Eaccess;
+
+		if ((type & CONFIGFS_ITEM_ATTR) && !ops->show_attribute)
+			goto Eaccess;
+
+		if ((type & CONFIGFS_ITEM_BIN_ATTR) && !ops->read_bin_attribute)
 			goto Eaccess;
 	}
 
@@ -262,6 +462,8 @@ static int check_perm(struct inode * inode, struct file * file)
 	}
 	mutex_init(&buffer->mutex);
 	buffer->needs_read_fill = 1;
+	buffer->read_in_progress = 0;
+	buffer->write_in_progress = 0;
 	buffer->ops = ops;
 	file->private_data = buffer;
 	goto Done;
@@ -279,17 +481,12 @@ static int check_perm(struct inode * inode, struct file * file)
 	return error;
 }
 
-static int configfs_open_file(struct inode * inode, struct file * filp)
-{
-	return check_perm(inode,filp);
-}
-
-static int configfs_release(struct inode * inode, struct file * filp)
+static int do_release(struct inode *inode, struct file *filp,
+		      struct config_item *item, struct configfs_attribute *attr,
+		      int type)
 {
-	struct config_item * item = to_item(filp->f_path.dentry->d_parent);
-	struct configfs_attribute * attr = to_attr(filp->f_path.dentry);
-	struct module * owner = attr->ca_owner;
-	struct configfs_buffer * buffer = filp->private_data;
+	struct module *owner = attr->ca_owner;
+	struct configfs_buffer *buffer = filp->private_data;
 
 	if (item)
 		config_item_put(item);
@@ -305,12 +502,86 @@ static int configfs_release(struct inode * inode, struct file * filp)
 	return 0;
 }
 
+static int configfs_open_file(struct inode *inode, struct file *filp)
+{
+	return check_perm(inode, filp,
+			configfs_get_config_item(filp->f_path.dentry->d_parent),
+			to_attr(filp->f_path.dentry), CONFIGFS_ITEM_ATTR);
+}
+
+static int configfs_release_file(struct inode *inode, struct file *filp)
+{
+	return do_release(inode, filp,
+			to_item(filp->f_path.dentry->d_parent),
+			to_attr(filp->f_path.dentry), CONFIGFS_ITEM_ATTR);
+}
+
+static int configfs_open_bin_file(struct inode *inode, struct file *filp)
+{
+	return check_perm(inode, filp,
+			configfs_get_config_item(filp->f_path.dentry->d_parent),
+			to_attr(filp->f_path.dentry), CONFIGFS_ITEM_BIN_ATTR);
+}
+
+/**
+ *	configfs_release_bin_file - write a binary attribute.
+ *	@inode: inode pointer
+ *	@filp:	file pointer
+ *
+ *	Releasing a binary attribute file is similar for a read op
+ *	to the normal attribute file. When writing we call the
+ *	attributes' ops->write_bin_attribute() method to commit the
+ *	changes to the attribute.
+ */
+
+static int configfs_release_bin_file(struct inode *inode, struct file *filp)
+{
+	struct configfs_buffer *buffer = filp->private_data;
+	struct dentry *dentry = filp->f_path.dentry;
+	struct config_item *item = to_item(dentry->d_parent);
+	struct configfs_item_operations *ops = buffer->ops;
+	struct configfs_bin_attribute *bin_attr = to_bin_attr(dentry);
+	ssize_t len = 0;
+	int ret;
+
+	buffer->read_in_progress = 0;
+
+	if (buffer->write_in_progress) {
+		buffer->write_in_progress = 0;
+
+		len = ops->write_bin_attribute(item, bin_attr,
+				buffer->bin_buffer, buffer->bin_buffer_size);
+
+		/* vfree on NULL is safe */
+		vfree(buffer->bin_buffer);
+		buffer->bin_buffer = NULL;
+		buffer->bin_buffer_size = 0;
+		buffer->needs_read_fill = 1;
+	}
+
+	ret = do_release(inode, filp,
+			to_item(filp->f_path.dentry->d_parent),
+			to_attr(filp->f_path.dentry), CONFIGFS_ITEM_BIN_ATTR);
+	if (len < 0)
+		return len;
+	return ret;
+}
+
+
 const struct file_operations configfs_file_operations = {
 	.read		= configfs_read_file,
 	.write		= configfs_write_file,
 	.llseek		= generic_file_llseek,
 	.open		= configfs_open_file,
-	.release	= configfs_release,
+	.release	= configfs_release_file,
+};
+
+const struct file_operations configfs_bin_file_operations = {
+	.read		= configfs_read_bin_file,
+	.write		= configfs_write_bin_file,
+	.llseek		= NULL,		/* bin file is not seekable */
+	.open		= configfs_open_bin_file,
+	.release	= configfs_release_bin_file,
 };
 
 /**
@@ -334,3 +605,24 @@ int configfs_create_file(struct config_item * item, const struct configfs_attrib
 	return error;
 }
 
+/**
+ *	configfs_create_bin_file - create a binary attribute file for an item.
+ *	@item:	item we're creating for.
+ *	@attr:	atrribute descriptor.
+ */
+
+int configfs_create_bin_file(struct config_item *item,
+		const struct configfs_bin_attribute *bin_attr)
+{
+	struct dentry *dir = item->ci_dentry;
+	struct configfs_dirent *parent_sd = dir->d_fsdata;
+	umode_t mode = (bin_attr->cb_attr.ca_mode & S_IALLUGO) | S_IFREG;
+	int error = 0;
+
+	mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_NORMAL);
+	error = configfs_make_dirent(parent_sd, NULL, (void *) bin_attr, mode,
+				     CONFIGFS_ITEM_BIN_ATTR);
+	mutex_unlock(&dir->d_inode->i_mutex);
+
+	return error;
+}
diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
index eae8757..0cc810e 100644
--- a/fs/configfs/inode.c
+++ b/fs/configfs/inode.c
@@ -218,7 +218,7 @@ const unsigned char * configfs_get_name(struct configfs_dirent *sd)
 	if (sd->s_type & (CONFIGFS_DIR | CONFIGFS_ITEM_LINK))
 		return sd->s_dentry->d_name.name;
 
-	if (sd->s_type & CONFIGFS_ITEM_ATTR) {
+	if (sd->s_type & (CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)) {
 		attr = sd->s_element;
 		return attr->ca_name;
 	}
diff --git a/include/linux/configfs.h b/include/linux/configfs.h
index 63a36e8..54a6720 100644
--- a/include/linux/configfs.h
+++ b/include/linux/configfs.h
@@ -51,6 +51,7 @@ struct module;
 struct configfs_item_operations;
 struct configfs_group_operations;
 struct configfs_attribute;
+struct configfs_bin_attribute;
 struct configfs_subsystem;
 
 struct config_item {
@@ -84,6 +85,7 @@ struct config_item_type {
 	struct configfs_item_operations		*ct_item_ops;
 	struct configfs_group_operations	*ct_group_ops;
 	struct configfs_attribute		**ct_attrs;
+	struct configfs_bin_attribute		**ct_bin_attrs;
 };
 
 /**
@@ -207,6 +209,96 @@ static ssize_t _item##_attr_store(struct config_item *item,		\
 	return ret;							\
 }
 
+struct file;
+struct vm_area_struct;
+
+struct configfs_bin_attribute {
+	struct configfs_attribute	cb_attr;	/* std. attribute */
+	void				*cb_private;	/* for user       */
+	size_t				cb_max_size;	/* max core size  */
+};
+
+/*
+ * Similar to CONFIGFS_ATTR_STRUCT
+ */
+#define CONFIGFS_BIN_ATTR_STRUCT(_item)					\
+struct _item##_bin_attribute {						\
+	struct configfs_bin_attribute bin_attr;				\
+	ssize_t (*read)(struct _item *, void *, size_t);		\
+	ssize_t (*write)(struct _item *, const void *, size_t);		\
+}
+
+/* macros to create static binary attributes easier */
+#define __CONFIGFS_BIN_ATTR(_name, _mode, _read, _write, _priv, _max)	\
+{									\
+	.bin_attr = {							\
+		.cb_attr	= {					\
+				.ca_name = __stringify(_name),		\
+				.ca_mode = _mode,			\
+				.ca_owner = THIS_MODULE,		\
+		},							\
+		.cb_private	= _priv,				\
+		.cb_max_size	= _max,					\
+	},								\
+	.read	= _read,						\
+	.write	= _write,						\
+}
+
+#define __CONFIGFS_BIN_ATTR_RO(_name, _priv, _max) {			\
+	.bin_attr = {							\
+		.cb_attr	= {					\
+			.ca_name = __stringify(_name),			\
+			.ca_mode = _mode,				\
+			.ca_owner = THIS_MODULE,			\
+		},							\
+		.cb_private	= _priv,				\
+		.cb_max_size	= _max,					\
+	},								\
+	.read	= _read,						\
+}
+
+#define __CONFIGFS_BIN_ATTR_RW(_name)					\
+	__CONFIGFS_BIN_ATTR(_name, (S_IWUSR | S_IRUGO),			\
+		_name##_read, _name##_write)
+
+#define CONFIGFS_BIN_ATTR(_name, _mode, _read, _write)			\
+struct configfs_bin_attribute bin_attr_##_name =			\
+	__CONFIGFS_BIN_ATTR(_name, _mode, _read, _write)
+
+#define CONFIGFS_BIN_ATTR_RO(_name)					\
+struct configfs_bin_attribute bin_attr_##_name =			\
+	__CONFIGFS_BIN_ATTR_RO(_name)
+
+#define CONFIGFS_BIN_ATTR_OPS(_item)					\
+static ssize_t _item##_bin_attr_read(struct config_item *item,		\
+			struct configfs_bin_attribute *bin_attr,	\
+			void *buf, size_t max_count)			\
+{									\
+	struct _item *_item = to_##_item(item);				\
+	struct _item##_bin_attribute *_item##_bin_attr =		\
+		container_of(bin_attr, struct _item##_bin_attribute,	\
+				bin_attr);				\
+	ssize_t ret = 0;						\
+									\
+	if (_item##_bin_attr->read)					\
+		ret = _item##_bin_attr->read(_item, buf, max_count);	\
+	return ret;							\
+}									\
+static ssize_t _item##_bin_attr_write(struct config_item *item,		\
+			struct configfs_bin_attribute *bin_attr,	\
+			const void *buf, size_t count)			\
+{									\
+	struct _item *_item = to_##_item(item);				\
+	struct _item##_bin_attribute *_item##_bin_attr =		\
+		container_of(bin_attr, struct _item##_bin_attribute,	\
+				bin_attr);				\
+	ssize_t ret = -EINVAL;						\
+									\
+	if (_item##_bin_attr->write)					\
+		ret = _item##_bin_attr->write(_item, buf, count);	\
+	return ret;							\
+}
+
 /*
  * If allow_link() exists, the item can symlink(2) out to other
  * items.  If the item is a group, it may support mkdir(2).
@@ -225,6 +317,12 @@ struct configfs_item_operations {
 	void (*release)(struct config_item *);
 	ssize_t	(*show_attribute)(struct config_item *, struct configfs_attribute *,char *);
 	ssize_t	(*store_attribute)(struct config_item *,struct configfs_attribute *,const char *, size_t);
+	ssize_t (*read_bin_attribute)(struct config_item *,
+				      struct configfs_bin_attribute *,
+				      void *, size_t);
+	ssize_t (*write_bin_attribute)(struct config_item *,
+				       struct configfs_bin_attribute *,
+				       const void *, size_t);
 	int (*allow_link)(struct config_item *src, struct config_item *target);
 	int (*drop_link)(struct config_item *src, struct config_item *target);
 };
-- 
1.7.12

Re: [PATCH] configfs: Implement binary attributes

[Pantelis Antoniou]

Hi Joel,

On Jun 25, 2014, at 3:52 PM, Joel Becker wrote:

> On Sun, Jun 22, 2014 at 12:37:42PM +0300, Pantelis Antoniou wrote:
>> ConfigFS lacked binary attributes up until now. This patch
>> introduces support for binary attributes in a somewhat similar
>> manner of sysfs binary attributes albeit with changes that
>> fit the configfs usage model.
>> 
>> Problems that configfs binary attributes fix are everything that
>> requires a binary blob as part of the configuration of a resource,
>> such as bitstream loading for FPGAs, DTBs for dynamically created
>> devices etc.
> 
> I wanted to object on principle, but I actually think I like the way you
> did this.  More comments inline.
> 

Hehe. It sounded weird to me at first, but actually it's not so bad.
It does solve a problem.

>> 
>> Look at Documentation/filesystems/configfs/configfs.txt for internals
>> and howto use them.
>> 
>> This patch generates a bunch of checkpatch warnings, but this stems from
>> following the formatting of the configfs code, so please ignore.
> 
> I thought someone had looked into cleaning up that cut-n-paste legacy.
> I'm fine with new code following sanity..
> 

OK.

>> @@ -431,7 +438,9 @@ static int configfs_attach_attr(struct configfs_dirent * sd, struct dentry * den
>> 	spin_unlock(&configfs_dirent_lock);
>> 
>> 	error = configfs_create(dentry, (attr->ca_mode & S_IALLUGO) | S_IFREG,
>> -				configfs_init_file);
>> +				(sd->s_type & CONFIGFS_ITEM_ATTR) ?
>> +					configfs_init_file :
>> +					configfs_init_bin_file);
>> 	if (error) {
>> 		configfs_put(sd);
>> 		return error;
>> @@ -591,6 +600,7 @@ static int populate_attrs(struct config_item *item)
>> {
>> 	struct config_item_type *t = item->ci_type;
>> 	struct configfs_attribute *attr;
>> +	struct configfs_bin_attribute *bin_attr;
>> 	int error = 0;
>> 	int i;
>> 
>> @@ -603,6 +613,13 @@ static int populate_attrs(struct config_item *item)
>> 		}
>> 	}
>> 
> 
> No need for this blank line.
> 

OK

>> +	if (t->ct_bin_attrs) {
>> +		for (i = 0; (bin_attr = t->ct_bin_attrs[i]) != NULL; i++) {
>> +			if ((error = configfs_create_bin_file(item, bin_attr)))
>> +				break;
>> +		}
>> +	}
>> +
>> 	if (error)
>> 		detach_attrs(item);
>> 
>> diff --git a/fs/configfs/file.c b/fs/configfs/file.c
>> index 1d1c41f..aec051b 100644
>> --- a/fs/configfs/file.c
>> +++ b/fs/configfs/file.c
>> @@ -48,6 +48,10 @@ struct configfs_buffer {
>> 	struct configfs_item_operations	* ops;
>> 	struct mutex		mutex;
>> 	int			needs_read_fill;
>> +	int			read_in_progress;
>> +	int			write_in_progress;
>> +	char			*bin_buffer;
>> +	int			bin_buffer_size;
>> };
>> 
>> 
>> @@ -107,8 +111,15 @@ static ssize_t
>> configfs_read_file(struct file *file, char __user *buf, size_t count, loff_t *ppos)
>> {
>> 	struct configfs_buffer * buffer = file->private_data;
>> +	struct configfs_dirent * sd = file->f_path.dentry->d_fsdata;
>> 	ssize_t retval = 0;
>> 
>> +	if (WARN_ON(sd == NULL))
>> +		return -EINVAL;
>> +
>> +	if (WARN_ON(!(sd->s_type & CONFIGFS_ITEM_ATTR)))
>> +		return -EINVAL;
>> +
>> 	mutex_lock(&buffer->mutex);
>> 	if (buffer->needs_read_fill) {
>> 		if ((retval = fill_read_buffer(file->f_path.dentry,buffer)))
>> @@ -123,6 +134,93 @@ out:
>> 	return retval;
>> }
>> 
>> +/**
>> + *	configfs_read_bin_file - read a binary attribute.
>> + *	@file:	file pointer.
>> + *	@buf:	buffer to fill.
>> + *	@count:	number of bytes to read.
>> + *	@ppos:	starting offset in file.
>> + *
>> + *	Userspace wants to read a binary attribute file. The attribute descriptor
>> + *	is in the file's ->d_fsdata. The target item is in the directory's
>> + *	->d_fsdata.
>> + *
>> + *	We check whether we need to refill the buffer. If so we will
>> + *	call the attributes' ops->read_bin_attribute() twice. The first time we
>> + *	will pass a NULL as a buffer pointer, which the attributes' method
>> + *	will use to return the size of the buffer required. If no error
>> + *	occurs we will allocate the buffer using kmalloc and call 
>> + *	ops->read_bin_atribute() again passing that buffer as an argument.
>> + *	Then we just copy to user-space using simple_read_from_buffer.
>> + */
>> +
>> +static ssize_t
>> +configfs_read_bin_file(struct file *file, char __user *buf, size_t count, loff_t *ppos)
>> +{
>> +	struct configfs_buffer * buffer = file->private_data;
>> +	struct dentry *dentry = file->f_path.dentry;
>> +	struct configfs_dirent * sd = dentry->d_fsdata;
>> +	struct config_item * item = to_item(dentry->d_parent);
>> +	struct configfs_item_operations * ops = buffer->ops;
>> +	struct configfs_attribute * attr = to_attr(dentry);
>> +	struct configfs_bin_attribute *bin_attr =
>> +		container_of(attr, struct configfs_bin_attribute, attr);
> 
> You defined to_bin_attr().  Use it here and in write_bin_file() rather
> than open-coding the container_of() calls.
> 

OK

>> +	ssize_t retval = 0;
>> +	ssize_t len = min_t(size_t, count, PAGE_SIZE);
>> +
>> +	if (WARN_ON(sd == NULL))
>> +		return -EINVAL;
>> +
>> +	if (WARN_ON(!(sd->s_type & CONFIGFS_ITEM_BIN_ATTR)))
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&buffer->mutex);
>> +
>> +	/* we don't support switching read/write modes */
>> +	if (buffer->write_in_progress) {
>> +		retval = -EINVAL;
>> +		goto out;
>> +	}
>> +	if (!buffer->read_in_progress)
>> +		buffer->read_in_progress = 1;
> 
> You can always set read_in_progress, even if the read has already
> started.

OK

> 
>> +
>> +	if (buffer->needs_read_fill) {
>> +
>> +		/* perform first read with buf == NULL to get extent */
>> +		len = ops->read_bin_attribute(item, bin_attr, NULL, 0);
>> +		if (len < 0) {
>> +			retval = len;
>> +			goto out;
>> +		}
>> +
>> +		buffer->bin_buffer = kmalloc(len, GFP_KERNEL);
>> +		if (buffer->bin_buffer == NULL) {
>> +			retval = -ENOMEM;
>> +			goto out;
>> +		}
>> +		buffer->bin_buffer_size = len;
>> +
>> +		/* perform second read to fill buffer */
>> +		len = ops->read_bin_attribute(item, bin_attr,
>> +				buffer->bin_buffer, len);
>> +		if (len < 0) {
>> +			retval = len;
>> +			kfree(buffer->bin_buffer);
>> +			buffer->bin_buffer_size = 0;
>> +			buffer->bin_buffer = NULL;
>> +			goto out;
>> +		}
>> +
>> +		buffer->needs_read_fill = 0;
>> +	}
>> +
>> +	retval = simple_read_from_buffer(buf, count, ppos, buffer->bin_buffer,
>> +					buffer->bin_buffer_size);
>> +out:
>> +	mutex_unlock(&buffer->mutex);
>> +	return retval;
>> +}
>> +
>> 
>> /**
>>  *	fill_write_buffer - copy buffer from userspace.
>> @@ -198,8 +296,15 @@ static ssize_t
>> configfs_write_file(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
>> {
>> 	struct configfs_buffer * buffer = file->private_data;
>> +	struct configfs_dirent * sd = file->f_path.dentry->d_fsdata;
>> 	ssize_t len;
>> 
>> +	if (WARN_ON(sd == NULL))
>> +		return -EINVAL;
>> +
>> +	if (WARN_ON(!(sd->s_type & CONFIGFS_ITEM_ATTR)))
>> +		return -EINVAL;
>> +
>> 	mutex_lock(&buffer->mutex);
>> 	len = fill_write_buffer(buffer, buf, count);
>> 	if (len > 0)
>> @@ -210,10 +315,80 @@ configfs_write_file(struct file *file, const char __user *buf, size_t count, lof
>> 	return len;
>> }
>> 
>> -static int check_perm(struct inode * inode, struct file * file)
>> +/**
>> + *	configfs_write_bin_file - write a binary attribute.
>> + *	@file:	file pointer
>> + *	@buf:	data to write
>> + *	@count:	number of bytes
>> + *	@ppos:	starting offset
>> + *
>> + *	Writting to a binary attribute file is similar to a normal read.
>> + *	We buffer the consecutive writes (binary attribute files do not
>> + *	support lseek) in a continuously growing buffer, but we don't 
>> + *	commit until the close of the file.
>> + */
>> +
>> +static ssize_t
>> +configfs_write_bin_file(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
>> +{
>> +	struct configfs_buffer * buffer = file->private_data;
>> +	struct dentry *dentry = file->f_path.dentry;
>> +	struct configfs_dirent * sd = dentry->d_fsdata;
>> +	void *tbuf = NULL;
>> +	ssize_t len;
>> +
>> +	if (WARN_ON(sd == NULL))
>> +		return -EINVAL;
>> +
>> +	if (WARN_ON(!(sd->s_type & CONFIGFS_ITEM_BIN_ATTR)))
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&buffer->mutex);
>> +
>> +	/* we don't support switching read/write modes */
>> +	if (buffer->read_in_progress) {
>> +		len = -EINVAL;
>> +		goto out;
>> +	}
>> +	if (!buffer->write_in_progress)
>> +		buffer->write_in_progress = 1;
>> +
>> +	/* buffer grows? */
>> +	if (*ppos + count > buffer->bin_buffer_size) {
>> +
>> +		tbuf = kmalloc(*ppos + count, GFP_KERNEL);
>> +		if (tbuf == NULL) {
>> +			len = -ENOMEM;
>> +			goto out;
>> +		}
>> +
>> +		/* copy old contents */
>> +		if (buffer->bin_buffer) {
>> +			memcpy(tbuf, buffer->bin_buffer,
>> +					buffer->bin_buffer_size);
> 
> Fix the argument alignment.  Do this elsewhere in your patch, too.
> 

Hmm, I see.

>> +			kfree(buffer->bin_buffer);
>> +		}
>> +
>> +		/* clear the new area */
>> +		memset(tbuf + buffer->bin_buffer_size, 0,
>> +				*ppos + count - buffer->bin_buffer_size);
>> +		buffer->bin_buffer = tbuf;
>> +		buffer->bin_buffer_size = *ppos + count;
>> +	}
>> +
>> +	len = simple_write_to_buffer(buffer->bin_buffer, buffer->bin_buffer_size,
>> +			ppos, buf, count);
>> +	if (len > 0)
>> +		*ppos += len;
>> +out:
>> +	mutex_unlock(&buffer->mutex);
>> +	return len;
>> +}
>> +
> 
> <snip>
> 
>> diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
>> index 5946ad9..dd9c24b 100644
>> --- a/fs/configfs/inode.c
>> +++ b/fs/configfs/inode.c
>> @@ -231,7 +231,7 @@ const unsigned char * configfs_get_name(struct configfs_dirent *sd)
>> 	if (sd->s_type & (CONFIGFS_DIR | CONFIGFS_ITEM_LINK))
>> 		return sd->s_dentry->d_name.name;
>> 
>> -	if (sd->s_type & CONFIGFS_ITEM_ATTR) {
>> +	if (sd->s_type & (CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)) {
>> 		attr = sd->s_element;
>> 		return attr->ca_name;
>> 	}
>> diff --git a/include/linux/configfs.h b/include/linux/configfs.h
>> index 34025df..15f1079 100644
>> --- a/include/linux/configfs.h
>> +++ b/include/linux/configfs.h
> 
> <snip>
> 
>> @@ -207,6 +209,89 @@ static ssize_t _item##_attr_store(struct config_item *item,		\
>> 	return ret;							\
>> }
>> 
>> +struct file;
>> +struct vm_area_struct;
>> +
>> +struct configfs_bin_attribute {
>> +	struct configfs_attribute	attr;
>> +	void				*private;
>> +};
> 
> cb_attr and cb_private or similar prefixes, please.
> 

OK, will roll everything up in the next submission.

> Joel
> 
> 

Regards

-- Pantelis

Re: [PATCH] configfs: Implement binary attributes

[Joel Becker]

On Sun, Jun 22, 2014 at 12:37:42PM +0300, Pantelis Antoniou wrote:
> ConfigFS lacked binary attributes up until now. This patch
> introduces support for binary attributes in a somewhat similar
> manner of sysfs binary attributes albeit with changes that
> fit the configfs usage model.
> 
> Problems that configfs binary attributes fix are everything that
> requires a binary blob as part of the configuration of a resource,
> such as bitstream loading for FPGAs, DTBs for dynamically created
> devices etc.

I wanted to object on principle, but I actually think I like the way you
did this.  More comments inline.

> 
> Look at Documentation/filesystems/configfs/configfs.txt for internals
> and howto use them.
> 
> This patch generates a bunch of checkpatch warnings, but this stems from
> following the formatting of the configfs code, so please ignore.

I thought someone had looked into cleaning up that cut-n-paste legacy.
I'm fine with new code following sanity..

> @@ -431,7 +438,9 @@ static int configfs_attach_attr(struct configfs_dirent * sd, struct dentry * den
>  	spin_unlock(&configfs_dirent_lock);
>  
>  	error = configfs_create(dentry, (attr->ca_mode & S_IALLUGO) | S_IFREG,
> -				configfs_init_file);
> +				(sd->s_type & CONFIGFS_ITEM_ATTR) ?
> +					configfs_init_file :
> +					configfs_init_bin_file);
>  	if (error) {
>  		configfs_put(sd);
>  		return error;
> @@ -591,6 +600,7 @@ static int populate_attrs(struct config_item *item)
>  {
>  	struct config_item_type *t = item->ci_type;
>  	struct configfs_attribute *attr;
> +	struct configfs_bin_attribute *bin_attr;
>  	int error = 0;
>  	int i;
>  
> @@ -603,6 +613,13 @@ static int populate_attrs(struct config_item *item)
>  		}
>  	}
>  

No need for this blank line.

> +	if (t->ct_bin_attrs) {
> +		for (i = 0; (bin_attr = t->ct_bin_attrs[i]) != NULL; i++) {
> +			if ((error = configfs_create_bin_file(item, bin_attr)))
> +				break;
> +		}
> +	}
> +
>  	if (error)
>  		detach_attrs(item);
>  
> diff --git a/fs/configfs/file.c b/fs/configfs/file.c
> index 1d1c41f..aec051b 100644
> --- a/fs/configfs/file.c
> +++ b/fs/configfs/file.c
> @@ -48,6 +48,10 @@ struct configfs_buffer {
>  	struct configfs_item_operations	* ops;
>  	struct mutex		mutex;
>  	int			needs_read_fill;
> +	int			read_in_progress;
> +	int			write_in_progress;
> +	char			*bin_buffer;
> +	int			bin_buffer_size;
>  };
>  
>  
> @@ -107,8 +111,15 @@ static ssize_t
>  configfs_read_file(struct file *file, char __user *buf, size_t count, loff_t *ppos)
>  {
>  	struct configfs_buffer * buffer = file->private_data;
> +	struct configfs_dirent * sd = file->f_path.dentry->d_fsdata;
>  	ssize_t retval = 0;
>  
> +	if (WARN_ON(sd == NULL))
> +		return -EINVAL;
> +
> +	if (WARN_ON(!(sd->s_type & CONFIGFS_ITEM_ATTR)))
> +		return -EINVAL;
> +
>  	mutex_lock(&buffer->mutex);
>  	if (buffer->needs_read_fill) {
>  		if ((retval = fill_read_buffer(file->f_path.dentry,buffer)))
> @@ -123,6 +134,93 @@ out:
>  	return retval;
>  }
>  
> +/**
> + *	configfs_read_bin_file - read a binary attribute.
> + *	@file:	file pointer.
> + *	@buf:	buffer to fill.
> + *	@count:	number of bytes to read.
> + *	@ppos:	starting offset in file.
> + *
> + *	Userspace wants to read a binary attribute file. The attribute descriptor
> + *	is in the file's ->d_fsdata. The target item is in the directory's
> + *	->d_fsdata.
> + *
> + *	We check whether we need to refill the buffer. If so we will
> + *	call the attributes' ops->read_bin_attribute() twice. The first time we
> + *	will pass a NULL as a buffer pointer, which the attributes' method
> + *	will use to return the size of the buffer required. If no error
> + *	occurs we will allocate the buffer using kmalloc and call 
> + *	ops->read_bin_atribute() again passing that buffer as an argument.
> + *	Then we just copy to user-space using simple_read_from_buffer.
> + */
> +
> +static ssize_t
> +configfs_read_bin_file(struct file *file, char __user *buf, size_t count, loff_t *ppos)
> +{
> +	struct configfs_buffer * buffer = file->private_data;
> +	struct dentry *dentry = file->f_path.dentry;
> +	struct configfs_dirent * sd = dentry->d_fsdata;
> +	struct config_item * item = to_item(dentry->d_parent);
> +	struct configfs_item_operations * ops = buffer->ops;
> +	struct configfs_attribute * attr = to_attr(dentry);
> +	struct configfs_bin_attribute *bin_attr =
> +		container_of(attr, struct configfs_bin_attribute, attr);

You defined to_bin_attr().  Use it here and in write_bin_file() rather
than open-coding the container_of() calls.

> +	ssize_t retval = 0;
> +	ssize_t len = min_t(size_t, count, PAGE_SIZE);
> +
> +	if (WARN_ON(sd == NULL))
> +		return -EINVAL;
> +
> +	if (WARN_ON(!(sd->s_type & CONFIGFS_ITEM_BIN_ATTR)))
> +		return -EINVAL;
> +
> +	mutex_lock(&buffer->mutex);
> +
> +	/* we don't support switching read/write modes */
> +	if (buffer->write_in_progress) {
> +		retval = -EINVAL;
> +		goto out;
> +	}
> +	if (!buffer->read_in_progress)
> +		buffer->read_in_progress = 1;

You can always set read_in_progress, even if the read has already
started.

> +
> +	if (buffer->needs_read_fill) {
> +
> +		/* perform first read with buf == NULL to get extent */
> +		len = ops->read_bin_attribute(item, bin_attr, NULL, 0);
> +		if (len < 0) {
> +			retval = len;
> +			goto out;
> +		}
> +
> +		buffer->bin_buffer = kmalloc(len, GFP_KERNEL);
> +		if (buffer->bin_buffer == NULL) {
> +			retval = -ENOMEM;
> +			goto out;
> +		}
> +		buffer->bin_buffer_size = len;
> +
> +		/* perform second read to fill buffer */
> +		len = ops->read_bin_attribute(item, bin_attr,
> +				buffer->bin_buffer, len);
> +		if (len < 0) {
> +			retval = len;
> +			kfree(buffer->bin_buffer);
> +			buffer->bin_buffer_size = 0;
> +			buffer->bin_buffer = NULL;
> +			goto out;
> +		}
> +
> +		buffer->needs_read_fill = 0;
> +	}
> +
> +	retval = simple_read_from_buffer(buf, count, ppos, buffer->bin_buffer,
> +					buffer->bin_buffer_size);
> +out:
> +	mutex_unlock(&buffer->mutex);
> +	return retval;
> +}
> +
>  
>  /**
>   *	fill_write_buffer - copy buffer from userspace.
> @@ -198,8 +296,15 @@ static ssize_t
>  configfs_write_file(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
>  {
>  	struct configfs_buffer * buffer = file->private_data;
> +	struct configfs_dirent * sd = file->f_path.dentry->d_fsdata;
>  	ssize_t len;
>  
> +	if (WARN_ON(sd == NULL))
> +		return -EINVAL;
> +
> +	if (WARN_ON(!(sd->s_type & CONFIGFS_ITEM_ATTR)))
> +		return -EINVAL;
> +
>  	mutex_lock(&buffer->mutex);
>  	len = fill_write_buffer(buffer, buf, count);
>  	if (len > 0)
> @@ -210,10 +315,80 @@ configfs_write_file(struct file *file, const char __user *buf, size_t count, lof
>  	return len;
>  }
>  
> -static int check_perm(struct inode * inode, struct file * file)
> +/**
> + *	configfs_write_bin_file - write a binary attribute.
> + *	@file:	file pointer
> + *	@buf:	data to write
> + *	@count:	number of bytes
> + *	@ppos:	starting offset
> + *
> + *	Writting to a binary attribute file is similar to a normal read.
> + *	We buffer the consecutive writes (binary attribute files do not
> + *	support lseek) in a continuously growing buffer, but we don't 
> + *	commit until the close of the file.
> + */
> +
> +static ssize_t
> +configfs_write_bin_file(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
> +{
> +	struct configfs_buffer * buffer = file->private_data;
> +	struct dentry *dentry = file->f_path.dentry;
> +	struct configfs_dirent * sd = dentry->d_fsdata;
> +	void *tbuf = NULL;
> +	ssize_t len;
> +
> +	if (WARN_ON(sd == NULL))
> +		return -EINVAL;
> +
> +	if (WARN_ON(!(sd->s_type & CONFIGFS_ITEM_BIN_ATTR)))
> +		return -EINVAL;
> +
> +	mutex_lock(&buffer->mutex);
> +
> +	/* we don't support switching read/write modes */
> +	if (buffer->read_in_progress) {
> +		len = -EINVAL;
> +		goto out;
> +	}
> +	if (!buffer->write_in_progress)
> +		buffer->write_in_progress = 1;
> +
> +	/* buffer grows? */
> +	if (*ppos + count > buffer->bin_buffer_size) {
> +
> +		tbuf = kmalloc(*ppos + count, GFP_KERNEL);
> +		if (tbuf == NULL) {
> +			len = -ENOMEM;
> +			goto out;
> +		}
> +
> +		/* copy old contents */
> +		if (buffer->bin_buffer) {
> +			memcpy(tbuf, buffer->bin_buffer,
> +					buffer->bin_buffer_size);

Fix the argument alignment.  Do this elsewhere in your patch, too.

> +			kfree(buffer->bin_buffer);
> +		}
> +
> +		/* clear the new area */
> +		memset(tbuf + buffer->bin_buffer_size, 0,
> +				*ppos + count - buffer->bin_buffer_size);
> +		buffer->bin_buffer = tbuf;
> +		buffer->bin_buffer_size = *ppos + count;
> +	}
> +
> +	len = simple_write_to_buffer(buffer->bin_buffer, buffer->bin_buffer_size,
> +			ppos, buf, count);
> +	if (len > 0)
> +		*ppos += len;
> +out:
> +	mutex_unlock(&buffer->mutex);
> +	return len;
> +}
> +

<snip>

> diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
> index 5946ad9..dd9c24b 100644
> --- a/fs/configfs/inode.c
> +++ b/fs/configfs/inode.c
> @@ -231,7 +231,7 @@ const unsigned char * configfs_get_name(struct configfs_dirent *sd)
>  	if (sd->s_type & (CONFIGFS_DIR | CONFIGFS_ITEM_LINK))
>  		return sd->s_dentry->d_name.name;
>  
> -	if (sd->s_type & CONFIGFS_ITEM_ATTR) {
> +	if (sd->s_type & (CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)) {
>  		attr = sd->s_element;
>  		return attr->ca_name;
>  	}
> diff --git a/include/linux/configfs.h b/include/linux/configfs.h
> index 34025df..15f1079 100644
> --- a/include/linux/configfs.h
> +++ b/include/linux/configfs.h

<snip>

> @@ -207,6 +209,89 @@ static ssize_t _item##_attr_store(struct config_item *item,		\
>  	return ret;							\
>  }
>  
> +struct file;
> +struct vm_area_struct;
> +
> +struct configfs_bin_attribute {
> +	struct configfs_attribute	attr;
> +	void				*private;
> +};

cb_attr and cb_private or similar prefixes, please.

Joel

[PATCH] configfs: Implement binary attributes

[Pantelis Antoniou]

ConfigFS lacked binary attributes up until now. This patch
introduces support for binary attributes in a somewhat similar
manner of sysfs binary attributes albeit with changes that
fit the configfs usage model.

Problems that configfs binary attributes fix are everything that
requires a binary blob as part of the configuration of a resource,
such as bitstream loading for FPGAs, DTBs for dynamically created
devices etc.

Look at Documentation/filesystems/configfs/configfs.txt for internals
and howto use them.

This patch generates a bunch of checkpatch warnings, but this stems from
following the formatting of the configfs code, so please ignore.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
---
 Documentation/filesystems/configfs/configfs.txt |  66 +++++-
 fs/configfs/configfs_internal.h                 |  14 +-
 fs/configfs/dir.c                               |  19 +-
 fs/configfs/file.c                              | 302 ++++++++++++++++++++++--
 fs/configfs/inode.c                             |   2 +-
 include/linux/configfs.h                        |  87 +++++++
 6 files changed, 460 insertions(+), 30 deletions(-)

diff --git a/Documentation/filesystems/configfs/configfs.txt b/Documentation/filesystems/configfs/configfs.txt
index b40fec9..fc8a820 100644
--- a/Documentation/filesystems/configfs/configfs.txt
+++ b/Documentation/filesystems/configfs/configfs.txt
@@ -51,15 +51,26 @@ configfs tree is always there, whether mounted on /config or not.
 An item is created via mkdir(2).  The item's attributes will also
 appear at this time.  readdir(3) can determine what the attributes are,
 read(2) can query their default values, and write(2) can store new
-values.  Like sysfs, attributes should be ASCII text files, preferably
-with only one value per file.  The same efficiency caveats from sysfs
-apply.  Don't mix more than one attribute in one attribute file.
-
-Like sysfs, configfs expects write(2) to store the entire buffer at
-once.  When writing to configfs attributes, userspace processes should
-first read the entire file, modify the portions they wish to change, and
-then write the entire buffer back.  Attribute files have a maximum size
-of one page (PAGE_SIZE, 4096 on i386).
+values.  Don't mix more than one attribute in one attribute file.
+
+There are two types of configfs attributes:
+
+* Normal attributes, which similar to sysfs attributes, are small ASCII text
+files, with a maximum size of one page (PAGE_SIZE, 4096 on i386).  Preferably
+only one value per file should be used, and the same caveats from sysfs apply.
+Configfs expects write(2) to store the entire buffer at once.  When writing to
+normal configfs attributes, userspace processes should first read the entire
+file, modify the portions they wish to change, and then write the entire
+buffer back.
+
+* Binary attributes, which are somewhat similar to sysfs binary attributes,
+but with a few slight changes to semantics.  The PAGE_SIZE limitation does not
+apply, but the whole binary item must fit in single kernel kmalloc'ed buffer,
+which in most architectures is a few tens of megabytes.  The write(2) calls
+from user space are buffered, and the attributes' write_bin_attribute method
+will be invoked on the final close, therefore it is imperative for user-space
+to check the return code of close(2) in order to verify that the operation
+finished succesfully.
 
 When an item needs to be destroyed, remove it with rmdir(2).  An
 item cannot be destroyed if any other item has a link to it (via
@@ -166,6 +177,12 @@ among other things.  For that, it needs a type.
 		ssize_t (*store_attribute)(struct config_item *,
 					   struct configfs_attribute *,
 					   const char *, size_t);
+		ssize_t (*read_bin_attribute)(struct config_item *,
+					      struct configfs_bin_attribute *,
+					      void *, size_t);
+		ssize_t (*write_bin_attribute)(struct config_item *,
+					       struct configfs_bin_attribute *,
+					       const void *, size_t);
 		int (*allow_link)(struct config_item *src,
 				  struct config_item *target);
 		int (*drop_link)(struct config_item *src,
@@ -177,15 +194,18 @@ among other things.  For that, it needs a type.
 		struct configfs_item_operations         *ct_item_ops;
 		struct configfs_group_operations        *ct_group_ops;
 		struct configfs_attribute               **ct_attrs;
+		struct configfs_bin_attribute		**ct_bin_attrs;
 	};
 
 The most basic function of a config_item_type is to define what
 operations can be performed on a config_item.  All items that have been
 allocated dynamically will need to provide the ct_item_ops->release()
 method.  This method is called when the config_item's reference count
-reaches zero.  Items that wish to display an attribute need to provide
-the ct_item_ops->show_attribute() method.  Similarly, storing a new
-attribute value uses the store_attribute() method.
+reaches zero.  Items that wish to display an normal attribute need to provide
+the ct_item_ops->show_attribute() method, while binary attributes provide the
+ct_item_ops->read_bin_attribute(). Similarly, storing a new normal attribute
+value uses the store_attribute() method, while the binarys' attribute equivalent
+is the ct_item_ops->write_bin_attribute() method.
 
 [struct configfs_attribute]
 
@@ -207,6 +227,28 @@ ct_item_ops->show_attribute() method, that method will be called
 whenever userspace asks for a read(2) on the attribute.  The converse
 will happen for write(2).
 
+[struct configfs_bin_attribute]
+
+	struct configfs_attribute {
+		struct configfs_attribute	attr;
+		void				*private;
+	};
+
+The binary attribute is used when the one needs to use binary blob to
+appear as the contents of a file in the item's configfs directory.
+To do so add the binary attribute to the NULL-terminated array
+config_item_type->ct_bin_attrs, and the item appears in configfs, the
+attribute file will appear with the configfs_bin_attribute->attr.ca_name
+filename.  configfs_attribute->bin_attribute.ca_mode specifies the file
+permissions.
+
+If binary attribute is readable and the config_item provides a
+ct_item_ops->read_bin_attribute() method, that method will be called
+whenever userspace asks for a read(2) on the attribute.  The converse
+will happen for write(2). The reads/writes are bufferred so only a
+single read/write will occur; the attributes' need not concern itself
+with it.
+
 [struct config_group]
 
 A config_item cannot live in a vacuum.  The only way one can be created
diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
index bd4a3c1..3a3f6d6 100644
--- a/fs/configfs/configfs_internal.h
+++ b/fs/configfs/configfs_internal.h
@@ -53,13 +53,14 @@ struct configfs_dirent {
 #define CONFIGFS_ROOT		0x0001
 #define CONFIGFS_DIR		0x0002
 #define CONFIGFS_ITEM_ATTR	0x0004
+#define CONFIGFS_ITEM_BIN_ATTR	0x0008
 #define CONFIGFS_ITEM_LINK	0x0020
 #define CONFIGFS_USET_DIR	0x0040
 #define CONFIGFS_USET_DEFAULT	0x0080
 #define CONFIGFS_USET_DROPPING	0x0100
 #define CONFIGFS_USET_IN_MKDIR	0x0200
 #define CONFIGFS_USET_CREATING	0x0400
-#define CONFIGFS_NOT_PINNED	(CONFIGFS_ITEM_ATTR)
+#define CONFIGFS_NOT_PINNED	(CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)
 
 extern struct mutex configfs_symlink_mutex;
 extern spinlock_t configfs_dirent_lock;
@@ -74,11 +75,13 @@ extern int configfs_inode_init(void);
 extern void configfs_inode_exit(void);
 
 extern int configfs_create_file(struct config_item *, const struct configfs_attribute *);
+extern int configfs_create_bin_file(struct config_item *, const struct configfs_bin_attribute *);
 extern int configfs_make_dirent(struct configfs_dirent *,
 				struct dentry *, void *, umode_t, int);
 extern int configfs_dirent_is_ready(struct configfs_dirent *);
 
 extern int configfs_add_file(struct dentry *, const struct configfs_attribute *, int);
+extern int configfs_add_bin_file(struct dentry *, const struct configfs_bin_attribute *, int);
 extern void configfs_hash_and_remove(struct dentry * dir, const char * name);
 
 extern const unsigned char * configfs_get_name(struct configfs_dirent *sd);
@@ -91,7 +94,7 @@ extern void configfs_release_fs(void);
 extern struct rw_semaphore configfs_rename_sem;
 extern const struct file_operations configfs_dir_operations;
 extern const struct file_operations configfs_file_operations;
-extern const struct file_operations bin_fops;
+extern const struct file_operations configfs_bin_file_operations;
 extern const struct inode_operations configfs_dir_inode_operations;
 extern const struct inode_operations configfs_root_inode_operations;
 extern const struct inode_operations configfs_symlink_inode_operations;
@@ -122,6 +125,13 @@ static inline struct configfs_attribute * to_attr(struct dentry * dentry)
 	return ((struct configfs_attribute *) sd->s_element);
 }
 
+static inline struct configfs_bin_attribute * to_bin_attr(struct dentry * dentry)
+{
+	struct configfs_attribute * attr = to_attr(dentry);
+
+	return container_of(attr, struct configfs_bin_attribute, attr);
+}
+
 static inline struct config_item *configfs_get_config_item(struct dentry *dentry)
 {
 	struct config_item * item = NULL;
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 668dcab..9474b86 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -257,6 +257,13 @@ static int configfs_init_file(struct inode * inode)
 	return 0;
 }
 
+static int configfs_init_bin_file(struct inode * inode)
+{
+	inode->i_size = 0;
+	inode->i_fop = &configfs_bin_file_operations;
+	return 0;
+}
+
 static int init_symlink(struct inode * inode)
 {
 	inode->i_op = &configfs_symlink_inode_operations;
@@ -431,7 +438,9 @@ static int configfs_attach_attr(struct configfs_dirent * sd, struct dentry * den
 	spin_unlock(&configfs_dirent_lock);
 
 	error = configfs_create(dentry, (attr->ca_mode & S_IALLUGO) | S_IFREG,
-				configfs_init_file);
+				(sd->s_type & CONFIGFS_ITEM_ATTR) ?
+					configfs_init_file :
+					configfs_init_bin_file);
 	if (error) {
 		configfs_put(sd);
 		return error;
@@ -591,6 +600,7 @@ static int populate_attrs(struct config_item *item)
 {
 	struct config_item_type *t = item->ci_type;
 	struct configfs_attribute *attr;
+	struct configfs_bin_attribute *bin_attr;
 	int error = 0;
 	int i;
 
@@ -603,6 +613,13 @@ static int populate_attrs(struct config_item *item)
 		}
 	}
 
+	if (t->ct_bin_attrs) {
+		for (i = 0; (bin_attr = t->ct_bin_attrs[i]) != NULL; i++) {
+			if ((error = configfs_create_bin_file(item, bin_attr)))
+				break;
+		}
+	}
+
 	if (error)
 		detach_attrs(item);
 
diff --git a/fs/configfs/file.c b/fs/configfs/file.c
index 1d1c41f..aec051b 100644
--- a/fs/configfs/file.c
+++ b/fs/configfs/file.c
@@ -48,6 +48,10 @@ struct configfs_buffer {
 	struct configfs_item_operations	* ops;
 	struct mutex		mutex;
 	int			needs_read_fill;
+	int			read_in_progress;
+	int			write_in_progress;
+	char			*bin_buffer;
+	int			bin_buffer_size;
 };
 
 
@@ -107,8 +111,15 @@ static ssize_t
 configfs_read_file(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 {
 	struct configfs_buffer * buffer = file->private_data;
+	struct configfs_dirent * sd = file->f_path.dentry->d_fsdata;
 	ssize_t retval = 0;
 
+	if (WARN_ON(sd == NULL))
+		return -EINVAL;
+
+	if (WARN_ON(!(sd->s_type & CONFIGFS_ITEM_ATTR)))
+		return -EINVAL;
+
 	mutex_lock(&buffer->mutex);
 	if (buffer->needs_read_fill) {
 		if ((retval = fill_read_buffer(file->f_path.dentry,buffer)))
@@ -123,6 +134,93 @@ out:
 	return retval;
 }
 
+/**
+ *	configfs_read_bin_file - read a binary attribute.
+ *	@file:	file pointer.
+ *	@buf:	buffer to fill.
+ *	@count:	number of bytes to read.
+ *	@ppos:	starting offset in file.
+ *
+ *	Userspace wants to read a binary attribute file. The attribute descriptor
+ *	is in the file's ->d_fsdata. The target item is in the directory's
+ *	->d_fsdata.
+ *
+ *	We check whether we need to refill the buffer. If so we will
+ *	call the attributes' ops->read_bin_attribute() twice. The first time we
+ *	will pass a NULL as a buffer pointer, which the attributes' method
+ *	will use to return the size of the buffer required. If no error
+ *	occurs we will allocate the buffer using kmalloc and call 
+ *	ops->read_bin_atribute() again passing that buffer as an argument.
+ *	Then we just copy to user-space using simple_read_from_buffer.
+ */
+
+static ssize_t
+configfs_read_bin_file(struct file *file, char __user *buf, size_t count, loff_t *ppos)
+{
+	struct configfs_buffer * buffer = file->private_data;
+	struct dentry *dentry = file->f_path.dentry;
+	struct configfs_dirent * sd = dentry->d_fsdata;
+	struct config_item * item = to_item(dentry->d_parent);
+	struct configfs_item_operations * ops = buffer->ops;
+	struct configfs_attribute * attr = to_attr(dentry);
+	struct configfs_bin_attribute *bin_attr =
+		container_of(attr, struct configfs_bin_attribute, attr);
+	ssize_t retval = 0;
+	ssize_t len = min_t(size_t, count, PAGE_SIZE);
+
+	if (WARN_ON(sd == NULL))
+		return -EINVAL;
+
+	if (WARN_ON(!(sd->s_type & CONFIGFS_ITEM_BIN_ATTR)))
+		return -EINVAL;
+
+	mutex_lock(&buffer->mutex);
+
+	/* we don't support switching read/write modes */
+	if (buffer->write_in_progress) {
+		retval = -EINVAL;
+		goto out;
+	}
+	if (!buffer->read_in_progress)
+		buffer->read_in_progress = 1;
+
+	if (buffer->needs_read_fill) {
+
+		/* perform first read with buf == NULL to get extent */
+		len = ops->read_bin_attribute(item, bin_attr, NULL, 0);
+		if (len < 0) {
+			retval = len;
+			goto out;
+		}
+
+		buffer->bin_buffer = kmalloc(len, GFP_KERNEL);
+		if (buffer->bin_buffer == NULL) {
+			retval = -ENOMEM;
+			goto out;
+		}
+		buffer->bin_buffer_size = len;
+
+		/* perform second read to fill buffer */
+		len = ops->read_bin_attribute(item, bin_attr,
+				buffer->bin_buffer, len);
+		if (len < 0) {
+			retval = len;
+			kfree(buffer->bin_buffer);
+			buffer->bin_buffer_size = 0;
+			buffer->bin_buffer = NULL;
+			goto out;
+		}
+
+		buffer->needs_read_fill = 0;
+	}
+
+	retval = simple_read_from_buffer(buf, count, ppos, buffer->bin_buffer,
+					buffer->bin_buffer_size);
+out:
+	mutex_unlock(&buffer->mutex);
+	return retval;
+}
+
 
 /**
  *	fill_write_buffer - copy buffer from userspace.
@@ -198,8 +296,15 @@ static ssize_t
 configfs_write_file(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 {
 	struct configfs_buffer * buffer = file->private_data;
+	struct configfs_dirent * sd = file->f_path.dentry->d_fsdata;
 	ssize_t len;
 
+	if (WARN_ON(sd == NULL))
+		return -EINVAL;
+
+	if (WARN_ON(!(sd->s_type & CONFIGFS_ITEM_ATTR)))
+		return -EINVAL;
+
 	mutex_lock(&buffer->mutex);
 	len = fill_write_buffer(buffer, buf, count);
 	if (len > 0)
@@ -210,10 +315,80 @@ configfs_write_file(struct file *file, const char __user *buf, size_t count, lof
 	return len;
 }
 
-static int check_perm(struct inode * inode, struct file * file)
+/**
+ *	configfs_write_bin_file - write a binary attribute.
+ *	@file:	file pointer
+ *	@buf:	data to write
+ *	@count:	number of bytes
+ *	@ppos:	starting offset
+ *
+ *	Writting to a binary attribute file is similar to a normal read.
+ *	We buffer the consecutive writes (binary attribute files do not
+ *	support lseek) in a continuously growing buffer, but we don't 
+ *	commit until the close of the file.
+ */
+
+static ssize_t
+configfs_write_bin_file(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
+{
+	struct configfs_buffer * buffer = file->private_data;
+	struct dentry *dentry = file->f_path.dentry;
+	struct configfs_dirent * sd = dentry->d_fsdata;
+	void *tbuf = NULL;
+	ssize_t len;
+
+	if (WARN_ON(sd == NULL))
+		return -EINVAL;
+
+	if (WARN_ON(!(sd->s_type & CONFIGFS_ITEM_BIN_ATTR)))
+		return -EINVAL;
+
+	mutex_lock(&buffer->mutex);
+
+	/* we don't support switching read/write modes */
+	if (buffer->read_in_progress) {
+		len = -EINVAL;
+		goto out;
+	}
+	if (!buffer->write_in_progress)
+		buffer->write_in_progress = 1;
+
+	/* buffer grows? */
+	if (*ppos + count > buffer->bin_buffer_size) {
+
+		tbuf = kmalloc(*ppos + count, GFP_KERNEL);
+		if (tbuf == NULL) {
+			len = -ENOMEM;
+			goto out;
+		}
+
+		/* copy old contents */
+		if (buffer->bin_buffer) {
+			memcpy(tbuf, buffer->bin_buffer,
+					buffer->bin_buffer_size);
+			kfree(buffer->bin_buffer);
+		}
+
+		/* clear the new area */
+		memset(tbuf + buffer->bin_buffer_size, 0,
+				*ppos + count - buffer->bin_buffer_size);
+		buffer->bin_buffer = tbuf;
+		buffer->bin_buffer_size = *ppos + count;
+	}
+
+	len = simple_write_to_buffer(buffer->bin_buffer, buffer->bin_buffer_size,
+			ppos, buf, count);
+	if (len > 0)
+		*ppos += len;
+out:
+	mutex_unlock(&buffer->mutex);
+	return len;
+}
+
+static int check_perm(struct inode * inode, struct file * file,
+		struct config_item * item, struct configfs_attribute * attr,
+		int type)
 {
-	struct config_item *item = configfs_get_config_item(file->f_path.dentry->d_parent);
-	struct configfs_attribute * attr = to_attr(file->f_path.dentry);
 	struct configfs_buffer * buffer;
 	struct configfs_item_operations * ops = NULL;
 	int error = 0;
@@ -238,9 +413,14 @@ static int check_perm(struct inode * inode, struct file * file)
 	 */
 	if (file->f_mode & FMODE_WRITE) {
 
-		if (!(inode->i_mode & S_IWUGO) || !ops->store_attribute)
+		if (!(inode->i_mode & S_IWUGO))
+			goto Eaccess;
+
+		if ((type & CONFIGFS_ITEM_ATTR) && !ops->store_attribute)
 			goto Eaccess;
 
+		if ((type & CONFIGFS_ITEM_BIN_ATTR) && !ops->write_bin_attribute)
+			goto Eaccess;
 	}
 
 	/* File needs read support.
@@ -248,7 +428,13 @@ static int check_perm(struct inode * inode, struct file * file)
 	 * must be a show method for it.
 	 */
 	if (file->f_mode & FMODE_READ) {
-		if (!(inode->i_mode & S_IRUGO) || !ops->show_attribute)
+		if (!(inode->i_mode & S_IRUGO))
+			goto Eaccess;
+
+		if ((type & CONFIGFS_ITEM_ATTR) && !ops->show_attribute)
+			goto Eaccess;
+
+		if ((type & CONFIGFS_ITEM_BIN_ATTR) && !ops->read_bin_attribute)
 			goto Eaccess;
 	}
 
@@ -262,6 +448,8 @@ static int check_perm(struct inode * inode, struct file * file)
 	}
 	mutex_init(&buffer->mutex);
 	buffer->needs_read_fill = 1;
+	buffer->read_in_progress = 0;
+	buffer->write_in_progress = 0;
 	buffer->ops = ops;
 	file->private_data = buffer;
 	goto Done;
@@ -279,15 +467,10 @@ static int check_perm(struct inode * inode, struct file * file)
 	return error;
 }
 
-static int configfs_open_file(struct inode * inode, struct file * filp)
-{
-	return check_perm(inode,filp);
-}
-
-static int configfs_release(struct inode * inode, struct file * filp)
+static int do_release(struct inode * inode, struct file * filp,
+		struct config_item *item, struct configfs_attribute *attr,
+		int type)
 {
-	struct config_item * item = to_item(filp->f_path.dentry->d_parent);
-	struct configfs_attribute * attr = to_attr(filp->f_path.dentry);
 	struct module * owner = attr->ca_owner;
 	struct configfs_buffer * buffer = filp->private_data;
 
@@ -305,14 +488,90 @@ static int configfs_release(struct inode * inode, struct file * filp)
 	return 0;
 }
 
+static int configfs_open_file(struct inode * inode, struct file * filp)
+{
+	return check_perm(inode, filp,
+			configfs_get_config_item(filp->f_path.dentry->d_parent),
+			to_attr(filp->f_path.dentry), CONFIGFS_ITEM_ATTR);
+}
+
+static int configfs_release_file(struct inode * inode, struct file * filp)
+{
+	return do_release(inode, filp,
+			to_item(filp->f_path.dentry->d_parent),
+			to_attr(filp->f_path.dentry), CONFIGFS_ITEM_ATTR);
+}
+
+static int configfs_open_bin_file(struct inode * inode, struct file * filp)
+{
+	return check_perm(inode, filp,
+			configfs_get_config_item(filp->f_path.dentry->d_parent),
+			to_attr(filp->f_path.dentry), CONFIGFS_ITEM_BIN_ATTR);
+}
+
+/**
+ *	configfs_release_bin_file - write a binary attribute.
+ *	@inode: inode pointer
+ *	@filp:	file pointer
+ *
+ *	Releasing a binary attribute file is similar for a read op
+ *	to the normal attribute file. When writting we call the
+ *	attributes' ops->write_bin_attribute() method to commit the
+ *	changes to the attribute.
+ */
+
+static int configfs_release_bin_file(struct inode * inode, struct file * filp)
+{
+	struct configfs_buffer * buffer = filp->private_data;
+	struct dentry *dentry = filp->f_path.dentry;
+	struct config_item * item = to_item(dentry->d_parent);
+	struct configfs_item_operations * ops = buffer->ops;
+	struct configfs_attribute * attr = to_attr(dentry);
+	struct configfs_bin_attribute *bin_attr =
+		container_of(attr, struct configfs_bin_attribute, attr);
+	ssize_t len = 0;
+	int ret;
+
+	if (buffer->read_in_progress)
+		buffer->read_in_progress = 0;
+
+	if (buffer->write_in_progress) {
+		buffer->write_in_progress = 0;
+
+		len = ops->write_bin_attribute(item, bin_attr, buffer->bin_buffer,
+				buffer->bin_buffer_size);
+
+		/* kfree on NULL is safe */
+		kfree(buffer->bin_buffer);
+		buffer->bin_buffer = NULL;
+		buffer->bin_buffer_size = 0;
+		buffer->needs_read_fill = 1;
+	}
+
+	ret = do_release(inode, filp,
+			to_item(filp->f_path.dentry->d_parent),
+			to_attr(filp->f_path.dentry), CONFIGFS_ITEM_BIN_ATTR);
+	if (len < 0)
+		return len;
+	return ret;
+}
+
+
 const struct file_operations configfs_file_operations = {
 	.read		= configfs_read_file,
 	.write		= configfs_write_file,
 	.llseek		= generic_file_llseek,
 	.open		= configfs_open_file,
-	.release	= configfs_release,
+	.release	= configfs_release_file,
 };
 
+const struct file_operations configfs_bin_file_operations = {
+	.read		= configfs_read_bin_file,
+	.write		= configfs_write_bin_file,
+	.llseek		= NULL,		/* bin file is not seekable */
+	.open		= configfs_open_bin_file,
+	.release	= configfs_release_bin_file,
+};
 
 int configfs_add_file(struct dentry * dir, const struct configfs_attribute * attr, int type)
 {
@@ -342,3 +601,18 @@ int configfs_create_file(struct config_item * item, const struct configfs_attrib
 				 CONFIGFS_ITEM_ATTR);
 }
 
+/**
+ *	configfs_create_bin_file - create a binary attribute file for an item.
+ *	@item:	item we're creating for.
+ *	@attr:	atrribute descriptor.
+ */
+
+int configfs_create_bin_file(struct config_item * item,
+		const struct configfs_bin_attribute * bin_attr)
+{
+	BUG_ON(!item || !item->ci_dentry || !bin_attr);
+
+	return configfs_add_file(item->ci_dentry, &bin_attr->attr,
+				 CONFIGFS_ITEM_BIN_ATTR);
+}
+
diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
index 5946ad9..dd9c24b 100644
--- a/fs/configfs/inode.c
+++ b/fs/configfs/inode.c
@@ -231,7 +231,7 @@ const unsigned char * configfs_get_name(struct configfs_dirent *sd)
 	if (sd->s_type & (CONFIGFS_DIR | CONFIGFS_ITEM_LINK))
 		return sd->s_dentry->d_name.name;
 
-	if (sd->s_type & CONFIGFS_ITEM_ATTR) {
+	if (sd->s_type & (CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)) {
 		attr = sd->s_element;
 		return attr->ca_name;
 	}
diff --git a/include/linux/configfs.h b/include/linux/configfs.h
index 34025df..15f1079 100644
--- a/include/linux/configfs.h
+++ b/include/linux/configfs.h
@@ -51,6 +51,7 @@ struct module;
 struct configfs_item_operations;
 struct configfs_group_operations;
 struct configfs_attribute;
+struct configfs_bin_attribute;
 struct configfs_subsystem;
 
 struct config_item {
@@ -84,6 +85,7 @@ struct config_item_type {
 	struct configfs_item_operations		*ct_item_ops;
 	struct configfs_group_operations	*ct_group_ops;
 	struct configfs_attribute		**ct_attrs;
+	struct configfs_bin_attribute		**ct_bin_attrs;
 };
 
 /**
@@ -207,6 +209,89 @@ static ssize_t _item##_attr_store(struct config_item *item,		\
 	return ret;							\
 }
 
+struct file;
+struct vm_area_struct;
+
+struct configfs_bin_attribute {
+	struct configfs_attribute	attr;
+	void				*private;
+};
+
+/*
+ * Similar to CONFIGFS_ATTR_STRUCT
+ */
+#define CONFIGFS_BIN_ATTR_STRUCT(_item)					\
+struct _item##_bin_attribute {						\
+	struct configfs_bin_attribute bin_attr;				\
+	ssize_t (*read)(struct _item *, void *, size_t);		\
+	ssize_t (*write)(struct _item *, const void *, size_t);		\
+}
+
+/* macros to create static binary attributes easier */
+#define __CONFIGFS_BIN_ATTR(_name, _mode, _read, _write)		\
+{									\
+	.bin_attr = {							\
+		.attr	= {						\
+				.ca_name = __stringify(_name),		\
+				.ca_mode = _mode,			\
+				.ca_owner = THIS_MODULE,		\
+		},							\
+	},								\
+	.read	= _read,						\
+	.write	= _write,						\
+}
+
+#define __CONFIGFS_BIN_ATTR_RO(_name) {					\
+	.attr	= {							\
+			.ca_name = __stringify(_name),			\
+			.ca_mode = _mode,				\
+			.ca_owner = THIS_MODULE,			\
+	},								\
+	.read	= _name##_read,						\
+}
+
+#define __CONFIGFS_BIN_ATTR_RW(_name)					\
+	__CONFIGFS_BIN_ATTR(_name, (S_IWUSR | S_IRUGO),			\
+		_name##_read, _name##_write)
+
+#define CONFIGFS_BIN_ATTR(_name, _mode, _read, _write)			\
+struct configfs_bin_attribute bin_attr_##_name =			\
+	__CONFIGFS_BIN_ATTR(_name, _mode, _read, _write)
+
+#define CONFIGFS_BIN_ATTR_RO(_name)					\
+struct configfs_bin_attribute bin_attr_##_name =			\
+	__CONFIGFS_BIN_ATTR_RO(_name)
+
+#define CONFIGFS_BIN_ATTR_OPS(_item)					\
+static ssize_t _item##_bin_attr_read(struct config_item *item,		\
+			struct configfs_bin_attribute *bin_attr,	\
+			void *buf, size_t max_count)			\
+{									\
+	struct _item *_item = to_##_item(item);				\
+	struct _item##_bin_attribute *_item##_bin_attr =		\
+		container_of(bin_attr, struct _item##_bin_attribute,	\
+				bin_attr);				\
+	ssize_t ret = 0;						\
+									\
+	if (_item##_bin_attr->read)					\
+		ret = _item##_bin_attr->read(_item, buf, max_count);	\
+	return ret;							\
+}									\
+static ssize_t _item##_bin_attr_write(struct config_item *item,		\
+			struct configfs_bin_attribute *bin_attr,	\
+			const void *buf, size_t count)			\
+{									\
+	struct _item *_item = to_##_item(item);				\
+	struct _item##_bin_attribute *_item##_bin_attr =		\
+		container_of(bin_attr, struct _item##_bin_attribute,	\
+				bin_attr);				\
+	ssize_t ret = -EINVAL;						\
+									\
+	if (_item##_bin_attr->write)					\
+		ret = _item##_bin_attr->write(_item, buf, count);	\
+	return ret;							\
+}
+
 /*
  * If allow_link() exists, the item can symlink(2) out to other
  * items.  If the item is a group, it may support mkdir(2).
@@ -225,6 +310,8 @@ struct configfs_item_operations {
 	void (*release)(struct config_item *);
 	ssize_t	(*show_attribute)(struct config_item *, struct configfs_attribute *,char *);
 	ssize_t	(*store_attribute)(struct config_item *,struct configfs_attribute *,const char *, size_t);
+	ssize_t (*read_bin_attribute)(struct config_item *, struct configfs_bin_attribute *, void *, size_t);
+	ssize_t (*write_bin_attribute)(struct config_item *, struct configfs_bin_attribute *, const void *, size_t);
 	int (*allow_link)(struct config_item *src, struct config_item *target);
 	int (*drop_link)(struct config_item *src, struct config_item *target);
 };
-- 
1.7.12