| Message ID | 1600712588-9514-4-git-send-email-michael.christie@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vhost scsi: fixes and cleanups | expand |
Hi Mike,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on vhost/linux-next]
[also build test WARNING on v5.9-rc6 next-20200921]
[cannot apply to target/for-next]
[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/Mike-Christie/vhost-scsi-fixes-and-cleanups/20200922-031251
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: s390-randconfig-r022-20200920 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 4e8c028158b56d9c2142a62464e8e0686bde3584)
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 s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390
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 >>):
include/asm-generic/io.h:490:45: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu(__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:20:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x0000ff00UL) << 8) | \
^
In file included from drivers/vhost/scsi.c:45:
In file included from include/uapi/linux/vhost.h:14:
In file included from include/uapi/linux/vhost_types.h:16:
In file included from include/linux/virtio_config.h:7:
In file included from include/linux/virtio.h:7:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:490:45: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu(__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:21:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x00ff0000UL) >> 8) | \
^
In file included from drivers/vhost/scsi.c:45:
In file included from include/uapi/linux/vhost.h:14:
In file included from include/uapi/linux/vhost_types.h:16:
In file included from include/linux/virtio_config.h:7:
In file included from include/linux/virtio.h:7:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:490:45: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu(__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:22:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0xff000000UL) >> 24)))
^
In file included from drivers/vhost/scsi.c:45:
In file included from include/uapi/linux/vhost.h:14:
In file included from include/uapi/linux/vhost_types.h:16:
In file included from include/linux/virtio_config.h:7:
In file included from include/linux/virtio.h:7:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:490:45: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu(__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:120:12: note: expanded from macro '__swab32'
__fswab32(x))
^
In file included from drivers/vhost/scsi.c:45:
In file included from include/uapi/linux/vhost.h:14:
In file included from include/uapi/linux/vhost_types.h:16:
In file included from include/linux/virtio_config.h:7:
In file included from include/linux/virtio.h:7:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:511:46: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew(cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:521:46: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel(cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> drivers/vhost/scsi.c:606:28: warning: variable 'cpu' is uninitialized when used here [-Wuninitialized]
cmd->tvc_se_cmd.map_cpu = cpu;
^~~
drivers/vhost/scsi.c:583:14: note: initialize the variable 'cpu' to silence this warning
int tag, cpu;
^
= 0
21 warnings generated.
# https://github.com/0day-ci/linux/commit/aef0e1e9298ab68f2d7bdf1afb9a376641b993d5
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Mike-Christie/vhost-scsi-fixes-and-cleanups/20200922-031251
git checkout aef0e1e9298ab68f2d7bdf1afb9a376641b993d5
vim +/cpu +606 drivers/vhost/scsi.c
057cbf49a1f082 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 571
1a1ff8256af679 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 572 static struct vhost_scsi_cmd *
aef0e1e9298ab6 drivers/vhost/scsi.c Mike Christie 2020-09-21 573 vhost_scsi_get_cmd(struct vhost_virtqueue *vq, struct vhost_scsi_tpg *tpg,
95e7c4341b8e28 drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 574 unsigned char *cdb, u64 scsi_tag, u16 lun, u8 task_attr,
95e7c4341b8e28 drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 575 u32 exp_data_len, int data_direction)
057cbf49a1f082 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 576 {
aef0e1e9298ab6 drivers/vhost/scsi.c Mike Christie 2020-09-21 577 struct vhost_scsi_virtqueue *svq = container_of(vq,
aef0e1e9298ab6 drivers/vhost/scsi.c Mike Christie 2020-09-21 578 struct vhost_scsi_virtqueue, vq);
1a1ff8256af679 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 579 struct vhost_scsi_cmd *cmd;
1a1ff8256af679 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 580 struct vhost_scsi_nexus *tv_nexus;
b1935f687bb93b drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 581 struct scatterlist *sg, *prot_sg;
3aee26b4ae9104 drivers/vhost/scsi.c Nicholas Bellinger 2013-06-21 582 struct page **pages;
10e9cbb6b53111 drivers/vhost/scsi.c Matthew Wilcox 2018-06-12 583 int tag, cpu;
057cbf49a1f082 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 584
9871831283e795 drivers/vhost/scsi.c Asias He 2013-05-06 585 tv_nexus = tpg->tpg_nexus;
057cbf49a1f082 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 586 if (!tv_nexus) {
1a1ff8256af679 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 587 pr_err("Unable to locate active struct vhost_scsi_nexus\n");
057cbf49a1f082 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 588 return ERR_PTR(-EIO);
057cbf49a1f082 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 589 }
057cbf49a1f082 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 590
aef0e1e9298ab6 drivers/vhost/scsi.c Mike Christie 2020-09-21 591 tag = sbitmap_get(&svq->scsi_tags, 0, false);
4a47d3a1ff10e5 drivers/vhost/scsi.c Nicholas Bellinger 2013-09-23 592 if (tag < 0) {
1a1ff8256af679 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 593 pr_err("Unable to obtain tag for vhost_scsi_cmd\n");
4a47d3a1ff10e5 drivers/vhost/scsi.c Nicholas Bellinger 2013-09-23 594 return ERR_PTR(-ENOMEM);
4a47d3a1ff10e5 drivers/vhost/scsi.c Nicholas Bellinger 2013-09-23 595 }
4a47d3a1ff10e5 drivers/vhost/scsi.c Nicholas Bellinger 2013-09-23 596
aef0e1e9298ab6 drivers/vhost/scsi.c Mike Christie 2020-09-21 597 cmd = &svq->scsi_cmds[tag];
3aee26b4ae9104 drivers/vhost/scsi.c Nicholas Bellinger 2013-06-21 598 sg = cmd->tvc_sgl;
b1935f687bb93b drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 599 prot_sg = cmd->tvc_prot_sgl;
3aee26b4ae9104 drivers/vhost/scsi.c Nicholas Bellinger 2013-06-21 600 pages = cmd->tvc_upages;
473f0b15a4c97d drivers/vhost/scsi.c Markus Elfring 2017-05-20 601 memset(cmd, 0, sizeof(*cmd));
3aee26b4ae9104 drivers/vhost/scsi.c Nicholas Bellinger 2013-06-21 602 cmd->tvc_sgl = sg;
b1935f687bb93b drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 603 cmd->tvc_prot_sgl = prot_sg;
3aee26b4ae9104 drivers/vhost/scsi.c Nicholas Bellinger 2013-06-21 604 cmd->tvc_upages = pages;
4824d3bfb9097a drivers/vhost/scsi.c Nicholas Bellinger 2013-06-07 605 cmd->tvc_se_cmd.map_tag = tag;
10e9cbb6b53111 drivers/vhost/scsi.c Matthew Wilcox 2018-06-12 @606 cmd->tvc_se_cmd.map_cpu = cpu;
95e7c4341b8e28 drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 607 cmd->tvc_tag = scsi_tag;
95e7c4341b8e28 drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 608 cmd->tvc_lun = lun;
95e7c4341b8e28 drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 609 cmd->tvc_task_attr = task_attr;
3c63f66a0dcdd6 drivers/vhost/scsi.c Asias He 2013-05-06 610 cmd->tvc_exp_data_len = exp_data_len;
3c63f66a0dcdd6 drivers/vhost/scsi.c Asias He 2013-05-06 611 cmd->tvc_data_direction = data_direction;
3c63f66a0dcdd6 drivers/vhost/scsi.c Asias He 2013-05-06 612 cmd->tvc_nexus = tv_nexus;
1a1ff8256af679 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 613 cmd->inflight = vhost_scsi_get_inflight(vq);
057cbf49a1f082 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 614
1a1ff8256af679 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 615 memcpy(cmd->tvc_cdb, cdb, VHOST_SCSI_MAX_CDB_SIZE);
95e7c4341b8e28 drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 616
3c63f66a0dcdd6 drivers/vhost/scsi.c Asias He 2013-05-06 617 return cmd;
057cbf49a1f082 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 618 }
057cbf49a1f082 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 619
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Mike,
url: https://github.com/0day-ci/linux/commits/Mike-Christie/vhost-scsi-fixes-and-cleanups/20200922-031251
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: i386-randconfig-m021-20200923 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/vhost/scsi.c:606 vhost_scsi_get_cmd() error: uninitialized symbol 'cpu'.
# https://github.com/0day-ci/linux/commit/aef0e1e9298ab68f2d7bdf1afb9a376641b993d5
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Mike-Christie/vhost-scsi-fixes-and-cleanups/20200922-031251
git checkout aef0e1e9298ab68f2d7bdf1afb9a376641b993d5
vim +/cpu +606 drivers/vhost/scsi.c
1a1ff8256af679c8 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 572 static struct vhost_scsi_cmd *
aef0e1e9298ab68f drivers/vhost/scsi.c Mike Christie 2020-09-21 573 vhost_scsi_get_cmd(struct vhost_virtqueue *vq, struct vhost_scsi_tpg *tpg,
95e7c4341b8e28da drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 574 unsigned char *cdb, u64 scsi_tag, u16 lun, u8 task_attr,
95e7c4341b8e28da drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 575 u32 exp_data_len, int data_direction)
057cbf49a1f08297 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 576 {
aef0e1e9298ab68f drivers/vhost/scsi.c Mike Christie 2020-09-21 577 struct vhost_scsi_virtqueue *svq = container_of(vq,
aef0e1e9298ab68f drivers/vhost/scsi.c Mike Christie 2020-09-21 578 struct vhost_scsi_virtqueue, vq);
1a1ff8256af679c8 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 579 struct vhost_scsi_cmd *cmd;
1a1ff8256af679c8 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 580 struct vhost_scsi_nexus *tv_nexus;
b1935f687bb93b20 drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 581 struct scatterlist *sg, *prot_sg;
3aee26b4ae91048c drivers/vhost/scsi.c Nicholas Bellinger 2013-06-21 582 struct page **pages;
10e9cbb6b531117b drivers/vhost/scsi.c Matthew Wilcox 2018-06-12 583 int tag, cpu;
057cbf49a1f08297 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 584
9871831283e79575 drivers/vhost/scsi.c Asias He 2013-05-06 585 tv_nexus = tpg->tpg_nexus;
057cbf49a1f08297 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 586 if (!tv_nexus) {
1a1ff8256af679c8 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 587 pr_err("Unable to locate active struct vhost_scsi_nexus\n");
057cbf49a1f08297 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 588 return ERR_PTR(-EIO);
057cbf49a1f08297 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 589 }
057cbf49a1f08297 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 590
aef0e1e9298ab68f drivers/vhost/scsi.c Mike Christie 2020-09-21 591 tag = sbitmap_get(&svq->scsi_tags, 0, false);
4a47d3a1ff10e564 drivers/vhost/scsi.c Nicholas Bellinger 2013-09-23 592 if (tag < 0) {
1a1ff8256af679c8 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 593 pr_err("Unable to obtain tag for vhost_scsi_cmd\n");
4a47d3a1ff10e564 drivers/vhost/scsi.c Nicholas Bellinger 2013-09-23 594 return ERR_PTR(-ENOMEM);
4a47d3a1ff10e564 drivers/vhost/scsi.c Nicholas Bellinger 2013-09-23 595 }
4a47d3a1ff10e564 drivers/vhost/scsi.c Nicholas Bellinger 2013-09-23 596
aef0e1e9298ab68f drivers/vhost/scsi.c Mike Christie 2020-09-21 597 cmd = &svq->scsi_cmds[tag];
3aee26b4ae91048c drivers/vhost/scsi.c Nicholas Bellinger 2013-06-21 598 sg = cmd->tvc_sgl;
b1935f687bb93b20 drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 599 prot_sg = cmd->tvc_prot_sgl;
3aee26b4ae91048c drivers/vhost/scsi.c Nicholas Bellinger 2013-06-21 600 pages = cmd->tvc_upages;
473f0b15a4c97d39 drivers/vhost/scsi.c Markus Elfring 2017-05-20 601 memset(cmd, 0, sizeof(*cmd));
3aee26b4ae91048c drivers/vhost/scsi.c Nicholas Bellinger 2013-06-21 602 cmd->tvc_sgl = sg;
b1935f687bb93b20 drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 603 cmd->tvc_prot_sgl = prot_sg;
3aee26b4ae91048c drivers/vhost/scsi.c Nicholas Bellinger 2013-06-21 604 cmd->tvc_upages = pages;
4824d3bfb9097ac5 drivers/vhost/scsi.c Nicholas Bellinger 2013-06-07 605 cmd->tvc_se_cmd.map_tag = tag;
10e9cbb6b531117b drivers/vhost/scsi.c Matthew Wilcox 2018-06-12 @606 cmd->tvc_se_cmd.map_cpu = cpu;
"cpu" is never initialized.
95e7c4341b8e28da drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 607 cmd->tvc_tag = scsi_tag;
95e7c4341b8e28da drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 608 cmd->tvc_lun = lun;
95e7c4341b8e28da drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 609 cmd->tvc_task_attr = task_attr;
3c63f66a0dcdd6cb drivers/vhost/scsi.c Asias He 2013-05-06 610 cmd->tvc_exp_data_len = exp_data_len;
3c63f66a0dcdd6cb drivers/vhost/scsi.c Asias He 2013-05-06 611 cmd->tvc_data_direction = data_direction;
3c63f66a0dcdd6cb drivers/vhost/scsi.c Asias He 2013-05-06 612 cmd->tvc_nexus = tv_nexus;
1a1ff8256af679c8 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 613 cmd->inflight = vhost_scsi_get_inflight(vq);
057cbf49a1f08297 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 614
1a1ff8256af679c8 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 615 memcpy(cmd->tvc_cdb, cdb, VHOST_SCSI_MAX_CDB_SIZE);
95e7c4341b8e28da drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 616
3c63f66a0dcdd6cb drivers/vhost/scsi.c Asias He 2013-05-06 617 return cmd;
057cbf49a1f08297 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 618 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Mon, Sep 21, 2020 at 01:23:03PM -0500, Mike Christie wrote: > We currently are limited to 256 cmds per session. This leads to problems > where if the user has increased virtqueue_size to more than 2 or > cmd_per_lun to more than 256 vhost_scsi_get_tag can fail and the guest > will get IO errors. > > This patch moves the cmd allocation to per vq so we can easily match > whatever the user has specified for num_queues and > virtqueue_size/cmd_per_lun. It also makes it easier to control how much > memory we preallocate. For cases, where perf is not as important and > we can use the current defaults (1 vq and 128 cmds per vq) memory use > from preallocate cmds is cut in half. For cases, where we are willing > to use more memory for higher perf, cmd mem use will now increase as > the num queues and queue depth increases. > > Signed-off-by: Mike Christie <michael.christie@oracle.com> > --- > drivers/vhost/scsi.c | 204 ++++++++++++++++++++++++++++++++------------------- > 1 file changed, 127 insertions(+), 77 deletions(-) > > diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c > index b22adf0..13311b8 100644 > --- a/drivers/vhost/scsi.c > +++ b/drivers/vhost/scsi.c > @@ -52,7 +52,6 @@ > #define VHOST_SCSI_VERSION "v0.1" > #define VHOST_SCSI_NAMELEN 256 > #define VHOST_SCSI_MAX_CDB_SIZE 32 > -#define VHOST_SCSI_DEFAULT_TAGS 256 > #define VHOST_SCSI_PREALLOC_SGLS 2048 > #define VHOST_SCSI_PREALLOC_UPAGES 2048 > #define VHOST_SCSI_PREALLOC_PROT_SGLS 2048 > @@ -189,6 +188,9 @@ struct vhost_scsi_virtqueue { > * Writers must also take dev mutex and flush under it. > */ > int inflight_idx; > + struct vhost_scsi_cmd *scsi_cmds; > + struct sbitmap scsi_tags; > + int max_cmds; > }; > > struct vhost_scsi { > @@ -324,7 +326,9 @@ static void vhost_scsi_release_cmd(struct se_cmd *se_cmd) > { > struct vhost_scsi_cmd *tv_cmd = container_of(se_cmd, > struct vhost_scsi_cmd, tvc_se_cmd); > - struct se_session *se_sess = tv_cmd->tvc_nexus->tvn_se_sess; > + struct vhost_scsi_virtqueue *svq = container_of(tv_cmd->tvc_vq, > + struct vhost_scsi_virtqueue, vq); > + struct vhost_scsi_inflight *inflight = tv_cmd->inflight; > int i; > > if (tv_cmd->tvc_sgl_count) { > @@ -336,8 +340,8 @@ static void vhost_scsi_release_cmd(struct se_cmd *se_cmd) > put_page(sg_page(&tv_cmd->tvc_prot_sgl[i])); > } > > - vhost_scsi_put_inflight(tv_cmd->inflight); > - target_free_tag(se_sess, se_cmd); > + sbitmap_clear_bit(&svq->scsi_tags, se_cmd->map_tag); > + vhost_scsi_put_inflight(inflight); > } > > static u32 vhost_scsi_sess_get_index(struct se_session *se_sess) > @@ -566,13 +570,14 @@ static void vhost_scsi_complete_cmd_work(struct vhost_work *work) > } > > static struct vhost_scsi_cmd * > -vhost_scsi_get_tag(struct vhost_virtqueue *vq, struct vhost_scsi_tpg *tpg, > +vhost_scsi_get_cmd(struct vhost_virtqueue *vq, struct vhost_scsi_tpg *tpg, > unsigned char *cdb, u64 scsi_tag, u16 lun, u8 task_attr, > u32 exp_data_len, int data_direction) > { > + struct vhost_scsi_virtqueue *svq = container_of(vq, > + struct vhost_scsi_virtqueue, vq); > struct vhost_scsi_cmd *cmd; > struct vhost_scsi_nexus *tv_nexus; > - struct se_session *se_sess; > struct scatterlist *sg, *prot_sg; > struct page **pages; > int tag, cpu; > @@ -582,15 +587,14 @@ static void vhost_scsi_complete_cmd_work(struct vhost_work *work) > pr_err("Unable to locate active struct vhost_scsi_nexus\n"); > return ERR_PTR(-EIO); > } > - se_sess = tv_nexus->tvn_se_sess; > > - tag = sbitmap_queue_get(&se_sess->sess_tag_pool, &cpu); > + tag = sbitmap_get(&svq->scsi_tags, 0, false); > if (tag < 0) { > pr_err("Unable to obtain tag for vhost_scsi_cmd\n"); > return ERR_PTR(-ENOMEM); > } After this change, cpu is uninitialized. > > - cmd = &((struct vhost_scsi_cmd *)se_sess->sess_cmd_map)[tag]; > + cmd = &svq->scsi_cmds[tag]; > sg = cmd->tvc_sgl; > prot_sg = cmd->tvc_prot_sgl; > pages = cmd->tvc_upages; > @@ -1065,11 +1069,11 @@ static void vhost_scsi_submission_work(struct work_struct *work) > scsi_command_size(cdb), VHOST_SCSI_MAX_CDB_SIZE); > goto err; > } > - cmd = vhost_scsi_get_tag(vq, tpg, cdb, tag, lun, task_attr, > + cmd = vhost_scsi_get_cmd(vq, tpg, cdb, tag, lun, task_attr, > exp_data_len + prot_bytes, > data_direction); > if (IS_ERR(cmd)) { > - vq_err(vq, "vhost_scsi_get_tag failed %ld\n", > + vq_err(vq, "vhost_scsi_get_cmd failed %ld\n", > PTR_ERR(cmd)); > goto err; > } > @@ -1373,6 +1377,83 @@ static void vhost_scsi_flush(struct vhost_scsi *vs) > wait_for_completion(&old_inflight[i]->comp); > } > > +static void vhost_scsi_destroy_vq_cmds(struct vhost_virtqueue *vq) > +{ > + struct vhost_scsi_virtqueue *svq = container_of(vq, > + struct vhost_scsi_virtqueue, vq); > + struct vhost_scsi_cmd *tv_cmd; > + unsigned int i; > + > + if (!svq->scsi_cmds) > + return; > + > + for (i = 0; i < svq->max_cmds; i++) { > + tv_cmd = &svq->scsi_cmds[i]; > + > + kfree(tv_cmd->tvc_sgl); > + kfree(tv_cmd->tvc_prot_sgl); > + kfree(tv_cmd->tvc_upages); > + } > + > + sbitmap_free(&svq->scsi_tags); > + kfree(svq->scsi_cmds); > + svq->scsi_cmds = NULL; > +} > + > +static int vhost_scsi_setup_vq_cmds(struct vhost_virtqueue *vq, int max_cmds) > +{ > + struct vhost_scsi_virtqueue *svq = container_of(vq, > + struct vhost_scsi_virtqueue, vq); > + struct vhost_scsi_cmd *tv_cmd; > + unsigned int i; > + > + if (svq->scsi_cmds) > + return 0; > + > + if (sbitmap_init_node(&svq->scsi_tags, max_cmds, -1, GFP_KERNEL, > + NUMA_NO_NODE)) > + return -ENOMEM; > + svq->max_cmds = max_cmds; > + > + svq->scsi_cmds = kcalloc(max_cmds, sizeof(*tv_cmd), GFP_KERNEL); > + if (!svq->scsi_cmds) { > + sbitmap_free(&svq->scsi_tags); > + return -ENOMEM; > + } > + > + for (i = 0; i < max_cmds; i++) { > + tv_cmd = &svq->scsi_cmds[i]; > + > + tv_cmd->tvc_sgl = kcalloc(VHOST_SCSI_PREALLOC_SGLS, > + sizeof(struct scatterlist), > + GFP_KERNEL); > + if (!tv_cmd->tvc_sgl) { > + pr_err("Unable to allocate tv_cmd->tvc_sgl\n"); > + goto out; > + } > + > + tv_cmd->tvc_upages = kcalloc(VHOST_SCSI_PREALLOC_UPAGES, > + sizeof(struct page *), > + GFP_KERNEL); > + if (!tv_cmd->tvc_upages) { > + pr_err("Unable to allocate tv_cmd->tvc_upages\n"); > + goto out; > + } > + > + tv_cmd->tvc_prot_sgl = kcalloc(VHOST_SCSI_PREALLOC_PROT_SGLS, > + sizeof(struct scatterlist), > + GFP_KERNEL); > + if (!tv_cmd->tvc_prot_sgl) { > + pr_err("Unable to allocate tv_cmd->tvc_prot_sgl\n"); > + goto out; > + } > + } > + return 0; > +out: > + vhost_scsi_destroy_vq_cmds(vq); > + return -ENOMEM; > +} > + > /* > * Called from vhost_scsi_ioctl() context to walk the list of available > * vhost_scsi_tpg with an active struct vhost_scsi_nexus > @@ -1427,10 +1508,9 @@ static void vhost_scsi_flush(struct vhost_scsi *vs) > > if (!strcmp(tv_tport->tport_name, t->vhost_wwpn)) { > if (vs->vs_tpg && vs->vs_tpg[tpg->tport_tpgt]) { > - kfree(vs_tpg); > mutex_unlock(&tpg->tv_tpg_mutex); > ret = -EEXIST; > - goto out; > + goto undepend; > } > /* > * In order to ensure individual vhost-scsi configfs > @@ -1442,9 +1522,8 @@ static void vhost_scsi_flush(struct vhost_scsi *vs) > ret = target_depend_item(&se_tpg->tpg_group.cg_item); > if (ret) { > pr_warn("target_depend_item() failed: %d\n", ret); > - kfree(vs_tpg); > mutex_unlock(&tpg->tv_tpg_mutex); > - goto out; > + goto undepend; > } > tpg->tv_tpg_vhost_count++; > tpg->vhost_scsi = vs; > @@ -1457,6 +1536,16 @@ static void vhost_scsi_flush(struct vhost_scsi *vs) > if (match) { > memcpy(vs->vs_vhost_wwpn, t->vhost_wwpn, > sizeof(vs->vs_vhost_wwpn)); > + > + for (i = VHOST_SCSI_VQ_IO; i < VHOST_SCSI_MAX_VQ; i++) { > + vq = &vs->vqs[i].vq; > + if (!vhost_vq_is_setup(vq)) > + continue; > + > + if (vhost_scsi_setup_vq_cmds(vq, vq->num)) > + goto destroy_vq_cmds; > + } > + > for (i = 0; i < VHOST_SCSI_MAX_VQ; i++) { > vq = &vs->vqs[i].vq; > mutex_lock(&vq->mutex); > @@ -1476,7 +1565,22 @@ static void vhost_scsi_flush(struct vhost_scsi *vs) > vhost_scsi_flush(vs); > kfree(vs->vs_tpg); > vs->vs_tpg = vs_tpg; > + goto out; > > +destroy_vq_cmds: > + for (i--; i >= VHOST_SCSI_VQ_IO; i--) { > + if (!vhost_vq_get_backend(&vs->vqs[i].vq)) > + vhost_scsi_destroy_vq_cmds(&vs->vqs[i].vq); > + } > +undepend: > + for (i = 0; i < VHOST_SCSI_MAX_TARGET; i++) { > + tpg = vs_tpg[i]; > + if (tpg) { > + tpg->tv_tpg_vhost_count--; > + target_undepend_item(&tpg->se_tpg.tpg_group.cg_item); > + } > + } > + kfree(vs_tpg); > out: > mutex_unlock(&vs->dev.mutex); > mutex_unlock(&vhost_scsi_mutex); > @@ -1549,6 +1653,12 @@ static void vhost_scsi_flush(struct vhost_scsi *vs) > mutex_lock(&vq->mutex); > vhost_vq_set_backend(vq, NULL); > mutex_unlock(&vq->mutex); > + /* > + * Make sure cmds are not running before tearing them > + * down. > + */ > + vhost_scsi_flush(vs); > + vhost_scsi_destroy_vq_cmds(vq); > } > } > /* > @@ -1842,23 +1952,6 @@ static void vhost_scsi_port_unlink(struct se_portal_group *se_tpg, > mutex_unlock(&vhost_scsi_mutex); > } > > -static void vhost_scsi_free_cmd_map_res(struct se_session *se_sess) > -{ > - struct vhost_scsi_cmd *tv_cmd; > - unsigned int i; > - > - if (!se_sess->sess_cmd_map) > - return; > - > - for (i = 0; i < VHOST_SCSI_DEFAULT_TAGS; i++) { > - tv_cmd = &((struct vhost_scsi_cmd *)se_sess->sess_cmd_map)[i]; > - > - kfree(tv_cmd->tvc_sgl); > - kfree(tv_cmd->tvc_prot_sgl); > - kfree(tv_cmd->tvc_upages); > - } > -} > - > static ssize_t vhost_scsi_tpg_attrib_fabric_prot_type_store( > struct config_item *item, const char *page, size_t count) > { > @@ -1898,45 +1991,6 @@ static ssize_t vhost_scsi_tpg_attrib_fabric_prot_type_show( > NULL, > }; > > -static int vhost_scsi_nexus_cb(struct se_portal_group *se_tpg, > - struct se_session *se_sess, void *p) > -{ > - struct vhost_scsi_cmd *tv_cmd; > - unsigned int i; > - > - for (i = 0; i < VHOST_SCSI_DEFAULT_TAGS; i++) { > - tv_cmd = &((struct vhost_scsi_cmd *)se_sess->sess_cmd_map)[i]; > - > - tv_cmd->tvc_sgl = kcalloc(VHOST_SCSI_PREALLOC_SGLS, > - sizeof(struct scatterlist), > - GFP_KERNEL); > - if (!tv_cmd->tvc_sgl) { > - pr_err("Unable to allocate tv_cmd->tvc_sgl\n"); > - goto out; > - } > - > - tv_cmd->tvc_upages = kcalloc(VHOST_SCSI_PREALLOC_UPAGES, > - sizeof(struct page *), > - GFP_KERNEL); > - if (!tv_cmd->tvc_upages) { > - pr_err("Unable to allocate tv_cmd->tvc_upages\n"); > - goto out; > - } > - > - tv_cmd->tvc_prot_sgl = kcalloc(VHOST_SCSI_PREALLOC_PROT_SGLS, > - sizeof(struct scatterlist), > - GFP_KERNEL); > - if (!tv_cmd->tvc_prot_sgl) { > - pr_err("Unable to allocate tv_cmd->tvc_prot_sgl\n"); > - goto out; > - } > - } > - return 0; > -out: > - vhost_scsi_free_cmd_map_res(se_sess); > - return -ENOMEM; > -} > - > static int vhost_scsi_make_nexus(struct vhost_scsi_tpg *tpg, > const char *name) > { > @@ -1960,12 +2014,9 @@ static int vhost_scsi_make_nexus(struct vhost_scsi_tpg *tpg, > * struct se_node_acl for the vhost_scsi struct se_portal_group with > * the SCSI Initiator port name of the passed configfs group 'name'. > */ > - tv_nexus->tvn_se_sess = target_setup_session(&tpg->se_tpg, > - VHOST_SCSI_DEFAULT_TAGS, > - sizeof(struct vhost_scsi_cmd), > + tv_nexus->tvn_se_sess = target_setup_session(&tpg->se_tpg, 0, 0, > TARGET_PROT_DIN_PASS | TARGET_PROT_DOUT_PASS, > - (unsigned char *)name, tv_nexus, > - vhost_scsi_nexus_cb); > + (unsigned char *)name, tv_nexus, NULL); > if (IS_ERR(tv_nexus->tvn_se_sess)) { > mutex_unlock(&tpg->tv_tpg_mutex); > kfree(tv_nexus); > @@ -2015,7 +2066,6 @@ static int vhost_scsi_drop_nexus(struct vhost_scsi_tpg *tpg) > " %s Initiator Port: %s\n", vhost_scsi_dump_proto_id(tpg->tport), > tv_nexus->tvn_se_sess->se_node_acl->initiatorname); > > - vhost_scsi_free_cmd_map_res(se_sess); > /* > * Release the SCSI I_T Nexus to the emulated vhost Target Port > */ > -- > 1.8.3.1
> On Sep 24, 2020, at 1:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Mon, Sep 21, 2020 at 01:23:03PM -0500, Mike Christie wrote: >> We currently are limited to 256 cmds per session. This leads to problems >> where if the user has increased virtqueue_size to more than 2 or >> cmd_per_lun to more than 256 vhost_scsi_get_tag can fail and the guest >> will get IO errors. >> >> This patch moves the cmd allocation to per vq so we can easily match >> whatever the user has specified for num_queues and >> virtqueue_size/cmd_per_lun. It also makes it easier to control how much >> memory we preallocate. For cases, where perf is not as important and >> we can use the current defaults (1 vq and 128 cmds per vq) memory use >> from preallocate cmds is cut in half. For cases, where we are willing >> to use more memory for higher perf, cmd mem use will now increase as >> the num queues and queue depth increases. >> >> Signed-off-by: Mike Christie <michael.christie@oracle.com> >> --- >> drivers/vhost/scsi.c | 204 ++++++++++++++++++++++++++++++++------------------- >> 1 file changed, 127 insertions(+), 77 deletions(-) >> >> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c >> index b22adf0..13311b8 100644 >> --- a/drivers/vhost/scsi.c >> +++ b/drivers/vhost/scsi.c >> @@ -52,7 +52,6 @@ >> #define VHOST_SCSI_VERSION "v0.1" >> #define VHOST_SCSI_NAMELEN 256 >> #define VHOST_SCSI_MAX_CDB_SIZE 32 >> -#define VHOST_SCSI_DEFAULT_TAGS 256 >> #define VHOST_SCSI_PREALLOC_SGLS 2048 >> #define VHOST_SCSI_PREALLOC_UPAGES 2048 >> #define VHOST_SCSI_PREALLOC_PROT_SGLS 2048 >> @@ -189,6 +188,9 @@ struct vhost_scsi_virtqueue { >> * Writers must also take dev mutex and flush under it. >> */ >> int inflight_idx; >> + struct vhost_scsi_cmd *scsi_cmds; >> + struct sbitmap scsi_tags; >> + int max_cmds; >> }; >> >> struct vhost_scsi { >> @@ -324,7 +326,9 @@ static void vhost_scsi_release_cmd(struct se_cmd *se_cmd) >> { >> struct vhost_scsi_cmd *tv_cmd = container_of(se_cmd, >> struct vhost_scsi_cmd, tvc_se_cmd); >> - struct se_session *se_sess = tv_cmd->tvc_nexus->tvn_se_sess; >> + struct vhost_scsi_virtqueue *svq = container_of(tv_cmd->tvc_vq, >> + struct vhost_scsi_virtqueue, vq); >> + struct vhost_scsi_inflight *inflight = tv_cmd->inflight; >> int i; >> >> if (tv_cmd->tvc_sgl_count) { >> @@ -336,8 +340,8 @@ static void vhost_scsi_release_cmd(struct se_cmd *se_cmd) >> put_page(sg_page(&tv_cmd->tvc_prot_sgl[i])); >> } >> >> - vhost_scsi_put_inflight(tv_cmd->inflight); >> - target_free_tag(se_sess, se_cmd); >> + sbitmap_clear_bit(&svq->scsi_tags, se_cmd->map_tag); >> + vhost_scsi_put_inflight(inflight); >> } >> >> static u32 vhost_scsi_sess_get_index(struct se_session *se_sess) >> @@ -566,13 +570,14 @@ static void vhost_scsi_complete_cmd_work(struct vhost_work *work) >> } >> >> static struct vhost_scsi_cmd * >> -vhost_scsi_get_tag(struct vhost_virtqueue *vq, struct vhost_scsi_tpg *tpg, >> +vhost_scsi_get_cmd(struct vhost_virtqueue *vq, struct vhost_scsi_tpg *tpg, >> unsigned char *cdb, u64 scsi_tag, u16 lun, u8 task_attr, >> u32 exp_data_len, int data_direction) >> { >> + struct vhost_scsi_virtqueue *svq = container_of(vq, >> + struct vhost_scsi_virtqueue, vq); >> struct vhost_scsi_cmd *cmd; >> struct vhost_scsi_nexus *tv_nexus; >> - struct se_session *se_sess; >> struct scatterlist *sg, *prot_sg; >> struct page **pages; >> int tag, cpu; >> @@ -582,15 +587,14 @@ static void vhost_scsi_complete_cmd_work(struct vhost_work *work) >> pr_err("Unable to locate active struct vhost_scsi_nexus\n"); >> return ERR_PTR(-EIO); >> } >> - se_sess = tv_nexus->tvn_se_sess; >> >> - tag = sbitmap_queue_get(&se_sess->sess_tag_pool, &cpu); >> + tag = sbitmap_get(&svq->scsi_tags, 0, false); >> if (tag < 0) { >> pr_err("Unable to obtain tag for vhost_scsi_cmd\n"); >> return ERR_PTR(-ENOMEM); >> } > > > After this change, cpu is uninitialized. I’ve fixed this. We don’t use the cmd’s map_cpu field anymore, so I have deleted it and the cpu var above.
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c index b22adf0..13311b8 100644 --- a/drivers/vhost/scsi.c +++ b/drivers/vhost/scsi.c @@ -52,7 +52,6 @@ #define VHOST_SCSI_VERSION "v0.1" #define VHOST_SCSI_NAMELEN 256 #define VHOST_SCSI_MAX_CDB_SIZE 32 -#define VHOST_SCSI_DEFAULT_TAGS 256 #define VHOST_SCSI_PREALLOC_SGLS 2048 #define VHOST_SCSI_PREALLOC_UPAGES 2048 #define VHOST_SCSI_PREALLOC_PROT_SGLS 2048 @@ -189,6 +188,9 @@ struct vhost_scsi_virtqueue { * Writers must also take dev mutex and flush under it. */ int inflight_idx; + struct vhost_scsi_cmd *scsi_cmds; + struct sbitmap scsi_tags; + int max_cmds; }; struct vhost_scsi { @@ -324,7 +326,9 @@ static void vhost_scsi_release_cmd(struct se_cmd *se_cmd) { struct vhost_scsi_cmd *tv_cmd = container_of(se_cmd, struct vhost_scsi_cmd, tvc_se_cmd); - struct se_session *se_sess = tv_cmd->tvc_nexus->tvn_se_sess; + struct vhost_scsi_virtqueue *svq = container_of(tv_cmd->tvc_vq, + struct vhost_scsi_virtqueue, vq); + struct vhost_scsi_inflight *inflight = tv_cmd->inflight; int i; if (tv_cmd->tvc_sgl_count) { @@ -336,8 +340,8 @@ static void vhost_scsi_release_cmd(struct se_cmd *se_cmd) put_page(sg_page(&tv_cmd->tvc_prot_sgl[i])); } - vhost_scsi_put_inflight(tv_cmd->inflight); - target_free_tag(se_sess, se_cmd); + sbitmap_clear_bit(&svq->scsi_tags, se_cmd->map_tag); + vhost_scsi_put_inflight(inflight); } static u32 vhost_scsi_sess_get_index(struct se_session *se_sess) @@ -566,13 +570,14 @@ static void vhost_scsi_complete_cmd_work(struct vhost_work *work) } static struct vhost_scsi_cmd * -vhost_scsi_get_tag(struct vhost_virtqueue *vq, struct vhost_scsi_tpg *tpg, +vhost_scsi_get_cmd(struct vhost_virtqueue *vq, struct vhost_scsi_tpg *tpg, unsigned char *cdb, u64 scsi_tag, u16 lun, u8 task_attr, u32 exp_data_len, int data_direction) { + struct vhost_scsi_virtqueue *svq = container_of(vq, + struct vhost_scsi_virtqueue, vq); struct vhost_scsi_cmd *cmd; struct vhost_scsi_nexus *tv_nexus; - struct se_session *se_sess; struct scatterlist *sg, *prot_sg; struct page **pages; int tag, cpu; @@ -582,15 +587,14 @@ static void vhost_scsi_complete_cmd_work(struct vhost_work *work) pr_err("Unable to locate active struct vhost_scsi_nexus\n"); return ERR_PTR(-EIO); } - se_sess = tv_nexus->tvn_se_sess; - tag = sbitmap_queue_get(&se_sess->sess_tag_pool, &cpu); + tag = sbitmap_get(&svq->scsi_tags, 0, false); if (tag < 0) { pr_err("Unable to obtain tag for vhost_scsi_cmd\n"); return ERR_PTR(-ENOMEM); } - cmd = &((struct vhost_scsi_cmd *)se_sess->sess_cmd_map)[tag]; + cmd = &svq->scsi_cmds[tag]; sg = cmd->tvc_sgl; prot_sg = cmd->tvc_prot_sgl; pages = cmd->tvc_upages; @@ -1065,11 +1069,11 @@ static void vhost_scsi_submission_work(struct work_struct *work) scsi_command_size(cdb), VHOST_SCSI_MAX_CDB_SIZE); goto err; } - cmd = vhost_scsi_get_tag(vq, tpg, cdb, tag, lun, task_attr, + cmd = vhost_scsi_get_cmd(vq, tpg, cdb, tag, lun, task_attr, exp_data_len + prot_bytes, data_direction); if (IS_ERR(cmd)) { - vq_err(vq, "vhost_scsi_get_tag failed %ld\n", + vq_err(vq, "vhost_scsi_get_cmd failed %ld\n", PTR_ERR(cmd)); goto err; } @@ -1373,6 +1377,83 @@ static void vhost_scsi_flush(struct vhost_scsi *vs) wait_for_completion(&old_inflight[i]->comp); } +static void vhost_scsi_destroy_vq_cmds(struct vhost_virtqueue *vq) +{ + struct vhost_scsi_virtqueue *svq = container_of(vq, + struct vhost_scsi_virtqueue, vq); + struct vhost_scsi_cmd *tv_cmd; + unsigned int i; + + if (!svq->scsi_cmds) + return; + + for (i = 0; i < svq->max_cmds; i++) { + tv_cmd = &svq->scsi_cmds[i]; + + kfree(tv_cmd->tvc_sgl); + kfree(tv_cmd->tvc_prot_sgl); + kfree(tv_cmd->tvc_upages); + } + + sbitmap_free(&svq->scsi_tags); + kfree(svq->scsi_cmds); + svq->scsi_cmds = NULL; +} + +static int vhost_scsi_setup_vq_cmds(struct vhost_virtqueue *vq, int max_cmds) +{ + struct vhost_scsi_virtqueue *svq = container_of(vq, + struct vhost_scsi_virtqueue, vq); + struct vhost_scsi_cmd *tv_cmd; + unsigned int i; + + if (svq->scsi_cmds) + return 0; + + if (sbitmap_init_node(&svq->scsi_tags, max_cmds, -1, GFP_KERNEL, + NUMA_NO_NODE)) + return -ENOMEM; + svq->max_cmds = max_cmds; + + svq->scsi_cmds = kcalloc(max_cmds, sizeof(*tv_cmd), GFP_KERNEL); + if (!svq->scsi_cmds) { + sbitmap_free(&svq->scsi_tags); + return -ENOMEM; + } + + for (i = 0; i < max_cmds; i++) { + tv_cmd = &svq->scsi_cmds[i]; + + tv_cmd->tvc_sgl = kcalloc(VHOST_SCSI_PREALLOC_SGLS, + sizeof(struct scatterlist), + GFP_KERNEL); + if (!tv_cmd->tvc_sgl) { + pr_err("Unable to allocate tv_cmd->tvc_sgl\n"); + goto out; + } + + tv_cmd->tvc_upages = kcalloc(VHOST_SCSI_PREALLOC_UPAGES, + sizeof(struct page *), + GFP_KERNEL); + if (!tv_cmd->tvc_upages) { + pr_err("Unable to allocate tv_cmd->tvc_upages\n"); + goto out; + } + + tv_cmd->tvc_prot_sgl = kcalloc(VHOST_SCSI_PREALLOC_PROT_SGLS, + sizeof(struct scatterlist), + GFP_KERNEL); + if (!tv_cmd->tvc_prot_sgl) { + pr_err("Unable to allocate tv_cmd->tvc_prot_sgl\n"); + goto out; + } + } + return 0; +out: + vhost_scsi_destroy_vq_cmds(vq); + return -ENOMEM; +} + /* * Called from vhost_scsi_ioctl() context to walk the list of available * vhost_scsi_tpg with an active struct vhost_scsi_nexus @@ -1427,10 +1508,9 @@ static void vhost_scsi_flush(struct vhost_scsi *vs) if (!strcmp(tv_tport->tport_name, t->vhost_wwpn)) { if (vs->vs_tpg && vs->vs_tpg[tpg->tport_tpgt]) { - kfree(vs_tpg); mutex_unlock(&tpg->tv_tpg_mutex); ret = -EEXIST; - goto out; + goto undepend; } /* * In order to ensure individual vhost-scsi configfs @@ -1442,9 +1522,8 @@ static void vhost_scsi_flush(struct vhost_scsi *vs) ret = target_depend_item(&se_tpg->tpg_group.cg_item); if (ret) { pr_warn("target_depend_item() failed: %d\n", ret); - kfree(vs_tpg); mutex_unlock(&tpg->tv_tpg_mutex); - goto out; + goto undepend; } tpg->tv_tpg_vhost_count++; tpg->vhost_scsi = vs; @@ -1457,6 +1536,16 @@ static void vhost_scsi_flush(struct vhost_scsi *vs) if (match) { memcpy(vs->vs_vhost_wwpn, t->vhost_wwpn, sizeof(vs->vs_vhost_wwpn)); + + for (i = VHOST_SCSI_VQ_IO; i < VHOST_SCSI_MAX_VQ; i++) { + vq = &vs->vqs[i].vq; + if (!vhost_vq_is_setup(vq)) + continue; + + if (vhost_scsi_setup_vq_cmds(vq, vq->num)) + goto destroy_vq_cmds; + } + for (i = 0; i < VHOST_SCSI_MAX_VQ; i++) { vq = &vs->vqs[i].vq; mutex_lock(&vq->mutex); @@ -1476,7 +1565,22 @@ static void vhost_scsi_flush(struct vhost_scsi *vs) vhost_scsi_flush(vs); kfree(vs->vs_tpg); vs->vs_tpg = vs_tpg; + goto out; +destroy_vq_cmds: + for (i--; i >= VHOST_SCSI_VQ_IO; i--) { + if (!vhost_vq_get_backend(&vs->vqs[i].vq)) + vhost_scsi_destroy_vq_cmds(&vs->vqs[i].vq); + } +undepend: + for (i = 0; i < VHOST_SCSI_MAX_TARGET; i++) { + tpg = vs_tpg[i]; + if (tpg) { + tpg->tv_tpg_vhost_count--; + target_undepend_item(&tpg->se_tpg.tpg_group.cg_item); + } + } + kfree(vs_tpg); out: mutex_unlock(&vs->dev.mutex); mutex_unlock(&vhost_scsi_mutex); @@ -1549,6 +1653,12 @@ static void vhost_scsi_flush(struct vhost_scsi *vs) mutex_lock(&vq->mutex); vhost_vq_set_backend(vq, NULL); mutex_unlock(&vq->mutex); + /* + * Make sure cmds are not running before tearing them + * down. + */ + vhost_scsi_flush(vs); + vhost_scsi_destroy_vq_cmds(vq); } } /* @@ -1842,23 +1952,6 @@ static void vhost_scsi_port_unlink(struct se_portal_group *se_tpg, mutex_unlock(&vhost_scsi_mutex); } -static void vhost_scsi_free_cmd_map_res(struct se_session *se_sess) -{ - struct vhost_scsi_cmd *tv_cmd; - unsigned int i; - - if (!se_sess->sess_cmd_map) - return; - - for (i = 0; i < VHOST_SCSI_DEFAULT_TAGS; i++) { - tv_cmd = &((struct vhost_scsi_cmd *)se_sess->sess_cmd_map)[i]; - - kfree(tv_cmd->tvc_sgl); - kfree(tv_cmd->tvc_prot_sgl); - kfree(tv_cmd->tvc_upages); - } -} - static ssize_t vhost_scsi_tpg_attrib_fabric_prot_type_store( struct config_item *item, const char *page, size_t count) { @@ -1898,45 +1991,6 @@ static ssize_t vhost_scsi_tpg_attrib_fabric_prot_type_show( NULL, }; -static int vhost_scsi_nexus_cb(struct se_portal_group *se_tpg, - struct se_session *se_sess, void *p) -{ - struct vhost_scsi_cmd *tv_cmd; - unsigned int i; - - for (i = 0; i < VHOST_SCSI_DEFAULT_TAGS; i++) { - tv_cmd = &((struct vhost_scsi_cmd *)se_sess->sess_cmd_map)[i]; - - tv_cmd->tvc_sgl = kcalloc(VHOST_SCSI_PREALLOC_SGLS, - sizeof(struct scatterlist), - GFP_KERNEL); - if (!tv_cmd->tvc_sgl) { - pr_err("Unable to allocate tv_cmd->tvc_sgl\n"); - goto out; - } - - tv_cmd->tvc_upages = kcalloc(VHOST_SCSI_PREALLOC_UPAGES, - sizeof(struct page *), - GFP_KERNEL); - if (!tv_cmd->tvc_upages) { - pr_err("Unable to allocate tv_cmd->tvc_upages\n"); - goto out; - } - - tv_cmd->tvc_prot_sgl = kcalloc(VHOST_SCSI_PREALLOC_PROT_SGLS, - sizeof(struct scatterlist), - GFP_KERNEL); - if (!tv_cmd->tvc_prot_sgl) { - pr_err("Unable to allocate tv_cmd->tvc_prot_sgl\n"); - goto out; - } - } - return 0; -out: - vhost_scsi_free_cmd_map_res(se_sess); - return -ENOMEM; -} - static int vhost_scsi_make_nexus(struct vhost_scsi_tpg *tpg, const char *name) { @@ -1960,12 +2014,9 @@ static int vhost_scsi_make_nexus(struct vhost_scsi_tpg *tpg, * struct se_node_acl for the vhost_scsi struct se_portal_group with * the SCSI Initiator port name of the passed configfs group 'name'. */ - tv_nexus->tvn_se_sess = target_setup_session(&tpg->se_tpg, - VHOST_SCSI_DEFAULT_TAGS, - sizeof(struct vhost_scsi_cmd), + tv_nexus->tvn_se_sess = target_setup_session(&tpg->se_tpg, 0, 0, TARGET_PROT_DIN_PASS | TARGET_PROT_DOUT_PASS, - (unsigned char *)name, tv_nexus, - vhost_scsi_nexus_cb); + (unsigned char *)name, tv_nexus, NULL); if (IS_ERR(tv_nexus->tvn_se_sess)) { mutex_unlock(&tpg->tv_tpg_mutex); kfree(tv_nexus); @@ -2015,7 +2066,6 @@ static int vhost_scsi_drop_nexus(struct vhost_scsi_tpg *tpg) " %s Initiator Port: %s\n", vhost_scsi_dump_proto_id(tpg->tport), tv_nexus->tvn_se_sess->se_node_acl->initiatorname); - vhost_scsi_free_cmd_map_res(se_sess); /* * Release the SCSI I_T Nexus to the emulated vhost Target Port */
We currently are limited to 256 cmds per session. This leads to problems where if the user has increased virtqueue_size to more than 2 or cmd_per_lun to more than 256 vhost_scsi_get_tag can fail and the guest will get IO errors. This patch moves the cmd allocation to per vq so we can easily match whatever the user has specified for num_queues and virtqueue_size/cmd_per_lun. It also makes it easier to control how much memory we preallocate. For cases, where perf is not as important and we can use the current defaults (1 vq and 128 cmds per vq) memory use from preallocate cmds is cut in half. For cases, where we are willing to use more memory for higher perf, cmd mem use will now increase as the num queues and queue depth increases. Signed-off-by: Mike Christie <michael.christie@oracle.com> --- drivers/vhost/scsi.c | 204 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 127 insertions(+), 77 deletions(-)