[PATCH 1/2] uio: add ioctl to uio

[PATCH 1/2] uio: add ioctl to uio

[Guixin Liu]

In TCMU, if backstore holds its own userspace buffer, for read cmd, the
data needs to be copied from userspace buffer to tcmu data area first,
and then needs to be copied from tcmu data area to scsi sgl pages again.

To solve this problem, add ioctl to uio to let userspace backstore can
copy data between scsi sgl pages and its own buffer directly.

Reviewed-by: Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com>
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/uio/uio.c          | 22 ++++++++++++++++++++++
 include/linux/uio_driver.h |  1 +
 2 files changed, 23 insertions(+)

diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index 43afbb7..0fb85a3 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -815,6 +815,24 @@ static int uio_mmap(struct file *filep, struct vm_area_struct *vma)
 	return ret;
 }
 
+long uio_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
+{
+	struct uio_listener *listener = filep->private_data;
+	struct uio_device *idev = listener->dev;
+	long retval = 0;
+
+	mutex_lock(&idev->info_lock);
+	if (!idev->info || !idev->info->ioctl) {
+		retval = -EINVAL;
+		goto out;
+	}
+
+	retval = idev->info->ioctl(idev->info, cmd, arg);
+out:
+	mutex_unlock(&idev->info_lock);
+	return retval;
+}
+
 static const struct file_operations uio_fops = {
 	.owner		= THIS_MODULE,
 	.open		= uio_open,
@@ -825,6 +843,10 @@ static int uio_mmap(struct file *filep, struct vm_area_struct *vma)
 	.poll		= uio_poll,
 	.fasync		= uio_fasync,
 	.llseek		= noop_llseek,
+	.unlocked_ioctl = uio_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl   = uio_ioctl,
+#endif
 };
 
 static int uio_major_init(void)
diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h
index 47c5962..971d172b 100644
--- a/include/linux/uio_driver.h
+++ b/include/linux/uio_driver.h
@@ -109,6 +109,7 @@ struct uio_info {
 	int (*open)(struct uio_info *info, struct inode *inode);
 	int (*release)(struct uio_info *info, struct inode *inode);
 	int (*irqcontrol)(struct uio_info *info, s32 irq_on);
+	long (*ioctl)(struct uio_info *info, unsigned int cmd, unsigned long arg);
 };
 
 extern int __must_check
-- 
1.8.3.1

[PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl

[Guixin Liu]

Currently the data needs to be copied twice between sg, tcmu data area and
userspace buffer if backstore holds its own userspace buffer, then we can
use uio ioctl to copy data between sg and userspace buffer directly to
bypass data area to improve performance.

Use tcm_loop and tcmu(backstore is file) to evaluate performance,
fio job: fio -filename=/dev/sdb  -direct=1 -size=2G -name=1 -thread
-runtime=60 -time_based -rw=randread -numjobs=16 -iodepth=16 -bs=128k

Without this patch:
    READ: bw=3539MiB/s (3711MB/s), 207MiB/s-233MiB/s (217MB/s-244MB/s),
io=104GiB (111GB), run=30001-30002msec

With this patch:
    READ: bw=4420MiB/s (4634MB/s), 274MiB/s-278MiB/s (287MB/s-291MB/s),
io=259GiB (278GB), run=60001-60002msec

Reviewed-by: Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com>
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/target/target_core_user.c     | 171 +++++++++++++++++++++++++++++-----
 include/uapi/linux/target_core_user.h |   9 ++
 2 files changed, 157 insertions(+), 23 deletions(-)

diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index 7b2a89a..afea088 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -122,6 +122,7 @@ struct tcmu_dev {
 #define TCMU_DEV_BIT_BLOCKED 2
 #define TCMU_DEV_BIT_TMR_NOTIFY 3
 #define TCMU_DEV_BIT_PLUGGED 4
+#define TCMU_DEV_BIT_BYPASS_DATA_AREA 5
 	unsigned long flags;
 
 	struct uio_info uio_info;
@@ -642,12 +643,17 @@ static struct tcmu_cmd *tcmu_alloc_cmd(struct se_cmd *se_cmd)
 	tcmu_cmd->se_cmd = se_cmd;
 	tcmu_cmd->tcmu_dev = udev;
 
-	tcmu_cmd_set_block_cnts(tcmu_cmd);
-	tcmu_cmd->dbi = kcalloc(tcmu_cmd->dbi_cnt, sizeof(uint32_t),
-				GFP_NOIO);
-	if (!tcmu_cmd->dbi) {
-		kmem_cache_free(tcmu_cmd_cache, tcmu_cmd);
-		return NULL;
+	if (!test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags)) {
+		tcmu_cmd_set_block_cnts(tcmu_cmd);
+		tcmu_cmd->dbi = kcalloc(tcmu_cmd->dbi_cnt, sizeof(uint32_t),
+					GFP_NOIO);
+		if (!tcmu_cmd->dbi) {
+			kmem_cache_free(tcmu_cmd_cache, tcmu_cmd);
+			return NULL;
+		}
+	} else {
+		tcmu_cmd->dbi_cnt = 0;
+		tcmu_cmd->dbi = NULL;
 	}
 
 	return tcmu_cmd;
@@ -1093,16 +1099,18 @@ static int queue_cmd_ring(struct tcmu_cmd *tcmu_cmd, sense_reason_t *scsi_err)
 	tcmu_cmd_reset_dbi_cur(tcmu_cmd);
 	iov = &entry->req.iov[0];
 
-	if (se_cmd->data_direction == DMA_TO_DEVICE ||
-	    se_cmd->se_cmd_flags & SCF_BIDI)
-		scatter_data_area(udev, tcmu_cmd, &iov);
-	else
-		tcmu_setup_iovs(udev, tcmu_cmd, &iov, se_cmd->data_length);
-
+	if (!test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags)) {
+		if (se_cmd->data_direction == DMA_TO_DEVICE ||
+		se_cmd->se_cmd_flags & SCF_BIDI)
+			scatter_data_area(udev, tcmu_cmd, &iov);
+		else
+			tcmu_setup_iovs(udev, tcmu_cmd, &iov, se_cmd->data_length);
+	}
 	entry->req.iov_cnt = iov_cnt - iov_bidi_cnt;
 
 	/* Handle BIDI commands */
-	if (se_cmd->se_cmd_flags & SCF_BIDI) {
+	if ((se_cmd->se_cmd_flags & SCF_BIDI)
+		&& !test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags)) {
 		iov++;
 		tcmu_setup_iovs(udev, tcmu_cmd, &iov, tcmu_cmd->data_len_bidi);
 		entry->req.iov_bidi_cnt = iov_bidi_cnt;
@@ -1366,16 +1374,19 @@ static bool tcmu_handle_completion(struct tcmu_cmd *cmd,
 		else
 			se_cmd->se_cmd_flags |= SCF_TREAT_READ_AS_NORMAL;
 	}
-	if (se_cmd->se_cmd_flags & SCF_BIDI) {
-		/* Get Data-In buffer before clean up */
-		gather_data_area(udev, cmd, true, read_len);
-	} else if (se_cmd->data_direction == DMA_FROM_DEVICE) {
-		gather_data_area(udev, cmd, false, read_len);
-	} else if (se_cmd->data_direction == DMA_TO_DEVICE) {
-		/* TODO: */
-	} else if (se_cmd->data_direction != DMA_NONE) {
-		pr_warn("TCMU: data direction was %d!\n",
-			se_cmd->data_direction);
+
+	if (!test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags)) {
+		if (se_cmd->se_cmd_flags & SCF_BIDI) {
+			/* Get Data-In buffer before clean up */
+			gather_data_area(udev, cmd, true, read_len);
+		} else if (se_cmd->data_direction == DMA_FROM_DEVICE) {
+			gather_data_area(udev, cmd, false, read_len);
+		} else if (se_cmd->data_direction == DMA_TO_DEVICE) {
+			/* TODO: */
+		} else if (se_cmd->data_direction != DMA_NONE) {
+			pr_warn("TCMU: data direction was %d!\n",
+				se_cmd->data_direction);
+		}
 	}
 
 done:
@@ -1973,6 +1984,84 @@ static int tcmu_release(struct uio_info *info, struct inode *inode)
 	return 0;
 }
 
+long tcmu_ioctl_copy_between_sgl_and_iovec(struct tcmu_cmd *tcmu_cmd,
+			struct iovec __user *uiovec,
+			unsigned long vcnt,
+			bool is_copy_to_sgl)
+{
+	struct iovec iovstack[UIO_FASTIOV];
+	struct iovec *iov = iovstack;
+	struct iov_iter iter;
+	ssize_t ret;
+	struct se_cmd *se_cmd = tcmu_cmd->se_cmd;
+	struct scatterlist *data_sg, *sg;
+	int i;
+	unsigned int data_nents;
+	long copy_ret = 0;
+
+	if (se_cmd->se_cmd_flags & SCF_BIDI) {
+		data_sg = se_cmd->t_bidi_data_sg;
+		data_nents = se_cmd->t_bidi_data_nents;
+	} else {
+		data_sg = se_cmd->t_data_sg;
+		data_nents = se_cmd->t_data_nents;
+	}
+
+	ret = import_iovec(READ, uiovec, vcnt, ARRAY_SIZE(iovstack), &iov, &iter);
+	if (ret < 0) {
+		pr_err("import iovec failed.\n");
+		return -EFAULT;
+	}
+
+	for_each_sg(data_sg, sg, data_nents, i) {
+		if (is_copy_to_sgl)
+			ret = copy_page_from_iter(sg_page(sg), sg->offset, sg->length, &iter);
+		else
+			ret = copy_page_to_iter(sg_page(sg), sg->offset, sg->length, &iter);
+		if (ret < 0) {
+			pr_err("copy failed.\n");
+			copy_ret = -EFAULT;
+			break;
+		}
+	}
+	kfree(iov);
+	return copy_ret;
+}
+
+long tcmu_ioctl(struct uio_info *info, unsigned int cmd, unsigned long arg)
+{
+	struct tcmu_dev *udev = container_of(info, struct tcmu_dev, uio_info);
+	struct tcmu_data_xfer __user *uxfer = (struct tcmu_data_xfer __user *)arg;
+	struct tcmu_data_xfer xfer;
+	struct tcmu_cmd *tcmu_cmd;
+
+	if (!test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags))
+		return -EINVAL;
+
+	if (copy_from_user(&xfer, uxfer, sizeof(xfer)))
+		return -EFAULT;
+
+	tcmu_cmd = xa_load(&udev->commands, xfer.cmd_id);
+	if (!tcmu_cmd) {
+		set_bit(TCMU_DEV_BIT_BROKEN, &udev->flags);
+		return -EFAULT;
+	}
+
+	if (test_bit(TCMU_CMD_BIT_EXPIRED, &tcmu_cmd->flags))
+		return -EFAULT;
+
+	switch (cmd) {
+	case TCMU_IOCTL_CMD_COPY_TO_SGL:
+		return tcmu_ioctl_copy_between_sgl_and_iovec(tcmu_cmd, xfer.iovec,
+							     xfer.iov_cnt, true);
+	case TCMU_IOCTL_CMD_COPY_FROM_SGL:
+		return tcmu_ioctl_copy_between_sgl_and_iovec(tcmu_cmd, xfer.iovec,
+							     xfer.iov_cnt, false);
+	default:
+		return -EINVAL;
+	}
+}
+
 static int tcmu_init_genl_cmd_reply(struct tcmu_dev *udev, int cmd)
 {
 	struct tcmu_nl_cmd *nl_cmd = &udev->curr_nl_cmd;
@@ -2230,6 +2319,7 @@ static int tcmu_configure_device(struct se_device *dev)
 	info->mmap = tcmu_mmap;
 	info->open = tcmu_open;
 	info->release = tcmu_release;
+	info->ioctl = tcmu_ioctl;
 
 	ret = uio_register_device(tcmu_root_device, info);
 	if (ret)
@@ -2838,6 +2928,40 @@ static ssize_t tcmu_nl_reply_supported_store(struct config_item *item,
 }
 CONFIGFS_ATTR(tcmu_, nl_reply_supported);
 
+static ssize_t tcmu_bypass_data_area_show(struct config_item *item, char *page)
+{
+	struct se_dev_attrib *da = container_of(to_config_group(item),
+						struct se_dev_attrib, da_group);
+	struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+
+	if (test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags))
+		return snprintf(page, PAGE_SIZE, "%s\n", "true");
+	else
+		return snprintf(page, PAGE_SIZE, "%s\n", "false");
+}
+
+static ssize_t tcmu_bypass_data_area_store(struct config_item *item, const char *page,
+					    size_t count)
+{
+	struct se_dev_attrib *da = container_of(to_config_group(item),
+						struct se_dev_attrib, da_group);
+	struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+	bool bypass_data_area;
+	int ret;
+
+	ret = strtobool(page, &bypass_data_area);
+	if (ret < 0)
+		return ret;
+
+	if (bypass_data_area)
+		set_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags);
+	else
+		clear_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags);
+
+	return count;
+}
+CONFIGFS_ATTR(tcmu_, bypass_data_area);
+
 static ssize_t tcmu_emulate_write_cache_show(struct config_item *item,
 					     char *page)
 {
@@ -3069,6 +3193,7 @@ static ssize_t tcmu_free_kept_buf_store(struct config_item *item, const char *pa
 	&tcmu_attr_emulate_write_cache,
 	&tcmu_attr_tmr_notification,
 	&tcmu_attr_nl_reply_supported,
+	&tcmu_attr_bypass_data_area,
 	NULL,
 };
 
diff --git a/include/uapi/linux/target_core_user.h b/include/uapi/linux/target_core_user.h
index 27ace51..c02a45e 100644
--- a/include/uapi/linux/target_core_user.h
+++ b/include/uapi/linux/target_core_user.h
@@ -185,4 +185,13 @@ enum tcmu_genl_attr {
 };
 #define TCMU_ATTR_MAX (__TCMU_ATTR_MAX - 1)
 
+struct tcmu_data_xfer {
+	unsigned short cmd_id;
+	unsigned long iov_cnt;
+	struct iovec __user *iovec;
+};
+
+#define TCMU_IOCTL_CMD_COPY_TO_SGL      _IOW('T', 0xe0, struct tcmu_data_xfer)
+#define TCMU_IOCTL_CMD_COPY_FROM_SGL    _IOW('T', 0xe1, struct tcmu_data_xfer)
+
 #endif
-- 
1.8.3.1

Re: [PATCH 1/2] uio: add ioctl to uio

[Greg KH]

On Thu, Feb 17, 2022 at 10:29:21AM +0800, Guixin Liu wrote:
> In TCMU, if backstore holds its own userspace buffer, for read cmd, the
> data needs to be copied from userspace buffer to tcmu data area first,
> and then needs to be copied from tcmu data area to scsi sgl pages again.
> 
> To solve this problem, add ioctl to uio to let userspace backstore can
> copy data between scsi sgl pages and its own buffer directly.
> 
> Reviewed-by: Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com>
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> ---
>  drivers/uio/uio.c          | 22 ++++++++++++++++++++++
>  include/linux/uio_driver.h |  1 +

No, sorry, thie uio driver will not be adding ioctls to them.  If you
need an ioctl, then you should not be using the UIO api but rather use a
custom character driver instead.

thanks,

greg k-h

Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl

[kernel test robot]

Hi Guixin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on mkp-scsi/for-next linux/master linus/master v5.17-rc4 next-20220216]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Guixin-Liu/uio-add-ioctl-to-uio/20220217-103120
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git e6cb9c167eeb8f90ab924666c573e69e85e700a0
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220217/202202171452.MblxpwJx-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/c604d03c2be8ca4b3533bb151bcd2d10379debff
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Guixin-Liu/uio-add-ioctl-to-uio/20220217-103120
        git checkout c604d03c2be8ca4b3533bb151bcd2d10379debff
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash drivers/target/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/target/target_core_user.c:1987:6: warning: no previous prototype for 'tcmu_ioctl_copy_between_sgl_and_iovec' [-Wmissing-prototypes]
    1987 | long tcmu_ioctl_copy_between_sgl_and_iovec(struct tcmu_cmd *tcmu_cmd,
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/target/target_core_user.c:2031:6: warning: no previous prototype for 'tcmu_ioctl' [-Wmissing-prototypes]
    2031 | long tcmu_ioctl(struct uio_info *info, unsigned int cmd, unsigned long arg)
         |      ^~~~~~~~~~


vim +/tcmu_ioctl_copy_between_sgl_and_iovec +1987 drivers/target/target_core_user.c

  1986	
> 1987	long tcmu_ioctl_copy_between_sgl_and_iovec(struct tcmu_cmd *tcmu_cmd,
  1988				struct iovec __user *uiovec,
  1989				unsigned long vcnt,
  1990				bool is_copy_to_sgl)
  1991	{
  1992		struct iovec iovstack[UIO_FASTIOV];
  1993		struct iovec *iov = iovstack;
  1994		struct iov_iter iter;
  1995		ssize_t ret;
  1996		struct se_cmd *se_cmd = tcmu_cmd->se_cmd;
  1997		struct scatterlist *data_sg, *sg;
  1998		int i;
  1999		unsigned int data_nents;
  2000		long copy_ret = 0;
  2001	
  2002		if (se_cmd->se_cmd_flags & SCF_BIDI) {
  2003			data_sg = se_cmd->t_bidi_data_sg;
  2004			data_nents = se_cmd->t_bidi_data_nents;
  2005		} else {
  2006			data_sg = se_cmd->t_data_sg;
  2007			data_nents = se_cmd->t_data_nents;
  2008		}
  2009	
  2010		ret = import_iovec(READ, uiovec, vcnt, ARRAY_SIZE(iovstack), &iov, &iter);
  2011		if (ret < 0) {
  2012			pr_err("import iovec failed.\n");
  2013			return -EFAULT;
  2014		}
  2015	
  2016		for_each_sg(data_sg, sg, data_nents, i) {
  2017			if (is_copy_to_sgl)
  2018				ret = copy_page_from_iter(sg_page(sg), sg->offset, sg->length, &iter);
  2019			else
  2020				ret = copy_page_to_iter(sg_page(sg), sg->offset, sg->length, &iter);
  2021			if (ret < 0) {
  2022				pr_err("copy failed.\n");
  2023				copy_ret = -EFAULT;
  2024				break;
  2025			}
  2026		}
  2027		kfree(iov);
  2028		return copy_ret;
  2029	}
  2030	
> 2031	long tcmu_ioctl(struct uio_info *info, unsigned int cmd, unsigned long arg)
  2032	{
  2033		struct tcmu_dev *udev = container_of(info, struct tcmu_dev, uio_info);
  2034		struct tcmu_data_xfer __user *uxfer = (struct tcmu_data_xfer __user *)arg;
  2035		struct tcmu_data_xfer xfer;
  2036		struct tcmu_cmd *tcmu_cmd;
  2037	
  2038		if (!test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags))
  2039			return -EINVAL;
  2040	
  2041		if (copy_from_user(&xfer, uxfer, sizeof(xfer)))
  2042			return -EFAULT;
  2043	
  2044		tcmu_cmd = xa_load(&udev->commands, xfer.cmd_id);
  2045		if (!tcmu_cmd) {
  2046			set_bit(TCMU_DEV_BIT_BROKEN, &udev->flags);
  2047			return -EFAULT;
  2048		}
  2049	
  2050		if (test_bit(TCMU_CMD_BIT_EXPIRED, &tcmu_cmd->flags))
  2051			return -EFAULT;
  2052	
  2053		switch (cmd) {
  2054		case TCMU_IOCTL_CMD_COPY_TO_SGL:
  2055			return tcmu_ioctl_copy_between_sgl_and_iovec(tcmu_cmd, xfer.iovec,
  2056								     xfer.iov_cnt, true);
  2057		case TCMU_IOCTL_CMD_COPY_FROM_SGL:
  2058			return tcmu_ioctl_copy_between_sgl_and_iovec(tcmu_cmd, xfer.iovec,
  2059								     xfer.iov_cnt, false);
  2060		default:
  2061			return -EINVAL;
  2062		}
  2063	}
  2064	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH 1/2] uio: add ioctl to uio

[kernel test robot]

Hi Guixin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on mkp-scsi/for-next linux/master linus/master v5.17-rc4 next-20220216]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Guixin-Liu/uio-add-ioctl-to-uio/20220217-103120
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git e6cb9c167eeb8f90ab924666c573e69e85e700a0
config: arc-randconfig-r035-20220217 (https://download.01.org/0day-ci/archive/20220217/202202171444.RSO1up2n-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/8786f129af888d844bc38ed78db7a6788e45655c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Guixin-Liu/uio-add-ioctl-to-uio/20220217-103120
        git checkout 8786f129af888d844bc38ed78db7a6788e45655c
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash drivers/uio/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/uio/uio.c:818:6: warning: no previous prototype for 'uio_ioctl' [-Wmissing-prototypes]
     818 | long uio_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
         |      ^~~~~~~~~


vim +/uio_ioctl +818 drivers/uio/uio.c

   817	
 > 818	long uio_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
   819	{
   820		struct uio_listener *listener = filep->private_data;
   821		struct uio_device *idev = listener->dev;
   822		long retval = 0;
   823	
   824		mutex_lock(&idev->info_lock);
   825		if (!idev->info || !idev->info->ioctl) {
   826			retval = -EINVAL;
   827			goto out;
   828		}
   829	
   830		retval = idev->info->ioctl(idev->info, cmd, arg);
   831	out:
   832		mutex_unlock(&idev->info_lock);
   833		return retval;
   834	}
   835	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl

[kernel test robot]

Hi Guixin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on mkp-scsi/for-next linux/master linus/master v5.17-rc4 next-20220216]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Guixin-Liu/uio-add-ioctl-to-uio/20220217-103120
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git e6cb9c167eeb8f90ab924666c573e69e85e700a0
config: riscv-randconfig-r042-20220217 (https://download.01.org/0day-ci/archive/20220217/202202171541.grdjAOIT-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 0bad7cb56526f2572c74449fcf97c1fcda42b41d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/c604d03c2be8ca4b3533bb151bcd2d10379debff
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Guixin-Liu/uio-add-ioctl-to-uio/20220217-103120
        git checkout c604d03c2be8ca4b3533bb151bcd2d10379debff
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/target/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/target/target_core_user.c:1987:6: warning: no previous prototype for function 'tcmu_ioctl_copy_between_sgl_and_iovec' [-Wmissing-prototypes]
   long tcmu_ioctl_copy_between_sgl_and_iovec(struct tcmu_cmd *tcmu_cmd,
        ^
   drivers/target/target_core_user.c:1987:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   long tcmu_ioctl_copy_between_sgl_and_iovec(struct tcmu_cmd *tcmu_cmd,
   ^
   static 
>> drivers/target/target_core_user.c:2031:6: warning: no previous prototype for function 'tcmu_ioctl' [-Wmissing-prototypes]
   long tcmu_ioctl(struct uio_info *info, unsigned int cmd, unsigned long arg)
        ^
   drivers/target/target_core_user.c:2031:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   long tcmu_ioctl(struct uio_info *info, unsigned int cmd, unsigned long arg)
   ^
   static 
   2 warnings generated.


vim +/tcmu_ioctl_copy_between_sgl_and_iovec +1987 drivers/target/target_core_user.c

  1986	
> 1987	long tcmu_ioctl_copy_between_sgl_and_iovec(struct tcmu_cmd *tcmu_cmd,
  1988				struct iovec __user *uiovec,
  1989				unsigned long vcnt,
  1990				bool is_copy_to_sgl)
  1991	{
  1992		struct iovec iovstack[UIO_FASTIOV];
  1993		struct iovec *iov = iovstack;
  1994		struct iov_iter iter;
  1995		ssize_t ret;
  1996		struct se_cmd *se_cmd = tcmu_cmd->se_cmd;
  1997		struct scatterlist *data_sg, *sg;
  1998		int i;
  1999		unsigned int data_nents;
  2000		long copy_ret = 0;
  2001	
  2002		if (se_cmd->se_cmd_flags & SCF_BIDI) {
  2003			data_sg = se_cmd->t_bidi_data_sg;
  2004			data_nents = se_cmd->t_bidi_data_nents;
  2005		} else {
  2006			data_sg = se_cmd->t_data_sg;
  2007			data_nents = se_cmd->t_data_nents;
  2008		}
  2009	
  2010		ret = import_iovec(READ, uiovec, vcnt, ARRAY_SIZE(iovstack), &iov, &iter);
  2011		if (ret < 0) {
  2012			pr_err("import iovec failed.\n");
  2013			return -EFAULT;
  2014		}
  2015	
  2016		for_each_sg(data_sg, sg, data_nents, i) {
  2017			if (is_copy_to_sgl)
  2018				ret = copy_page_from_iter(sg_page(sg), sg->offset, sg->length, &iter);
  2019			else
  2020				ret = copy_page_to_iter(sg_page(sg), sg->offset, sg->length, &iter);
  2021			if (ret < 0) {
  2022				pr_err("copy failed.\n");
  2023				copy_ret = -EFAULT;
  2024				break;
  2025			}
  2026		}
  2027		kfree(iov);
  2028		return copy_ret;
  2029	}
  2030	
> 2031	long tcmu_ioctl(struct uio_info *info, unsigned int cmd, unsigned long arg)
  2032	{
  2033		struct tcmu_dev *udev = container_of(info, struct tcmu_dev, uio_info);
  2034		struct tcmu_data_xfer __user *uxfer = (struct tcmu_data_xfer __user *)arg;
  2035		struct tcmu_data_xfer xfer;
  2036		struct tcmu_cmd *tcmu_cmd;
  2037	
  2038		if (!test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags))
  2039			return -EINVAL;
  2040	
  2041		if (copy_from_user(&xfer, uxfer, sizeof(xfer)))
  2042			return -EFAULT;
  2043	
  2044		tcmu_cmd = xa_load(&udev->commands, xfer.cmd_id);
  2045		if (!tcmu_cmd) {
  2046			set_bit(TCMU_DEV_BIT_BROKEN, &udev->flags);
  2047			return -EFAULT;
  2048		}
  2049	
  2050		if (test_bit(TCMU_CMD_BIT_EXPIRED, &tcmu_cmd->flags))
  2051			return -EFAULT;
  2052	
  2053		switch (cmd) {
  2054		case TCMU_IOCTL_CMD_COPY_TO_SGL:
  2055			return tcmu_ioctl_copy_between_sgl_and_iovec(tcmu_cmd, xfer.iovec,
  2056								     xfer.iov_cnt, true);
  2057		case TCMU_IOCTL_CMD_COPY_FROM_SGL:
  2058			return tcmu_ioctl_copy_between_sgl_and_iovec(tcmu_cmd, xfer.iovec,
  2059								     xfer.iov_cnt, false);
  2060		default:
  2061			return -EINVAL;
  2062		}
  2063	}
  2064	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl

[Greg KH]

On Thu, Feb 17, 2022 at 10:29:22AM +0800, Guixin Liu wrote:
> --- a/include/uapi/linux/target_core_user.h
> +++ b/include/uapi/linux/target_core_user.h
> @@ -185,4 +185,13 @@ enum tcmu_genl_attr {
>  };
>  #define TCMU_ATTR_MAX (__TCMU_ATTR_MAX - 1)
>  
> +struct tcmu_data_xfer {
> +	unsigned short cmd_id;
> +	unsigned long iov_cnt;
> +	struct iovec __user *iovec;
> +};

That is no way to define a structure that crosses the user/kernel
boundry, it just will not work at all, even if we wanted it to :(

sorry,

greg k-h

[RFC PATCH] scsi:target:tcmu: tcmu_ioctl_copy_between_sgl_and_iovec() can be static

[kernel test robot]

drivers/target/target_core_user.c:1987:6: warning: symbol 'tcmu_ioctl_copy_between_sgl_and_iovec' was not declared. Should it be static?
drivers/target/target_core_user.c:2031:6: warning: symbol 'tcmu_ioctl' was not declared. Should it be static?

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: kernel test robot <lkp@intel.com>
---
 drivers/target/target_core_user.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index afea088f24862b..ea1af59e03deff 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1984,7 +1984,7 @@ static int tcmu_release(struct uio_info *info, struct inode *inode)
 	return 0;
 }
 
-long tcmu_ioctl_copy_between_sgl_and_iovec(struct tcmu_cmd *tcmu_cmd,
+static long tcmu_ioctl_copy_between_sgl_and_iovec(struct tcmu_cmd *tcmu_cmd,
 			struct iovec __user *uiovec,
 			unsigned long vcnt,
 			bool is_copy_to_sgl)
@@ -2028,7 +2028,7 @@ long tcmu_ioctl_copy_between_sgl_and_iovec(struct tcmu_cmd *tcmu_cmd,
 	return copy_ret;
 }
 
-long tcmu_ioctl(struct uio_info *info, unsigned int cmd, unsigned long arg)
+static long tcmu_ioctl(struct uio_info *info, unsigned int cmd, unsigned long arg)
 {
 	struct tcmu_dev *udev = container_of(info, struct tcmu_dev, uio_info);
 	struct tcmu_data_xfer __user *uxfer = (struct tcmu_data_xfer __user *)arg;

Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl

[kernel test robot]

Hi Guixin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on mkp-scsi/for-next linux/master linus/master v5.17-rc4 next-20220216]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Guixin-Liu/uio-add-ioctl-to-uio/20220217-103120
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git e6cb9c167eeb8f90ab924666c573e69e85e700a0
config: i386-randconfig-s002 (https://download.01.org/0day-ci/archive/20220217/202202171918.s8P0xa1Y-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/c604d03c2be8ca4b3533bb151bcd2d10379debff
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Guixin-Liu/uio-add-ioctl-to-uio/20220217-103120
        git checkout c604d03c2be8ca4b3533bb151bcd2d10379debff
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/target/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/target/target_core_user.c:1987:6: sparse: sparse: symbol 'tcmu_ioctl_copy_between_sgl_and_iovec' was not declared. Should it be static?
>> drivers/target/target_core_user.c:2031:6: sparse: sparse: symbol 'tcmu_ioctl' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl

[Xiaoguang Wang]

hi,

> On Thu, Feb 17, 2022 at 10:29:22AM +0800, Guixin Liu wrote:
>> --- a/include/uapi/linux/target_core_user.h
>> +++ b/include/uapi/linux/target_core_user.h
>> @@ -185,4 +185,13 @@ enum tcmu_genl_attr {
>>   };
>>   #define TCMU_ATTR_MAX (__TCMU_ATTR_MAX - 1)
>>   
>> +struct tcmu_data_xfer {
>> +	unsigned short cmd_id;
>> +	unsigned long iov_cnt;
>> +	struct iovec __user *iovec;
>> +};
> That is no way to define a structure that crosses the user/kernel
> boundry, it just will not work at all, even if we wanted it to :(
Sorry, I don't quite understand your comments well, can you explain more 
why this structure
does not work, except that "unsigned short" or "unsigned long" here 
isn't correct, should use
u16 or u32.
Syscalls, such as readv/preadv also pass a struct iovec from user to 
kernel, thanks.


Regards,
Xiaoguang Wang
>
> sorry,
>
> greg k-h

Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl

[Greg KH]

On Thu, Feb 17, 2022 at 07:13:07PM +0800, Xiaoguang Wang wrote:
> hi,
> 
> > On Thu, Feb 17, 2022 at 10:29:22AM +0800, Guixin Liu wrote:
> > > --- a/include/uapi/linux/target_core_user.h
> > > +++ b/include/uapi/linux/target_core_user.h
> > > @@ -185,4 +185,13 @@ enum tcmu_genl_attr {
> > >   };
> > >   #define TCMU_ATTR_MAX (__TCMU_ATTR_MAX - 1)
> > > +struct tcmu_data_xfer {
> > > +	unsigned short cmd_id;
> > > +	unsigned long iov_cnt;
> > > +	struct iovec __user *iovec;
> > > +};
> > That is no way to define a structure that crosses the user/kernel
> > boundry, it just will not work at all, even if we wanted it to :(
> Sorry, I don't quite understand your comments well, can you explain more why
> this structure
> does not work, except that "unsigned short" or "unsigned long" here isn't
> correct, should use
> u16 or u32.

__u32 and __u16 must be used for structures like this.  Also it's
unaligned and has a hole in it, and will cause problems with compilers.
You didn't try this on a 32/64bit system either :(

thanks,

greg k-h

Re: [PATCH 1/2] uio: add ioctl to uio

[Greg KH]

On Thu, Feb 17, 2022 at 08:15:29PM +0800, Xiaoguang Wang wrote:
> hi,
> 
> > On Thu, Feb 17, 2022 at 10:29:21AM +0800, Guixin Liu wrote:
> > > In TCMU, if backstore holds its own userspace buffer, for read cmd, the
> > > data needs to be copied from userspace buffer to tcmu data area first,
> > > and then needs to be copied from tcmu data area to scsi sgl pages again.
> > > 
> > > To solve this problem, add ioctl to uio to let userspace backstore can
> > > copy data between scsi sgl pages and its own buffer directly.
> > > 
> > > Reviewed-by: Xiaoguang Wang<xiaoguang.wang@linux.alibaba.com>
> > > Signed-off-by: Guixin Liu<kanie@linux.alibaba.com>
> > > ---
> > >   drivers/uio/uio.c          | 22 ++++++++++++++++++++++
> > >   include/linux/uio_driver.h |  1 +
> > No, sorry, thie uio driver will not be adding ioctls to them.  If you
> > need an ioctl, then you should not be using the UIO api but rather use a
> > custom character driver instead.
> 
> I found that early in 2015, there was developer trying to add ioctl interface
> to uio framework:https://lore.kernel.org/lkml/20151005080149.GB1747@kroah.com/

See, I rejected your ioctl for the very same reasons :)

It's good that I was consistent...

> Some of my customers use tcm_loop & tcmu to simulate block devices, it's tcmu
> driver that uses uio framework. There maybe more extra work if we tries to replace
> uio with a new character driver.

Why is tcmu using uio at all?

> Currently tcmu has performance bottleneck, Guixin's patch uses ioctl interface
> to bypass tcmu data area. I also have implemented a tcmu zero-copy feature that
> allows tcmu driver map io request sgl's pages to user space, which uses ioctl
> to do this mapping work, similar to network getsockopt(TCP_ZEROCOPY_RECEIVE).
> 
> I also understand your concerns about ioctl interface. Except that replacing
> uio with a new character driver in tcmu, are there any less complicated methods
> to complete our needs? Thanks.

I do not know what your needs are, nor what tcmu is, nor why you would
want to simulate a block device to userspace like this through the uio
api, sorry.

good luck!

greg k-h

Re: [PATCH 1/2] uio: add ioctl to uio

[Xiaoguang Wang]

hi,

> On Thu, Feb 17, 2022 at 10:29:21AM +0800, Guixin Liu wrote:
>> In TCMU, if backstore holds its own userspace buffer, for read cmd, the
>> data needs to be copied from userspace buffer to tcmu data area first,
>> and then needs to be copied from tcmu data area to scsi sgl pages again.
>>
>> To solve this problem, add ioctl to uio to let userspace backstore can
>> copy data between scsi sgl pages and its own buffer directly.
>>
>> Reviewed-by: Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com>
>> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
>> ---
>>   drivers/uio/uio.c          | 22 ++++++++++++++++++++++
>>   include/linux/uio_driver.h |  1 +
> No, sorry, thie uio driver will not be adding ioctls to them.  If you
> need an ioctl, then you should not be using the UIO api but rather use a
> custom character driver instead.
I found that early in 2015, there was developer trying to add ioctl 
interface
to uio framework: 
https://lore.kernel.org/lkml/20151005080149.GB1747@kroah.com/

Some of my customers use tcm_loop & tcmu to simulate block devices, it's 
tcmu
driver that uses uio framework. There maybe more extra work if we tries 
to replace
uio with a new character driver.

Currently tcmu has performance bottleneck, Guixin's patch uses ioctl 
interface
to bypass tcmu data area. I also have implemented a tcmu zero-copy 
feature that
allows tcmu driver map io request sgl's pages to user space, which uses 
ioctl
to do this mapping work, similar to network 
getsockopt(TCP_ZEROCOPY_RECEIVE).

I also understand your concerns about ioctl interface. Except that replacing
uio with a new character driver in tcmu, are there any less complicated 
methods
to complete our needs? Thanks.

Regards,
Xiaoguang Wang
>
> thanks,
>
> greg k-h

Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl

[Bodo Stroesser]

Liu,

generally I like ideas to speed up tcmu.

OTOH, since Andy Grover implemented tcmu based on uio device, we are
restricted to what uio offers. With today's knowledge I think we would
not use the uio device in tcmu again, but switching away from uio now
would break existing userspace SW.

Before we start thinking how the same performance gain could be reached
without a change in uio device, please let me know why you use file
backend on top of tcmu instead of target_core_file kernel module?
Wouldn't target_core_file be even faster for your purpose?

Bodo



On 17.02.22 03:29, Guixin Liu wrote:
> Currently the data needs to be copied twice between sg, tcmu data area and
> userspace buffer if backstore holds its own userspace buffer, then we can
> use uio ioctl to copy data between sg and userspace buffer directly to
> bypass data area to improve performance.
> 
> Use tcm_loop and tcmu(backstore is file) to evaluate performance,
> fio job: fio -filename=/dev/sdb  -direct=1 -size=2G -name=1 -thread
> -runtime=60 -time_based -rw=randread -numjobs=16 -iodepth=16 -bs=128k
> 
> Without this patch:
>      READ: bw=3539MiB/s (3711MB/s), 207MiB/s-233MiB/s (217MB/s-244MB/s),
> io=104GiB (111GB), run=30001-30002msec
> 
> With this patch:
>      READ: bw=4420MiB/s (4634MB/s), 274MiB/s-278MiB/s (287MB/s-291MB/s),
> io=259GiB (278GB), run=60001-60002msec
> 
> Reviewed-by: Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com>
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> ---
>   drivers/target/target_core_user.c     | 171 +++++++++++++++++++++++++++++-----
>   include/uapi/linux/target_core_user.h |   9 ++
>   2 files changed, 157 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
> index 7b2a89a..afea088 100644
> --- a/drivers/target/target_core_user.c
> +++ b/drivers/target/target_core_user.c
> @@ -122,6 +122,7 @@ struct tcmu_dev {
>   #define TCMU_DEV_BIT_BLOCKED 2
>   #define TCMU_DEV_BIT_TMR_NOTIFY 3
>   #define TCMU_DEV_BIT_PLUGGED 4
> +#define TCMU_DEV_BIT_BYPASS_DATA_AREA 5
>   	unsigned long flags;
>   
>   	struct uio_info uio_info;
> @@ -642,12 +643,17 @@ static struct tcmu_cmd *tcmu_alloc_cmd(struct se_cmd *se_cmd)
>   	tcmu_cmd->se_cmd = se_cmd;
>   	tcmu_cmd->tcmu_dev = udev;
>   
> -	tcmu_cmd_set_block_cnts(tcmu_cmd);
> -	tcmu_cmd->dbi = kcalloc(tcmu_cmd->dbi_cnt, sizeof(uint32_t),
> -				GFP_NOIO);
> -	if (!tcmu_cmd->dbi) {
> -		kmem_cache_free(tcmu_cmd_cache, tcmu_cmd);
> -		return NULL;
> +	if (!test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags)) {
> +		tcmu_cmd_set_block_cnts(tcmu_cmd);
> +		tcmu_cmd->dbi = kcalloc(tcmu_cmd->dbi_cnt, sizeof(uint32_t),
> +					GFP_NOIO);
> +		if (!tcmu_cmd->dbi) {
> +			kmem_cache_free(tcmu_cmd_cache, tcmu_cmd);
> +			return NULL;
> +		}
> +	} else {
> +		tcmu_cmd->dbi_cnt = 0;
> +		tcmu_cmd->dbi = NULL;
>   	}
>   
>   	return tcmu_cmd;
> @@ -1093,16 +1099,18 @@ static int queue_cmd_ring(struct tcmu_cmd *tcmu_cmd, sense_reason_t *scsi_err)
>   	tcmu_cmd_reset_dbi_cur(tcmu_cmd);
>   	iov = &entry->req.iov[0];
>   
> -	if (se_cmd->data_direction == DMA_TO_DEVICE ||
> -	    se_cmd->se_cmd_flags & SCF_BIDI)
> -		scatter_data_area(udev, tcmu_cmd, &iov);
> -	else
> -		tcmu_setup_iovs(udev, tcmu_cmd, &iov, se_cmd->data_length);
> -
> +	if (!test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags)) {
> +		if (se_cmd->data_direction == DMA_TO_DEVICE ||
> +		se_cmd->se_cmd_flags & SCF_BIDI)
> +			scatter_data_area(udev, tcmu_cmd, &iov);
> +		else
> +			tcmu_setup_iovs(udev, tcmu_cmd, &iov, se_cmd->data_length);
> +	}
>   	entry->req.iov_cnt = iov_cnt - iov_bidi_cnt;
>   
>   	/* Handle BIDI commands */
> -	if (se_cmd->se_cmd_flags & SCF_BIDI) {
> +	if ((se_cmd->se_cmd_flags & SCF_BIDI)
> +		&& !test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags)) {
>   		iov++;
>   		tcmu_setup_iovs(udev, tcmu_cmd, &iov, tcmu_cmd->data_len_bidi);
>   		entry->req.iov_bidi_cnt = iov_bidi_cnt;
> @@ -1366,16 +1374,19 @@ static bool tcmu_handle_completion(struct tcmu_cmd *cmd,
>   		else
>   			se_cmd->se_cmd_flags |= SCF_TREAT_READ_AS_NORMAL;
>   	}
> -	if (se_cmd->se_cmd_flags & SCF_BIDI) {
> -		/* Get Data-In buffer before clean up */
> -		gather_data_area(udev, cmd, true, read_len);
> -	} else if (se_cmd->data_direction == DMA_FROM_DEVICE) {
> -		gather_data_area(udev, cmd, false, read_len);
> -	} else if (se_cmd->data_direction == DMA_TO_DEVICE) {
> -		/* TODO: */
> -	} else if (se_cmd->data_direction != DMA_NONE) {
> -		pr_warn("TCMU: data direction was %d!\n",
> -			se_cmd->data_direction);
> +
> +	if (!test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags)) {
> +		if (se_cmd->se_cmd_flags & SCF_BIDI) {
> +			/* Get Data-In buffer before clean up */
> +			gather_data_area(udev, cmd, true, read_len);
> +		} else if (se_cmd->data_direction == DMA_FROM_DEVICE) {
> +			gather_data_area(udev, cmd, false, read_len);
> +		} else if (se_cmd->data_direction == DMA_TO_DEVICE) {
> +			/* TODO: */
> +		} else if (se_cmd->data_direction != DMA_NONE) {
> +			pr_warn("TCMU: data direction was %d!\n",
> +				se_cmd->data_direction);
> +		}
>   	}
>   
>   done:
> @@ -1973,6 +1984,84 @@ static int tcmu_release(struct uio_info *info, struct inode *inode)
>   	return 0;
>   }
>   
> +long tcmu_ioctl_copy_between_sgl_and_iovec(struct tcmu_cmd *tcmu_cmd,
> +			struct iovec __user *uiovec,
> +			unsigned long vcnt,
> +			bool is_copy_to_sgl)
> +{
> +	struct iovec iovstack[UIO_FASTIOV];
> +	struct iovec *iov = iovstack;
> +	struct iov_iter iter;
> +	ssize_t ret;
> +	struct se_cmd *se_cmd = tcmu_cmd->se_cmd;
> +	struct scatterlist *data_sg, *sg;
> +	int i;
> +	unsigned int data_nents;
> +	long copy_ret = 0;
> +
> +	if (se_cmd->se_cmd_flags & SCF_BIDI) {
> +		data_sg = se_cmd->t_bidi_data_sg;
> +		data_nents = se_cmd->t_bidi_data_nents;
> +	} else {
> +		data_sg = se_cmd->t_data_sg;
> +		data_nents = se_cmd->t_data_nents;
> +	}
> +
> +	ret = import_iovec(READ, uiovec, vcnt, ARRAY_SIZE(iovstack), &iov, &iter);
> +	if (ret < 0) {
> +		pr_err("import iovec failed.\n");
> +		return -EFAULT;
> +	}
> +
> +	for_each_sg(data_sg, sg, data_nents, i) {
> +		if (is_copy_to_sgl)
> +			ret = copy_page_from_iter(sg_page(sg), sg->offset, sg->length, &iter);
> +		else
> +			ret = copy_page_to_iter(sg_page(sg), sg->offset, sg->length, &iter);
> +		if (ret < 0) {
> +			pr_err("copy failed.\n");
> +			copy_ret = -EFAULT;
> +			break;
> +		}
> +	}
> +	kfree(iov);
> +	return copy_ret;
> +}
> +
> +long tcmu_ioctl(struct uio_info *info, unsigned int cmd, unsigned long arg)
> +{
> +	struct tcmu_dev *udev = container_of(info, struct tcmu_dev, uio_info);
> +	struct tcmu_data_xfer __user *uxfer = (struct tcmu_data_xfer __user *)arg;
> +	struct tcmu_data_xfer xfer;
> +	struct tcmu_cmd *tcmu_cmd;
> +
> +	if (!test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags))
> +		return -EINVAL;
> +
> +	if (copy_from_user(&xfer, uxfer, sizeof(xfer)))
> +		return -EFAULT;
> +
> +	tcmu_cmd = xa_load(&udev->commands, xfer.cmd_id);
> +	if (!tcmu_cmd) {
> +		set_bit(TCMU_DEV_BIT_BROKEN, &udev->flags);
> +		return -EFAULT;
> +	}
> +
> +	if (test_bit(TCMU_CMD_BIT_EXPIRED, &tcmu_cmd->flags))
> +		return -EFAULT;
> +
> +	switch (cmd) {
> +	case TCMU_IOCTL_CMD_COPY_TO_SGL:
> +		return tcmu_ioctl_copy_between_sgl_and_iovec(tcmu_cmd, xfer.iovec,
> +							     xfer.iov_cnt, true);
> +	case TCMU_IOCTL_CMD_COPY_FROM_SGL:
> +		return tcmu_ioctl_copy_between_sgl_and_iovec(tcmu_cmd, xfer.iovec,
> +							     xfer.iov_cnt, false);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>   static int tcmu_init_genl_cmd_reply(struct tcmu_dev *udev, int cmd)
>   {
>   	struct tcmu_nl_cmd *nl_cmd = &udev->curr_nl_cmd;
> @@ -2230,6 +2319,7 @@ static int tcmu_configure_device(struct se_device *dev)
>   	info->mmap = tcmu_mmap;
>   	info->open = tcmu_open;
>   	info->release = tcmu_release;
> +	info->ioctl = tcmu_ioctl;
>   
>   	ret = uio_register_device(tcmu_root_device, info);
>   	if (ret)
> @@ -2838,6 +2928,40 @@ static ssize_t tcmu_nl_reply_supported_store(struct config_item *item,
>   }
>   CONFIGFS_ATTR(tcmu_, nl_reply_supported);
>   
> +static ssize_t tcmu_bypass_data_area_show(struct config_item *item, char *page)
> +{
> +	struct se_dev_attrib *da = container_of(to_config_group(item),
> +						struct se_dev_attrib, da_group);
> +	struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
> +
> +	if (test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags))
> +		return snprintf(page, PAGE_SIZE, "%s\n", "true");
> +	else
> +		return snprintf(page, PAGE_SIZE, "%s\n", "false");
> +}
> +
> +static ssize_t tcmu_bypass_data_area_store(struct config_item *item, const char *page,
> +					    size_t count)
> +{
> +	struct se_dev_attrib *da = container_of(to_config_group(item),
> +						struct se_dev_attrib, da_group);
> +	struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
> +	bool bypass_data_area;
> +	int ret;
> +
> +	ret = strtobool(page, &bypass_data_area);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (bypass_data_area)
> +		set_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags);
> +	else
> +		clear_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags);
> +
> +	return count;
> +}
> +CONFIGFS_ATTR(tcmu_, bypass_data_area);
> +
>   static ssize_t tcmu_emulate_write_cache_show(struct config_item *item,
>   					     char *page)
>   {
> @@ -3069,6 +3193,7 @@ static ssize_t tcmu_free_kept_buf_store(struct config_item *item, const char *pa
>   	&tcmu_attr_emulate_write_cache,
>   	&tcmu_attr_tmr_notification,
>   	&tcmu_attr_nl_reply_supported,
> +	&tcmu_attr_bypass_data_area,
>   	NULL,
>   };
>   
> diff --git a/include/uapi/linux/target_core_user.h b/include/uapi/linux/target_core_user.h
> index 27ace51..c02a45e 100644
> --- a/include/uapi/linux/target_core_user.h
> +++ b/include/uapi/linux/target_core_user.h
> @@ -185,4 +185,13 @@ enum tcmu_genl_attr {
>   };
>   #define TCMU_ATTR_MAX (__TCMU_ATTR_MAX - 1)
>   
> +struct tcmu_data_xfer {
> +	unsigned short cmd_id;
> +	unsigned long iov_cnt;
> +	struct iovec __user *iovec;
> +};
> +
> +#define TCMU_IOCTL_CMD_COPY_TO_SGL      _IOW('T', 0xe0, struct tcmu_data_xfer)
> +#define TCMU_IOCTL_CMD_COPY_FROM_SGL    _IOW('T', 0xe1, struct tcmu_data_xfer)
> +
>   #endif

Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl

[Xiaoguang Wang]

hi Bodo,

> Liu,
>
> generally I like ideas to speed up tcmu.
>
> OTOH, since Andy Grover implemented tcmu based on uio device, we are
> restricted to what uio offers. With today's knowledge I think we would
> not use the uio device in tcmu again, but switching away from uio now
> would break existing userspace SW.
Yeah, it will have much work if deciding to switch away from uio.
I came up with a hacky or crazy idea :) what about we create a new file
in tcmu_open() by anon_inode_getfile_secure(), and export this fd by
tcmu mail box, we can do ioctl() on this new file, then uio framework
won't be touched...

static int tcmu_open(struct uio_info *info, struct inode *inode)
{
         struct tcmu_dev *udev = container_of(info, struct tcmu_dev, 
uio_info);

         /* O_EXCL not supported for char devs, so fake it? */
         if (test_and_set_bit(TCMU_DEV_BIT_OPEN, &udev->flags))
                 return -EBUSY;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
tcmu device can only be opened once before closed.

Regards,
Xiaoguang Wang
>
> Before we start thinking how the same performance gain could be reached
> without a change in uio device, please let me know why you use file
> backend on top of tcmu instead of target_core_file kernel module?
> Wouldn't target_core_file be even faster for your purpose?
>
> Bodo
>
>
>
> On 17.02.22 03:29, Guixin Liu wrote:
>> Currently the data needs to be copied twice between sg, tcmu data 
>> area and
>> userspace buffer if backstore holds its own userspace buffer, then we 
>> can
>> use uio ioctl to copy data between sg and userspace buffer directly to
>> bypass data area to improve performance.
>>
>> Use tcm_loop and tcmu(backstore is file) to evaluate performance,
>> fio job: fio -filename=/dev/sdb  -direct=1 -size=2G -name=1 -thread
>> -runtime=60 -time_based -rw=randread -numjobs=16 -iodepth=16 -bs=128k
>>
>> Without this patch:
>>      READ: bw=3539MiB/s (3711MB/s), 207MiB/s-233MiB/s (217MB/s-244MB/s),
>> io=104GiB (111GB), run=30001-30002msec
>>
>> With this patch:
>>      READ: bw=4420MiB/s (4634MB/s), 274MiB/s-278MiB/s (287MB/s-291MB/s),
>> io=259GiB (278GB), run=60001-60002msec
>>
>> Reviewed-by: Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com>
>> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
>> ---
>>   drivers/target/target_core_user.c     | 171 
>> +++++++++++++++++++++++++++++-----
>>   include/uapi/linux/target_core_user.h |   9 ++
>>   2 files changed, 157 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/target/target_core_user.c 
>> b/drivers/target/target_core_user.c
>> index 7b2a89a..afea088 100644
>> --- a/drivers/target/target_core_user.c
>> +++ b/drivers/target/target_core_user.c
>> @@ -122,6 +122,7 @@ struct tcmu_dev {
>>   #define TCMU_DEV_BIT_BLOCKED 2
>>   #define TCMU_DEV_BIT_TMR_NOTIFY 3
>>   #define TCMU_DEV_BIT_PLUGGED 4
>> +#define TCMU_DEV_BIT_BYPASS_DATA_AREA 5
>>       unsigned long flags;
>>         struct uio_info uio_info;
>> @@ -642,12 +643,17 @@ static struct tcmu_cmd *tcmu_alloc_cmd(struct 
>> se_cmd *se_cmd)
>>       tcmu_cmd->se_cmd = se_cmd;
>>       tcmu_cmd->tcmu_dev = udev;
>>   -    tcmu_cmd_set_block_cnts(tcmu_cmd);
>> -    tcmu_cmd->dbi = kcalloc(tcmu_cmd->dbi_cnt, sizeof(uint32_t),
>> -                GFP_NOIO);
>> -    if (!tcmu_cmd->dbi) {
>> -        kmem_cache_free(tcmu_cmd_cache, tcmu_cmd);
>> -        return NULL;
>> +    if (!test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags)) {
>> +        tcmu_cmd_set_block_cnts(tcmu_cmd);
>> +        tcmu_cmd->dbi = kcalloc(tcmu_cmd->dbi_cnt, sizeof(uint32_t),
>> +                    GFP_NOIO);
>> +        if (!tcmu_cmd->dbi) {
>> +            kmem_cache_free(tcmu_cmd_cache, tcmu_cmd);
>> +            return NULL;
>> +        }
>> +    } else {
>> +        tcmu_cmd->dbi_cnt = 0;
>> +        tcmu_cmd->dbi = NULL;
>>       }
>>         return tcmu_cmd;
>> @@ -1093,16 +1099,18 @@ static int queue_cmd_ring(struct tcmu_cmd 
>> *tcmu_cmd, sense_reason_t *scsi_err)
>>       tcmu_cmd_reset_dbi_cur(tcmu_cmd);
>>       iov = &entry->req.iov[0];
>>   -    if (se_cmd->data_direction == DMA_TO_DEVICE ||
>> -        se_cmd->se_cmd_flags & SCF_BIDI)
>> -        scatter_data_area(udev, tcmu_cmd, &iov);
>> -    else
>> -        tcmu_setup_iovs(udev, tcmu_cmd, &iov, se_cmd->data_length);
>> -
>> +    if (!test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags)) {
>> +        if (se_cmd->data_direction == DMA_TO_DEVICE ||
>> +        se_cmd->se_cmd_flags & SCF_BIDI)
>> +            scatter_data_area(udev, tcmu_cmd, &iov);
>> +        else
>> +            tcmu_setup_iovs(udev, tcmu_cmd, &iov, se_cmd->data_length);
>> +    }
>>       entry->req.iov_cnt = iov_cnt - iov_bidi_cnt;
>>         /* Handle BIDI commands */
>> -    if (se_cmd->se_cmd_flags & SCF_BIDI) {
>> +    if ((se_cmd->se_cmd_flags & SCF_BIDI)
>> +        && !test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags)) {
>>           iov++;
>>           tcmu_setup_iovs(udev, tcmu_cmd, &iov, 
>> tcmu_cmd->data_len_bidi);
>>           entry->req.iov_bidi_cnt = iov_bidi_cnt;
>> @@ -1366,16 +1374,19 @@ static bool tcmu_handle_completion(struct 
>> tcmu_cmd *cmd,
>>           else
>>               se_cmd->se_cmd_flags |= SCF_TREAT_READ_AS_NORMAL;
>>       }
>> -    if (se_cmd->se_cmd_flags & SCF_BIDI) {
>> -        /* Get Data-In buffer before clean up */
>> -        gather_data_area(udev, cmd, true, read_len);
>> -    } else if (se_cmd->data_direction == DMA_FROM_DEVICE) {
>> -        gather_data_area(udev, cmd, false, read_len);
>> -    } else if (se_cmd->data_direction == DMA_TO_DEVICE) {
>> -        /* TODO: */
>> -    } else if (se_cmd->data_direction != DMA_NONE) {
>> -        pr_warn("TCMU: data direction was %d!\n",
>> -            se_cmd->data_direction);
>> +
>> +    if (!test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags)) {
>> +        if (se_cmd->se_cmd_flags & SCF_BIDI) {
>> +            /* Get Data-In buffer before clean up */
>> +            gather_data_area(udev, cmd, true, read_len);
>> +        } else if (se_cmd->data_direction == DMA_FROM_DEVICE) {
>> +            gather_data_area(udev, cmd, false, read_len);
>> +        } else if (se_cmd->data_direction == DMA_TO_DEVICE) {
>> +            /* TODO: */
>> +        } else if (se_cmd->data_direction != DMA_NONE) {
>> +            pr_warn("TCMU: data direction was %d!\n",
>> +                se_cmd->data_direction);
>> +        }
>>       }
>>     done:
>> @@ -1973,6 +1984,84 @@ static int tcmu_release(struct uio_info *info, 
>> struct inode *inode)
>>       return 0;
>>   }
>>   +long tcmu_ioctl_copy_between_sgl_and_iovec(struct tcmu_cmd *tcmu_cmd,
>> +            struct iovec __user *uiovec,
>> +            unsigned long vcnt,
>> +            bool is_copy_to_sgl)
>> +{
>> +    struct iovec iovstack[UIO_FASTIOV];
>> +    struct iovec *iov = iovstack;
>> +    struct iov_iter iter;
>> +    ssize_t ret;
>> +    struct se_cmd *se_cmd = tcmu_cmd->se_cmd;
>> +    struct scatterlist *data_sg, *sg;
>> +    int i;
>> +    unsigned int data_nents;
>> +    long copy_ret = 0;
>> +
>> +    if (se_cmd->se_cmd_flags & SCF_BIDI) {
>> +        data_sg = se_cmd->t_bidi_data_sg;
>> +        data_nents = se_cmd->t_bidi_data_nents;
>> +    } else {
>> +        data_sg = se_cmd->t_data_sg;
>> +        data_nents = se_cmd->t_data_nents;
>> +    }
>> +
>> +    ret = import_iovec(READ, uiovec, vcnt, ARRAY_SIZE(iovstack), 
>> &iov, &iter);
>> +    if (ret < 0) {
>> +        pr_err("import iovec failed.\n");
>> +        return -EFAULT;
>> +    }
>> +
>> +    for_each_sg(data_sg, sg, data_nents, i) {
>> +        if (is_copy_to_sgl)
>> +            ret = copy_page_from_iter(sg_page(sg), sg->offset, 
>> sg->length, &iter);
>> +        else
>> +            ret = copy_page_to_iter(sg_page(sg), sg->offset, 
>> sg->length, &iter);
>> +        if (ret < 0) {
>> +            pr_err("copy failed.\n");
>> +            copy_ret = -EFAULT;
>> +            break;
>> +        }
>> +    }
>> +    kfree(iov);
>> +    return copy_ret;
>> +}
>> +
>> +long tcmu_ioctl(struct uio_info *info, unsigned int cmd, unsigned 
>> long arg)
>> +{
>> +    struct tcmu_dev *udev = container_of(info, struct tcmu_dev, 
>> uio_info);
>> +    struct tcmu_data_xfer __user *uxfer = (struct tcmu_data_xfer 
>> __user *)arg;
>> +    struct tcmu_data_xfer xfer;
>> +    struct tcmu_cmd *tcmu_cmd;
>> +
>> +    if (!test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags))
>> +        return -EINVAL;
>> +
>> +    if (copy_from_user(&xfer, uxfer, sizeof(xfer)))
>> +        return -EFAULT;
>> +
>> +    tcmu_cmd = xa_load(&udev->commands, xfer.cmd_id);
>> +    if (!tcmu_cmd) {
>> +        set_bit(TCMU_DEV_BIT_BROKEN, &udev->flags);
>> +        return -EFAULT;
>> +    }
>> +
>> +    if (test_bit(TCMU_CMD_BIT_EXPIRED, &tcmu_cmd->flags))
>> +        return -EFAULT;
>> +
>> +    switch (cmd) {
>> +    case TCMU_IOCTL_CMD_COPY_TO_SGL:
>> +        return tcmu_ioctl_copy_between_sgl_and_iovec(tcmu_cmd, 
>> xfer.iovec,
>> +                                 xfer.iov_cnt, true);
>> +    case TCMU_IOCTL_CMD_COPY_FROM_SGL:
>> +        return tcmu_ioctl_copy_between_sgl_and_iovec(tcmu_cmd, 
>> xfer.iovec,
>> +                                 xfer.iov_cnt, false);
>> +    default:
>> +        return -EINVAL;
>> +    }
>> +}
>> +
>>   static int tcmu_init_genl_cmd_reply(struct tcmu_dev *udev, int cmd)
>>   {
>>       struct tcmu_nl_cmd *nl_cmd = &udev->curr_nl_cmd;
>> @@ -2230,6 +2319,7 @@ static int tcmu_configure_device(struct 
>> se_device *dev)
>>       info->mmap = tcmu_mmap;
>>       info->open = tcmu_open;
>>       info->release = tcmu_release;
>> +    info->ioctl = tcmu_ioctl;
>>         ret = uio_register_device(tcmu_root_device, info);
>>       if (ret)
>> @@ -2838,6 +2928,40 @@ static ssize_t 
>> tcmu_nl_reply_supported_store(struct config_item *item,
>>   }
>>   CONFIGFS_ATTR(tcmu_, nl_reply_supported);
>>   +static ssize_t tcmu_bypass_data_area_show(struct config_item 
>> *item, char *page)
>> +{
>> +    struct se_dev_attrib *da = container_of(to_config_group(item),
>> +                        struct se_dev_attrib, da_group);
>> +    struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
>> +
>> +    if (test_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags))
>> +        return snprintf(page, PAGE_SIZE, "%s\n", "true");
>> +    else
>> +        return snprintf(page, PAGE_SIZE, "%s\n", "false");
>> +}
>> +
>> +static ssize_t tcmu_bypass_data_area_store(struct config_item *item, 
>> const char *page,
>> +                        size_t count)
>> +{
>> +    struct se_dev_attrib *da = container_of(to_config_group(item),
>> +                        struct se_dev_attrib, da_group);
>> +    struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
>> +    bool bypass_data_area;
>> +    int ret;
>> +
>> +    ret = strtobool(page, &bypass_data_area);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    if (bypass_data_area)
>> +        set_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags);
>> +    else
>> +        clear_bit(TCMU_DEV_BIT_BYPASS_DATA_AREA, &udev->flags);
>> +
>> +    return count;
>> +}
>> +CONFIGFS_ATTR(tcmu_, bypass_data_area);
>> +
>>   static ssize_t tcmu_emulate_write_cache_show(struct config_item *item,
>>                            char *page)
>>   {
>> @@ -3069,6 +3193,7 @@ static ssize_t tcmu_free_kept_buf_store(struct 
>> config_item *item, const char *pa
>>       &tcmu_attr_emulate_write_cache,
>>       &tcmu_attr_tmr_notification,
>>       &tcmu_attr_nl_reply_supported,
>> +    &tcmu_attr_bypass_data_area,
>>       NULL,
>>   };
>>   diff --git a/include/uapi/linux/target_core_user.h 
>> b/include/uapi/linux/target_core_user.h
>> index 27ace51..c02a45e 100644
>> --- a/include/uapi/linux/target_core_user.h
>> +++ b/include/uapi/linux/target_core_user.h
>> @@ -185,4 +185,13 @@ enum tcmu_genl_attr {
>>   };
>>   #define TCMU_ATTR_MAX (__TCMU_ATTR_MAX - 1)
>>   +struct tcmu_data_xfer {
>> +    unsigned short cmd_id;
>> +    unsigned long iov_cnt;
>> +    struct iovec __user *iovec;
>> +};
>> +
>> +#define TCMU_IOCTL_CMD_COPY_TO_SGL      _IOW('T', 0xe0, struct 
>> tcmu_data_xfer)
>> +#define TCMU_IOCTL_CMD_COPY_FROM_SGL    _IOW('T', 0xe1, struct 
>> tcmu_data_xfer)
>> +
>>   #endif

Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl

[Greg KH]

On Mon, Feb 28, 2022 at 04:52:52PM +0800, Xiaoguang Wang wrote:
> 
> hi Bodo,
> 
> > Liu,
> > 
> > generally I like ideas to speed up tcmu.
> > 
> > OTOH, since Andy Grover implemented tcmu based on uio device, we are
> > restricted to what uio offers. With today's knowledge I think we would
> > not use the uio device in tcmu again, but switching away from uio now
> > would break existing userspace SW.
> Yeah, it will have much work if deciding to switch away from uio.
> I came up with a hacky or crazy idea :) what about we create a new file
> in tcmu_open() by anon_inode_getfile_secure(), and export this fd by
> tcmu mail box, we can do ioctl() on this new file, then uio framework
> won't be touched...

No new ioctls please.  That is creating a new user/kernel api that you
must support for the next 20+ years.  Please do not do that.

thanks,

greg k-h