Re: [PATCH 3/3] target: use vfs_bvec_read/write

[Sagi Grimberg <sagig@dev.mellanox.co.il>]

On 1/18/2015 5:07 PM, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/target/target_core_file.c | 29 ++++++++++++-----------------
>   1 file changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
> index d836de2..63ca7d5 100644
> --- a/drivers/target/target_core_file.c
> +++ b/drivers/target/target_core_file.c
> @@ -331,36 +331,31 @@ static int fd_do_rw(struct se_cmd *cmd, struct scatterlist *sgl,
>   	struct fd_dev *dev = FD_DEV(se_dev);
>   	struct file *fd = dev->fd_file;
>   	struct scatterlist *sg;
> -	struct iovec *iov;
> -	mm_segment_t old_fs;
> +	struct bio_vec *bvec;
> +	ssize_t len = 0;
>   	loff_t pos = (cmd->t_task_lba * se_dev->dev_attrib.block_size);
>   	int ret = 0, i;
>
> -	iov = kzalloc(sizeof(struct iovec) * sgl_nents, GFP_KERNEL);
> -	if (!iov) {
> +	bvec = kzalloc(sizeof(struct bio_vec) * sgl_nents, GFP_KERNEL);
> +	if (!bvec) {

Can you make that kcalloc so checkpatch would be happy?

Otherwise, Looks good to me.
Reviewed-by: Sagi Grimberg <sagig@mellanox.com>

>   		pr_err("Unable to allocate fd_do_readv 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;
> -	}
> +		bvec[i].bv_page = sg_page(sg);
> +		bvec[i].bv_len = sg->length;
> +		bvec[i].bv_offset = sg->offset;
>
> -	old_fs = get_fs();
> -	set_fs(get_ds());
> +		len += sg->length;
> +	}
>
>   	if (is_write)
> -		ret = vfs_writev(fd, &iov[0], sgl_nents, &pos);
> +		ret = vfs_bvec_write(fd, bvec, sgl_nents, len, &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));
> +		ret = vfs_bvec_read(fd, bvec, sgl_nents, len, &pos);
>
> -	kfree(iov);
> +	kfree(bvec);
>
>   	if (is_write) {
>   		if (ret < 0 || ret != cmd->data_length) {
>