Re: [PATCH 2/2] target/file: merge fd_do_readv() and fd_do_writev()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2012-12-05 at 12:08 +0100, Sebastian Andrzej Siewior wrote:
> Those two functions are almost identical so merge them. Noticed this
> while fixing the highmem in both cases.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
>  drivers/target/target_core_file.c |  101 +++++++++++++------------------------
>  1 file changed, 36 insertions(+), 65 deletions(-)
> 

Applied to for-next with a minor bit of fuzz.

Thanks again Sebastian!

--nab

> diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
> index c639b42..8d8a552 100644
> --- a/drivers/target/target_core_file.c
> +++ b/drivers/target/target_core_file.c
> @@ -239,8 +239,8 @@ static void fd_free_device(void *p)
>  	kfree(fd_dev);
>  }
>  
> -static int fd_do_readv(struct se_cmd *cmd, struct scatterlist *sgl,
> -		u32 sgl_nents)
> +static int fd_do_rw(struct se_cmd *cmd, struct scatterlist *sgl,
> +		u32 sgl_nents, int is_write)
>  {
>  	struct se_device *se_dev = cmd->se_dev;
>  	struct fd_dev *dev = se_dev->dev_ptr;
> @@ -248,13 +248,13 @@ static int fd_do_readv(struct se_cmd *cmd, struct scatterlist *sgl,
>  	struct scatterlist *sg;
>  	struct iovec *iov;
>  	mm_segment_t old_fs;
> -	loff_t pos = (cmd->t_task_lba *
> -		      se_dev->se_sub_dev->se_dev_attrib.block_size);
> -	int ret = 0, i;
> +	loff_t pos = cmd->t_task_lba *
> +		se_dev->se_sub_dev->se_dev_attrib.block_size;
> +	int ret, i;
>  
>  	iov = kzalloc(sizeof(struct iovec) * sgl_nents, GFP_KERNEL);
>  	if (!iov) {
> -		pr_err("Unable to allocate fd_do_readv iov[]\n");
> +		pr_err("Unable to allocate fd_do_writev iov[]\n");
>  		return -ENOMEM;
>  	}
>  
> @@ -265,74 +265,45 @@ static int fd_do_readv(struct se_cmd *cmd, struct scatterlist *sgl,
>  
>  	old_fs = get_fs();
>  	set_fs(get_ds());
> -	ret = vfs_readv(fd, &iov[0], sgl_nents, &pos);
> +
> +	if (is_write)
> +		ret = vfs_writev(fd, &iov[0], sgl_nents, &pos);
> +	else
> +		ret = vfs_readv(fd, &iov[0], sgl_nents, &pos);
> +
>  	set_fs(old_fs);
>  
>  	for_each_sg(sgl, sg, sgl_nents, i)
>  		kunmap(sg_page(sg));
> +
>  	kfree(iov);
> -	/*
> -	 * Return zeros and GOOD status even if the READ did not return
> -	 * the expected virt_size for struct file w/o a backing struct
> -	 * block_device.
> -	 */
> -	if (S_ISBLK(fd->f_dentry->d_inode->i_mode)) {
> +
> +	if (is_write) {
>  		if (ret < 0 || ret != cmd->data_length) {
> -			pr_err("vfs_readv() returned %d,"
> -				" expecting %d for S_ISBLK\n", ret,
> -				(int)cmd->data_length);
> +			pr_err("%s() write returned %d\n", __func__, ret);
>  			return (ret < 0 ? ret : -EINVAL);
>  		}
>  	} else {
> -		if (ret < 0) {
> -			pr_err("vfs_readv() returned %d for non"
> -				" S_ISBLK\n", ret);
> -			return ret;
> +		/*
> +		 * Return zeros and GOOD status even if the READ did not return
> +		 * the expected virt_size for struct file w/o a backing struct
> +		 * block_device.
> +		 */
> +		if (S_ISBLK(fd->f_dentry->d_inode->i_mode)) {
> +			if (ret < 0 || ret != cmd->data_length) {
> +				pr_err("%s() returned %d, expecting %u for "
> +						"S_ISBLK\n", __func__, ret,
> +						cmd->data_length);
> +				return (ret < 0 ? ret : -EINVAL);
> +			}
> +		} else {
> +			if (ret < 0) {
> +				pr_err("%s() returned %d for non S_ISBLK\n",
> +						__func__, ret);
> +				return ret;
> +			}
>  		}
>  	}
> -
> -	return 1;
> -}
> -
> -static int fd_do_writev(struct se_cmd *cmd, struct scatterlist *sgl,
> -		u32 sgl_nents)
> -{
> -	struct se_device *se_dev = cmd->se_dev;
> -	struct fd_dev *dev = se_dev->dev_ptr;
> -	struct file *fd = dev->fd_file;
> -	struct scatterlist *sg;
> -	struct iovec *iov;
> -	mm_segment_t old_fs;
> -	loff_t pos = (cmd->t_task_lba *
> -		      se_dev->se_sub_dev->se_dev_attrib.block_size);
> -	int ret, i = 0;
> -
> -	iov = kzalloc(sizeof(struct iovec) * sgl_nents, GFP_KERNEL);
> -	if (!iov) {
> -		pr_err("Unable to allocate fd_do_writev iov[]\n");
> -		return -ENOMEM;
> -	}
> -
> -	for_each_sg(sgl, sg, sgl_nents, i) {
> -		iov[i].iov_len = sg->length;
> -		iov[i].iov_base = kmap(sg_page(sg)) + sg->offset;
> -	}
> -
> -	old_fs = get_fs();
> -	set_fs(get_ds());
> -	ret = vfs_writev(fd, &iov[0], sgl_nents, &pos);
> -	set_fs(old_fs);
> -
> -	for_each_sg(sgl, sg, sgl_nents, i)
> -		kunmap(sg_page(sg));
> -
> -	kfree(iov);
> -
> -	if (ret < 0 || ret != cmd->data_length) {
> -		pr_err("vfs_writev() returned %d\n", ret);
> -		return (ret < 0 ? ret : -EINVAL);
> -	}
> -
>  	return 1;
>  }
>  
> @@ -395,9 +366,9 @@ static int fd_execute_rw(struct se_cmd *cmd)
>  	 * physical memory addresses to struct iovec virtual memory.
>  	 */
>  	if (data_direction == DMA_FROM_DEVICE) {
> -		ret = fd_do_readv(cmd, sgl, sgl_nents);
> +		ret = fd_do_rw(cmd, sgl, sgl_nents, 0);
>  	} else {
> -		ret = fd_do_writev(cmd, sgl, sgl_nents);
> +		ret = fd_do_rw(cmd, sgl, sgl_nents, 1);
>  		/*
>  		 * Perform implict vfs_fsync_range() for fd_do_writev() ops
>  		 * for SCSI WRITEs with Forced Unit Access (FUA) set.


--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Device Mapper]

  Powered by Linux