* [PATCH 1/2] uio: add ioctl to uio
@ 2022-02-17 2:29 Guixin Liu
2022-02-17 2:29 ` [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl Guixin Liu
` (2 more replies)
0 siblings, 3 replies; 21+ messages in thread
From: Guixin Liu @ 2022-02-17 2:29 UTC (permalink / raw)
To: gregkh, bostroesser, martin.petersen
Cc: linux-scsi, target-devel, linux-kernel, xiaoguang.wang, xlpang
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl
2022-02-17 2:29 [PATCH 1/2] uio: add ioctl to uio Guixin Liu
@ 2022-02-17 2:29 ` Guixin Liu
2022-02-17 6:45 ` kernel test robot
` (5 more replies)
2022-02-17 6:13 ` [PATCH 1/2] uio: add ioctl to uio Greg KH
2022-02-17 6:55 ` kernel test robot
2 siblings, 6 replies; 21+ messages in thread
From: Guixin Liu @ 2022-02-17 2:29 UTC (permalink / raw)
To: gregkh, bostroesser, martin.petersen
Cc: linux-scsi, target-devel, linux-kernel, xiaoguang.wang, xlpang
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] uio: add ioctl to uio
2022-02-17 2:29 [PATCH 1/2] uio: add ioctl to uio Guixin Liu
2022-02-17 2:29 ` [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl Guixin Liu
@ 2022-02-17 6:13 ` Greg KH
[not found] ` <362edb61-b8ad-495e-2346-8020355c0938@linux.alibaba.com>
2022-02-17 12:30 ` Xiaoguang Wang
2022-02-17 6:55 ` kernel test robot
2 siblings, 2 replies; 21+ messages in thread
From: Greg KH @ 2022-02-17 6:13 UTC (permalink / raw)
To: Guixin Liu
Cc: bostroesser, martin.petersen, linux-scsi, target-devel,
linux-kernel, xiaoguang.wang, xlpang
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl
2022-02-17 2:29 ` [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl Guixin Liu
@ 2022-02-17 6:45 ` kernel test robot
2022-02-17 8:07 ` kernel test robot
` (4 subsequent siblings)
5 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2022-02-17 6:45 UTC (permalink / raw)
To: Guixin Liu, gregkh, bostroesser, martin.petersen
Cc: kbuild-all, linux-scsi, target-devel, linux-kernel,
xiaoguang.wang, xlpang
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl
@ 2022-02-17 6:45 ` kernel test robot
0 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2022-02-17 6:45 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 5049 bytes --]
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(a)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(a)lists.01.org
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] uio: add ioctl to uio
2022-02-17 2:29 [PATCH 1/2] uio: add ioctl to uio Guixin Liu
@ 2022-02-17 6:55 ` kernel test robot
2022-02-17 6:13 ` [PATCH 1/2] uio: add ioctl to uio Greg KH
2022-02-17 6:55 ` kernel test robot
2 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2022-02-17 6:55 UTC (permalink / raw)
To: Guixin Liu, gregkh, bostroesser, martin.petersen
Cc: kbuild-all, linux-scsi, target-devel, linux-kernel,
xiaoguang.wang, xlpang
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] uio: add ioctl to uio
@ 2022-02-17 6:55 ` kernel test robot
0 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2022-02-17 6:55 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2588 bytes --]
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(a)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(a)lists.01.org
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl
2022-02-17 2:29 ` [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl Guixin Liu
@ 2022-02-17 8:07 ` kernel test robot
2022-02-17 8:07 ` kernel test robot
` (4 subsequent siblings)
5 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2022-02-17 8:07 UTC (permalink / raw)
To: Guixin Liu, gregkh, bostroesser, martin.petersen
Cc: llvm, kbuild-all, linux-scsi, target-devel, linux-kernel,
xiaoguang.wang, xlpang
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl
@ 2022-02-17 8:07 ` kernel test robot
0 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2022-02-17 8:07 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 5701 bytes --]
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(a)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(a)lists.01.org
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl
2022-02-17 2:29 ` [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl Guixin Liu
2022-02-17 6:45 ` kernel test robot
2022-02-17 8:07 ` kernel test robot
@ 2022-02-17 10:15 ` Greg KH
2022-02-17 11:13 ` Xiaoguang Wang
2022-02-17 11:10 ` kernel test robot
` (2 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Greg KH @ 2022-02-17 10:15 UTC (permalink / raw)
To: Guixin Liu
Cc: bostroesser, martin.petersen, linux-scsi, target-devel,
linux-kernel, xiaoguang.wang, xlpang
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC PATCH] scsi:target:tcmu: tcmu_ioctl_copy_between_sgl_and_iovec() can be static
2022-02-17 2:29 ` [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl Guixin Liu
@ 2022-02-17 11:10 ` kernel test robot
2022-02-17 8:07 ` kernel test robot
` (4 subsequent siblings)
5 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2022-02-17 11:10 UTC (permalink / raw)
To: Guixin Liu, gregkh, bostroesser, martin.petersen
Cc: kbuild-all, linux-scsi, target-devel, linux-kernel,
xiaoguang.wang, xlpang
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;
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [RFC PATCH] scsi:target:tcmu: tcmu_ioctl_copy_between_sgl_and_iovec() can be static
@ 2022-02-17 11:10 ` kernel test robot
0 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2022-02-17 11:10 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 1449 bytes --]
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;
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl
2022-02-17 2:29 ` [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl Guixin Liu
@ 2022-02-17 11:12 ` kernel test robot
2022-02-17 8:07 ` kernel test robot
` (4 subsequent siblings)
5 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2022-02-17 11:12 UTC (permalink / raw)
To: Guixin Liu, gregkh, bostroesser, martin.petersen
Cc: kbuild-all, linux-scsi, target-devel, linux-kernel,
xiaoguang.wang, xlpang
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl
@ 2022-02-17 11:12 ` kernel test robot
0 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2022-02-17 11:12 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2004 bytes --]
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(a)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(a)lists.01.org
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl
2022-02-17 10:15 ` Greg KH
@ 2022-02-17 11:13 ` Xiaoguang Wang
2022-02-17 12:28 ` Greg KH
0 siblings, 1 reply; 21+ messages in thread
From: Xiaoguang Wang @ 2022-02-17 11:13 UTC (permalink / raw)
To: Greg KH, Guixin Liu
Cc: bostroesser, martin.petersen, linux-scsi, target-devel,
linux-kernel, xlpang
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl
2022-02-17 11:13 ` Xiaoguang Wang
@ 2022-02-17 12:28 ` Greg KH
0 siblings, 0 replies; 21+ messages in thread
From: Greg KH @ 2022-02-17 12:28 UTC (permalink / raw)
To: Xiaoguang Wang
Cc: Guixin Liu, bostroesser, martin.petersen, linux-scsi,
target-devel, linux-kernel, xlpang
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] uio: add ioctl to uio
[not found] ` <362edb61-b8ad-495e-2346-8020355c0938@linux.alibaba.com>
@ 2022-02-17 12:30 ` Greg KH
0 siblings, 0 replies; 21+ messages in thread
From: Greg KH @ 2022-02-17 12:30 UTC (permalink / raw)
To: Xiaoguang Wang
Cc: Guixin Liu, bostroesser, martin.petersen, linux-scsi,
target-devel, linux-kernel, xlpang
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] uio: add ioctl to uio
2022-02-17 6:13 ` [PATCH 1/2] uio: add ioctl to uio Greg KH
[not found] ` <362edb61-b8ad-495e-2346-8020355c0938@linux.alibaba.com>
@ 2022-02-17 12:30 ` Xiaoguang Wang
1 sibling, 0 replies; 21+ messages in thread
From: Xiaoguang Wang @ 2022-02-17 12:30 UTC (permalink / raw)
To: Greg KH, Guixin Liu
Cc: bostroesser, martin.petersen, linux-scsi, target-devel,
linux-kernel, xlpang
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl
2022-02-17 2:29 ` [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl Guixin Liu
` (4 preceding siblings ...)
2022-02-17 11:12 ` kernel test robot
@ 2022-02-21 17:09 ` Bodo Stroesser
2022-02-28 8:52 ` Xiaoguang Wang
5 siblings, 1 reply; 21+ messages in thread
From: Bodo Stroesser @ 2022-02-21 17:09 UTC (permalink / raw)
To: Guixin Liu, gregkh, martin.petersen
Cc: linux-scsi, target-devel, linux-kernel, xiaoguang.wang, xlpang
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl
2022-02-21 17:09 ` Bodo Stroesser
@ 2022-02-28 8:52 ` Xiaoguang Wang
2022-02-28 10:47 ` Greg KH
0 siblings, 1 reply; 21+ messages in thread
From: Xiaoguang Wang @ 2022-02-28 8:52 UTC (permalink / raw)
To: Bodo Stroesser, Guixin Liu, gregkh, martin.petersen
Cc: linux-scsi, target-devel, linux-kernel, xlpang
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl
2022-02-28 8:52 ` Xiaoguang Wang
@ 2022-02-28 10:47 ` Greg KH
0 siblings, 0 replies; 21+ messages in thread
From: Greg KH @ 2022-02-28 10:47 UTC (permalink / raw)
To: Xiaoguang Wang
Cc: Bodo Stroesser, Guixin Liu, martin.petersen, linux-scsi,
target-devel, linux-kernel, xlpang
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
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2022-02-28 10:47 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-17 2:29 [PATCH 1/2] uio: add ioctl to uio Guixin Liu
2022-02-17 2:29 ` [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl Guixin Liu
2022-02-17 6:45 ` kernel test robot
2022-02-17 6:45 ` kernel test robot
2022-02-17 8:07 ` kernel test robot
2022-02-17 8:07 ` kernel test robot
2022-02-17 10:15 ` Greg KH
2022-02-17 11:13 ` Xiaoguang Wang
2022-02-17 12:28 ` Greg KH
2022-02-17 11:10 ` [RFC PATCH] scsi:target:tcmu: tcmu_ioctl_copy_between_sgl_and_iovec() can be static kernel test robot
2022-02-17 11:10 ` kernel test robot
2022-02-17 11:12 ` [PATCH 2/2] scsi:target:tcmu: reduce once copy by using uio ioctl kernel test robot
2022-02-17 11:12 ` kernel test robot
2022-02-21 17:09 ` Bodo Stroesser
2022-02-28 8:52 ` Xiaoguang Wang
2022-02-28 10:47 ` Greg KH
2022-02-17 6:13 ` [PATCH 1/2] uio: add ioctl to uio Greg KH
[not found] ` <362edb61-b8ad-495e-2346-8020355c0938@linux.alibaba.com>
2022-02-17 12:30 ` Greg KH
2022-02-17 12:30 ` Xiaoguang Wang
2022-02-17 6:55 ` kernel test robot
2022-02-17 6:55 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.