sysfs: use seq_file when reading regular files
sysfs read path implements its own buffering scheme between userland and kernel callbacks, which essentially is a degenerate duplicate of seq_file. This patch replaces the custom read buffering implementation in sysfs with seq_file. While the amount of code reduction is small, this reduces low level hairiness and enables future development of a new versatile API based on seq_file so that sysfs features can be shared with other subsystems. As write path was already converted to not use sysfs_open_file->page, this patch makes ->page and ->count unused and removes them. Userland behavior remains the same except for some extreme corner cases - e.g. sysfs will now regenerate the content each time a file is read after a non-contiguous seek whereas the original code would keep using the same content. While this is a userland visible behavior change, it is extremely unlikely to be noticeable and brings sysfs behavior closer to that of procfs. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Kay Sievers <kay@vrfy.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
		
							parent
							
								
									8ef445f080
								
							
						
					
					
						commit
						13c589d5b0
					
				
							
								
								
									
										164
									
								
								fs/sysfs/file.c
									
									
									
									
									
								
							
							
						
						
									
										164
									
								
								fs/sysfs/file.c
									
									
									
									
									
								
							| @ -21,6 +21,7 @@ | ||||
| #include <linux/mutex.h> | ||||
| #include <linux/limits.h> | ||||
| #include <linux/uaccess.h> | ||||
| #include <linux/seq_file.h> | ||||
| 
 | ||||
| #include "sysfs.h" | ||||
| 
 | ||||
| @ -31,7 +32,8 @@ | ||||
|  * sysfs_dirent->s_attr.open points to sysfs_open_dirent.  s_attr.open is | ||||
|  * protected by sysfs_open_dirent_lock. | ||||
|  * | ||||
|  * filp->private_data points to sysfs_open_file which is chained at | ||||
|  * filp->private_data points to seq_file whose ->private points to | ||||
|  * sysfs_open_file.  sysfs_open_files are chained at | ||||
|  * sysfs_open_dirent->files, which is protected by sysfs_open_file_mutex. | ||||
|  */ | ||||
| static DEFINE_SPINLOCK(sysfs_open_dirent_lock); | ||||
| @ -47,13 +49,16 @@ struct sysfs_open_dirent { | ||||
| struct sysfs_open_file { | ||||
| 	struct sysfs_dirent	*sd; | ||||
| 	struct file		*file; | ||||
| 	size_t			count; | ||||
| 	char			*page; | ||||
| 	struct mutex		mutex; | ||||
| 	int			event; | ||||
| 	struct list_head	list; | ||||
| }; | ||||
| 
 | ||||
| static struct sysfs_open_file *sysfs_of(struct file *file) | ||||
| { | ||||
| 	return ((struct seq_file *)file->private_data)->private; | ||||
| } | ||||
| 
 | ||||
| /*
 | ||||
|  * Determine ktype->sysfs_ops for the given sysfs_dirent.  This function | ||||
|  * must be called while holding an active reference. | ||||
| @ -66,40 +71,54 @@ static const struct sysfs_ops *sysfs_file_ops(struct sysfs_dirent *sd) | ||||
| 	return kobj->ktype ? kobj->ktype->sysfs_ops : NULL; | ||||
| } | ||||
| 
 | ||||
| /**
 | ||||
|  *	fill_read_buffer - allocate and fill buffer from object. | ||||
|  *	@dentry:	dentry pointer. | ||||
|  *	@buffer:	data buffer for file. | ||||
|  * | ||||
|  *	Allocate @buffer->page, if it hasn't been already, then call the | ||||
|  *	kobject's show() method to fill the buffer with this attribute's | ||||
|  *	data. | ||||
|  *	This is called only once, on the file's first read unless an error | ||||
|  *	is returned. | ||||
| /*
 | ||||
|  * Reads on sysfs are handled through seq_file, which takes care of hairy | ||||
|  * details like buffering and seeking.  The following function pipes | ||||
|  * sysfs_ops->show() result through seq_file. | ||||
|  */ | ||||
| static int fill_read_buffer(struct dentry *dentry, struct sysfs_open_file *of) | ||||
| static int sysfs_seq_show(struct seq_file *sf, void *v) | ||||
| { | ||||
| 	struct sysfs_dirent *attr_sd = dentry->d_fsdata; | ||||
| 	struct kobject *kobj = attr_sd->s_parent->s_dir.kobj; | ||||
| 	struct sysfs_open_file *of = sf->private; | ||||
| 	struct kobject *kobj = of->sd->s_parent->s_dir.kobj; | ||||
| 	const struct sysfs_ops *ops; | ||||
| 	int ret = 0; | ||||
| 	char *buf; | ||||
| 	ssize_t count; | ||||
| 
 | ||||
| 	if (!of->page) | ||||
| 		of->page = (char *) get_zeroed_page(GFP_KERNEL); | ||||
| 	if (!of->page) | ||||
| 		return -ENOMEM; | ||||
| 	/* acquire buffer and ensure that it's >= PAGE_SIZE */ | ||||
| 	count = seq_get_buf(sf, &buf); | ||||
| 	if (count < PAGE_SIZE) { | ||||
| 		seq_commit(sf, -1); | ||||
| 		return 0; | ||||
| 	} | ||||
| 
 | ||||
| 	/* need attr_sd for attr and ops, its parent for kobj */ | ||||
| 	if (!sysfs_get_active(attr_sd)) | ||||
| 	/*
 | ||||
| 	 * Need @of->sd for attr and ops, its parent for kobj.  @of->mutex | ||||
| 	 * nests outside active ref and is just to ensure that the ops | ||||
| 	 * aren't called concurrently for the same open file. | ||||
| 	 */ | ||||
| 	mutex_lock(&of->mutex); | ||||
| 	if (!sysfs_get_active(of->sd)) { | ||||
| 		mutex_unlock(&of->mutex); | ||||
| 		return -ENODEV; | ||||
| 	} | ||||
| 
 | ||||
| 	of->event = atomic_read(&attr_sd->s_attr.open->event); | ||||
| 	of->event = atomic_read(&of->sd->s_attr.open->event); | ||||
| 
 | ||||
| 	ops = sysfs_file_ops(attr_sd); | ||||
| 	count = ops->show(kobj, attr_sd->s_attr.attr, of->page); | ||||
| 	/*
 | ||||
| 	 * Lookup @ops and invoke show().  Control may reach here via seq | ||||
| 	 * file lseek even if @ops->show() isn't implemented. | ||||
| 	 */ | ||||
| 	ops = sysfs_file_ops(of->sd); | ||||
| 	if (ops->show) | ||||
| 		count = ops->show(kobj, of->sd->s_attr.attr, buf); | ||||
| 	else | ||||
| 		count = 0; | ||||
| 
 | ||||
| 	sysfs_put_active(attr_sd); | ||||
| 	sysfs_put_active(of->sd); | ||||
| 	mutex_unlock(&of->mutex); | ||||
| 
 | ||||
| 	if (count < 0) | ||||
| 		return count; | ||||
| 
 | ||||
| 	/*
 | ||||
| 	 * The code works fine with PAGE_SIZE return but it's likely to | ||||
| @ -111,54 +130,8 @@ static int fill_read_buffer(struct dentry *dentry, struct sysfs_open_file *of) | ||||
| 		/* Try to struggle along */ | ||||
| 		count = PAGE_SIZE - 1; | ||||
| 	} | ||||
| 	if (count >= 0) | ||||
| 		of->count = count; | ||||
| 	else | ||||
| 		ret = count; | ||||
| 	return ret; | ||||
| } | ||||
| 
 | ||||
| /**
 | ||||
|  *	sysfs_read_file - read an attribute. | ||||
|  *	@file:	file pointer. | ||||
|  *	@buf:	buffer to fill. | ||||
|  *	@count:	number of bytes to read. | ||||
|  *	@ppos:	starting offset in file. | ||||
|  * | ||||
|  *	Userspace wants to read an attribute file. The attribute descriptor | ||||
|  *	is in the file's ->d_fsdata. The target object is in the directory's | ||||
|  *	->d_fsdata. | ||||
|  * | ||||
|  *	We call fill_read_buffer() to allocate and fill the buffer from the | ||||
|  *	object's show() method exactly once (if the read is happening from | ||||
|  *	the beginning of the file). That should fill the entire buffer with | ||||
|  *	all the data the object has to offer for that attribute. | ||||
|  *	We then call flush_read_buffer() to copy the buffer to userspace | ||||
|  *	in the increments specified. | ||||
|  */ | ||||
| 
 | ||||
| static ssize_t | ||||
| sysfs_read_file(struct file *file, char __user *buf, size_t count, loff_t *ppos) | ||||
| { | ||||
| 	struct sysfs_open_file *of = file->private_data; | ||||
| 	ssize_t retval = 0; | ||||
| 
 | ||||
| 	mutex_lock(&of->mutex); | ||||
| 	/*
 | ||||
| 	 * Fill on zero offset and the first read so that silly things like | ||||
| 	 * "dd bs=1 skip=N" can work on sysfs files. | ||||
| 	 */ | ||||
| 	if (*ppos == 0 || !of->page) { | ||||
| 		retval = fill_read_buffer(file->f_path.dentry, of); | ||||
| 		if (retval) | ||||
| 			goto out; | ||||
| 	} | ||||
| 	pr_debug("%s: count = %zd, ppos = %lld, buf = %s\n", | ||||
| 		 __func__, count, *ppos, of->page); | ||||
| 	retval = simple_read_from_buffer(buf, count, ppos, of->page, of->count); | ||||
| out: | ||||
| 	mutex_unlock(&of->mutex); | ||||
| 	return retval; | ||||
| 	seq_commit(sf, count); | ||||
| 	return 0; | ||||
| } | ||||
| 
 | ||||
| /**
 | ||||
| @ -216,7 +189,7 @@ static int flush_write_buffer(struct sysfs_open_file *of, char *buf, | ||||
| static ssize_t sysfs_write_file(struct file *file, const char __user *user_buf, | ||||
| 				size_t count, loff_t *ppos) | ||||
| { | ||||
| 	struct sysfs_open_file *of = file->private_data; | ||||
| 	struct sysfs_open_file *of = sysfs_of(file); | ||||
| 	ssize_t len = min_t(size_t, count, PAGE_SIZE - 1); | ||||
| 	char *buf; | ||||
| 
 | ||||
| @ -364,10 +337,7 @@ static int sysfs_open_file(struct inode *inode, struct file *file) | ||||
| 			goto err_out; | ||||
| 	} | ||||
| 
 | ||||
| 	/*
 | ||||
| 	 * No error? Great, allocate a sysfs_open_file for the file, and | ||||
| 	 * store it it in file->private_data for easy access. | ||||
| 	 */ | ||||
| 	/* allocate a sysfs_open_file for the file */ | ||||
| 	error = -ENOMEM; | ||||
| 	of = kzalloc(sizeof(struct sysfs_open_file), GFP_KERNEL); | ||||
| 	if (!of) | ||||
| @ -376,20 +346,34 @@ static int sysfs_open_file(struct inode *inode, struct file *file) | ||||
| 	mutex_init(&of->mutex); | ||||
| 	of->sd = attr_sd; | ||||
| 	of->file = file; | ||||
| 	file->private_data = of; | ||||
| 
 | ||||
| 	/*
 | ||||
| 	 * Always instantiate seq_file even if read access is not | ||||
| 	 * implemented or requested.  This unifies private data access and | ||||
| 	 * most files are readable anyway. | ||||
| 	 */ | ||||
| 	error = single_open(file, sysfs_seq_show, of); | ||||
| 	if (error) | ||||
| 		goto err_free; | ||||
| 
 | ||||
| 	/* seq_file clears PWRITE unconditionally, restore it if WRITE */ | ||||
| 	if (file->f_mode & FMODE_WRITE) | ||||
| 		file->f_mode |= FMODE_PWRITE; | ||||
| 
 | ||||
| 	/* make sure we have open dirent struct */ | ||||
| 	error = sysfs_get_open_dirent(attr_sd, of); | ||||
| 	if (error) | ||||
| 		goto err_free; | ||||
| 		goto err_close; | ||||
| 
 | ||||
| 	/* open succeeded, put active references */ | ||||
| 	sysfs_put_active(attr_sd); | ||||
| 	return 0; | ||||
| 
 | ||||
|  err_free: | ||||
| err_close: | ||||
| 	single_release(inode, file); | ||||
| err_free: | ||||
| 	kfree(of); | ||||
|  err_out: | ||||
| err_out: | ||||
| 	sysfs_put_active(attr_sd); | ||||
| 	return error; | ||||
| } | ||||
| @ -397,12 +381,10 @@ static int sysfs_open_file(struct inode *inode, struct file *file) | ||||
| static int sysfs_release(struct inode *inode, struct file *filp) | ||||
| { | ||||
| 	struct sysfs_dirent *sd = filp->f_path.dentry->d_fsdata; | ||||
| 	struct sysfs_open_file *of = filp->private_data; | ||||
| 	struct sysfs_open_file *of = sysfs_of(filp); | ||||
| 
 | ||||
| 	sysfs_put_open_dirent(sd, of); | ||||
| 
 | ||||
| 	if (of->page) | ||||
| 		free_page((unsigned long)of->page); | ||||
| 	single_release(inode, filp); | ||||
| 	kfree(of); | ||||
| 
 | ||||
| 	return 0; | ||||
| @ -423,7 +405,7 @@ static int sysfs_release(struct inode *inode, struct file *filp) | ||||
|  */ | ||||
| static unsigned int sysfs_poll(struct file *filp, poll_table *wait) | ||||
| { | ||||
| 	struct sysfs_open_file *of = filp->private_data; | ||||
| 	struct sysfs_open_file *of = sysfs_of(filp); | ||||
| 	struct sysfs_dirent *attr_sd = filp->f_path.dentry->d_fsdata; | ||||
| 	struct sysfs_open_dirent *od = attr_sd->s_attr.open; | ||||
| 
 | ||||
| @ -481,9 +463,9 @@ void sysfs_notify(struct kobject *k, const char *dir, const char *attr) | ||||
| EXPORT_SYMBOL_GPL(sysfs_notify); | ||||
| 
 | ||||
| const struct file_operations sysfs_file_operations = { | ||||
| 	.read		= sysfs_read_file, | ||||
| 	.read		= seq_read, | ||||
| 	.write		= sysfs_write_file, | ||||
| 	.llseek		= generic_file_llseek, | ||||
| 	.llseek		= seq_lseek, | ||||
| 	.open		= sysfs_open_file, | ||||
| 	.release	= sysfs_release, | ||||
| 	.poll		= sysfs_poll, | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user