Re: [PATCH v3] scsi: require CAP_SYS_ADMIN to write to procfs interface
On 11/05/2017 01:56 PM, Aleksa Sarai wrote: Previously, the only capability effectively required to operate on the /proc/scsi interface was CAP_DAC_OVERRIDE (or for some other files, having an fsuid of GLOBAL_ROOT_UID was enough). This means that semi-privileged processes could interfere with core components of a system (such as causing a DoS by removing the underlying SCSI device of the host's / mount). An alternative to this patch would be to make the open(2) call fail, if you try to open it write-only or read-write. Not sure which would be preferred (should it be possible to pass /proc/scsi/scsi to a semi-privileged process to write to?). -- Aleksa Sarai Senior Software Engineer (Containers) SUSE Linux GmbH https://www.cyphar.com/
[PATCH v3] scsi: require CAP_SYS_ADMIN to write to procfs interface
Previously, the only capability effectively required to operate on the /proc/scsi interface was CAP_DAC_OVERRIDE (or for some other files, having an fsuid of GLOBAL_ROOT_UID was enough). This means that semi-privileged processes could interfere with core components of a system (such as causing a DoS by removing the underlying SCSI device of the host's / mount). Cc: Cc: "Eric W. Biederman" Signed-off-by: Aleksa Sarai --- drivers/scsi/scsi_proc.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c index 480a597b3877..05d70e200c5f 100644 --- a/drivers/scsi/scsi_proc.c +++ b/drivers/scsi/scsi_proc.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -51,7 +52,10 @@ static ssize_t proc_scsi_host_write(struct file *file, const char __user *buf, struct Scsi_Host *shost = PDE_DATA(file_inode(file)); ssize_t ret = -ENOMEM; char *page; - + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + if (count > PROC_BLOCK_SIZE) return -EOVERFLOW; @@ -313,6 +317,9 @@ static ssize_t proc_scsi_write(struct file *file, const char __user *buf, char *buffer, *p; int err; + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + if (!buf || length > PAGE_SIZE) return -EINVAL; -- 2.14.3
Re: [PATCH V9 3/4] scsi: Align block queue to dma_get_cache_alignment()
Hi Huacai,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.14-rc7]
[cannot apply to next-20171103]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Huacai-Chen/dma-mapping-Rework-dma_get_cache_alignment/20171023-154436
config: m68k-sun3_defconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 4.9.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m68k
All errors (new ones prefixed by >>):
drivers//scsi/scsi_lib.c: In function '__scsi_init_queue':
>> drivers//scsi/scsi_lib.c:2139:2: error: implicit declaration of function
>> 'dma_get_cache_alignment' [-Werror=implicit-function-declaration]
blk_queue_dma_alignment(q, max(4, dma_get_cache_alignment(dev)) - 1);
^
cc1: some warnings being treated as errors
vim +/dma_get_cache_alignment +2139 drivers//scsi/scsi_lib.c
2103
2104 void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
2105 {
2106 struct device *dev = shost->dma_dev;
2107
2108 queue_flag_set_unlocked(QUEUE_FLAG_SCSI_PASSTHROUGH, q);
2109
2110 /*
2111 * this limit is imposed by hardware restrictions
2112 */
2113 blk_queue_max_segments(q, min_t(unsigned short,
shost->sg_tablesize,
2114 SG_MAX_SEGMENTS));
2115
2116 if (scsi_host_prot_dma(shost)) {
2117 shost->sg_prot_tablesize =
2118 min_not_zero(shost->sg_prot_tablesize,
2119 (unsigned
short)SCSI_MAX_PROT_SG_SEGMENTS);
2120 BUG_ON(shost->sg_prot_tablesize < shost->sg_tablesize);
2121 blk_queue_max_integrity_segments(q,
shost->sg_prot_tablesize);
2122 }
2123
2124 blk_queue_max_hw_sectors(q, shost->max_sectors);
2125 blk_queue_bounce_limit(q, scsi_calculate_bounce_limit(shost));
2126 blk_queue_segment_boundary(q, shost->dma_boundary);
2127 dma_set_seg_boundary(dev, shost->dma_boundary);
2128
2129 blk_queue_max_segment_size(q, dma_get_max_seg_size(dev));
2130
2131 if (!shost->use_clustering)
2132 q->limits.cluster = 0;
2133
2134 /*
2135 * set a reasonable default alignment on word/cacheline
boundaries:
2136 * the host and device may alter it using
2137 * blk_queue_update_dma_alignment() later.
2138 */
> 2139 blk_queue_dma_alignment(q, max(4, dma_get_cache_alignment(dev))
> - 1);
2140 }
2141 EXPORT_SYMBOL_GPL(__scsi_init_queue);
2142
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
target-pending/for-next patches
Hi all, Just a friendly email after catching up on patches this week, the majority of those outstanding on the list have been merged into target-pending/for-next. Please see below. For those who submitted patches, please have a look and let me know if anything is else missing. Note there are two exceptions that have been left out for now that I'll be following up with separately. Thus far it's all been either straight-forward bug-fixes, minor cleanups, or small miscellaneous enhancements. AFAICT, nothing looks particularly concerning. Thank you, --nab 17c45b9 iSCSI-target: Use common error handling code in iscsi_decode_text_input() 6eaf69e target/iscsi: Detect conn_cmd_list corruption early cfe2b62 target/iscsi: Fix a race condition in iscsit_add_reject_from_cmd() e1dfb21 target/iscsi: Modify iscsit_do_crypto_hash_buf() prototype de3493a target/iscsi: Fix endianness in an error message 919765e target/iscsi: Use min() in iscsit_dump_data_payload() instead of open-coding it 8d973ab target/iscsi: Define OFFLOAD_BUF_SIZE once c017069 target: Inline transport_put_cmd() d7e595d target: Suppress gcc 7 fallthrough warnings c48e559 target: Move a declaration of a global variable into a header file 0d44374 tcmu: fix double se_cmd completion a271eac target: return SAM_STAT_TASK_SET_FULL for TCM_OUT_OF_RESOURCES 55435ba target: fix ALUA state file path truncation bdc79f0 target: fix PR state file path truncation 1ae0172 cxgbit: Abort the TCP connection in case of data out timeout b849b45 target: Add netlink command reply supported option for each device b5ab697 target/tcmu: Use macro to call container_of in tcmu_cmd_time_out_show c22adc0 tcmu: fix crash when removing the tcmu device a0884d4 iscsi-target: fix memory leak in iscsit_release_discovery_tpg() 12d5a43 iscsi-target: fix memory leak in lio_target_tiqn_addtpg() 24528f0 target:fix condition return in core_pr_dump_initiator_port() a2db857 target: fix match_token option in target_core_configfs.c 79dd6f2 target: add sense code INSUFFICIENT REGISTRATION RESOURCES e437fa3 target: fix double unmap data sg in core_scsi3_emulate_pro_register_and_move() c58a252 target: fix buffer offset in core_scsi3_pri_read_full_status 88fb2fa target: fix null pointer regression in core_tmr_drain_tmr_list 594e25e target/file: Do not return error for UNMAP if length is zero
[PATCH] scsi: Use vzalloc instead of vmalloc/memset
Use vzalloc instead of vmalloc/memset to allocate memory filled with 0
value.
Done using Coccinelle.
Semantic patch used :
@@
expression x,a;
statement S;
@@
- x = vmalloc(a);
+ x = vzalloc(a);
if (x == NULL || ...) S
- memset(x, 0, a);
Signed-off-by: Himanshu Jha
---
drivers/scsi/bfa/bfad.c| 3 +--
drivers/scsi/bfa/bfad_debugfs.c| 8 ++--
drivers/scsi/qla2xxx/qla_bsg.c | 3 +--
drivers/scsi/qla2xxx/tcm_qla2xxx.c | 5 +
drivers/scsi/scsi_debug.c | 6 ++
drivers/scsi/snic/snic_trc.c | 3 +--
6 files changed, 8 insertions(+), 20 deletions(-)
diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
index 5caf5f3..ab91c59 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -610,13 +610,12 @@ bfad_hal_mem_alloc(struct bfad_s *bfad)
/* Iterate through the KVA meminfo queue */
list_for_each(km_qe, &kva_info->qe) {
kva_elem = (struct bfa_mem_kva_s *) km_qe;
- kva_elem->kva = vmalloc(kva_elem->mem_len);
+ kva_elem->kva = vzalloc(kva_elem->mem_len);
if (kva_elem->kva == NULL) {
bfad_hal_mem_release(bfad);
rc = BFA_STATUS_ENOMEM;
goto ext;
}
- memset(kva_elem->kva, 0, kva_elem->mem_len);
}
/* Iterate through the DMA meminfo queue */
diff --git a/drivers/scsi/bfa/bfad_debugfs.c b/drivers/scsi/bfa/bfad_debugfs.c
index 05f5239..349cfe7 100644
--- a/drivers/scsi/bfa/bfad_debugfs.c
+++ b/drivers/scsi/bfa/bfad_debugfs.c
@@ -81,7 +81,7 @@ bfad_debugfs_open_fwtrc(struct inode *inode, struct file
*file)
fw_debug->buffer_len = sizeof(struct bfa_trc_mod_s);
- fw_debug->debug_buffer = vmalloc(fw_debug->buffer_len);
+ fw_debug->debug_buffer = vzalloc(fw_debug->buffer_len);
if (!fw_debug->debug_buffer) {
kfree(fw_debug);
printk(KERN_INFO "bfad[%d]: Failed to allocate fwtrc buffer\n",
@@ -89,8 +89,6 @@ bfad_debugfs_open_fwtrc(struct inode *inode, struct file
*file)
return -ENOMEM;
}
- memset(fw_debug->debug_buffer, 0, fw_debug->buffer_len);
-
spin_lock_irqsave(&bfad->bfad_lock, flags);
rc = bfa_ioc_debug_fwtrc(&bfad->bfa.ioc,
fw_debug->debug_buffer,
@@ -125,7 +123,7 @@ bfad_debugfs_open_fwsave(struct inode *inode, struct file
*file)
fw_debug->buffer_len = sizeof(struct bfa_trc_mod_s);
- fw_debug->debug_buffer = vmalloc(fw_debug->buffer_len);
+ fw_debug->debug_buffer = vzalloc(fw_debug->buffer_len);
if (!fw_debug->debug_buffer) {
kfree(fw_debug);
printk(KERN_INFO "bfad[%d]: Failed to allocate fwsave buffer\n",
@@ -133,8 +131,6 @@ bfad_debugfs_open_fwsave(struct inode *inode, struct file
*file)
return -ENOMEM;
}
- memset(fw_debug->debug_buffer, 0, fw_debug->buffer_len);
-
spin_lock_irqsave(&bfad->bfad_lock, flags);
rc = bfa_ioc_debug_fwsave(&bfad->bfa.ioc,
fw_debug->debug_buffer,
diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
index e3ac707..fdd9059 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -1435,7 +1435,7 @@ qla2x00_optrom_setup(struct bsg_job *bsg_job,
scsi_qla_host_t *vha,
ha->optrom_state = QLA_SREADING;
}
- ha->optrom_buffer = vmalloc(ha->optrom_region_size);
+ ha->optrom_buffer = vzalloc(ha->optrom_region_size);
if (!ha->optrom_buffer) {
ql_log(ql_log_warn, vha, 0x7059,
"Read: Unable to allocate memory for optrom retrieval "
@@ -1445,7 +1445,6 @@ qla2x00_optrom_setup(struct bsg_job *bsg_job,
scsi_qla_host_t *vha,
return -ENOMEM;
}
- memset(ha->optrom_buffer, 0, ha->optrom_region_size);
return 0;
}
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 3f82ea1..aadfeaa 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -1635,16 +1635,13 @@ static int tcm_qla2xxx_init_lport(struct
tcm_qla2xxx_lport *lport)
return rc;
}
- lport->lport_loopid_map = vmalloc(sizeof(struct tcm_qla2xxx_fc_loopid) *
- 65536);
+ lport->lport_loopid_map = vzalloc(sizeof(struct tcm_qla2xxx_fc_loopid)
* 65536);
if (!lport->lport_loopid_map) {
pr_err("Unable to allocate lport->lport_loopid_map of %zu
bytes\n",
sizeof(struct tcm_qla2xxx_fc_loopid) * 65536);
btree_destroy32(&lport->lport_fcport_map);
return -ENOMEM;
}
- memset(lport->lport_loopid_map, 0, sizeof(struct tcm_qla2xxx_fc_loopid)
- * 65536);
pr_debug("qla2xxx: Allocated lport_loopid_map of %zu bytes\n",
Re: [PATCH v2] scsi: require CAP_SYS_ADMIN to write to procfs interface
On 11/04/2017 01:26 PM, Aleksa Sarai wrote: > Previously, the only capability effectively required to operate on the > /proc/scsi interface was CAP_DAC_OVERRIDE (or for some other files, > having an fsuid of GLOBAL_ROOT_UID was enough). This means that > semi-privileged processes could interfere with core components of a > system (such as causing a DoS by removing the underlying SCSI device of > the host's / mount). > > Cc: > Cc: "Eric W. Biederman" > Signed-off-by: Aleksa Sarai There should be an #include somewhere... even if some other header file is dragging it in (which it must be), the preferred style is to always explicitly #include header files that are used. > --- > drivers/scsi/scsi_proc.c | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c > index 480a597b3877..8c891ab16b11 100644 > --- a/drivers/scsi/scsi_proc.c > +++ b/drivers/scsi/scsi_proc.c > @@ -51,7 +51,10 @@ static ssize_t proc_scsi_host_write(struct file *file, > const char __user *buf, > struct Scsi_Host *shost = PDE_DATA(file_inode(file)); > ssize_t ret = -ENOMEM; > char *page; > - > + > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + > if (count > PROC_BLOCK_SIZE) > return -EOVERFLOW; > > @@ -313,6 +316,9 @@ static ssize_t proc_scsi_write(struct file *file, const > char __user *buf, > char *buffer, *p; > int err; > > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + > if (!buf || length > PAGE_SIZE) > return -EINVAL; > > -- ~Randy
[PATCH v2] scsi: require CAP_SYS_ADMIN to write to procfs interface
Previously, the only capability effectively required to operate on the /proc/scsi interface was CAP_DAC_OVERRIDE (or for some other files, having an fsuid of GLOBAL_ROOT_UID was enough). This means that semi-privileged processes could interfere with core components of a system (such as causing a DoS by removing the underlying SCSI device of the host's / mount). Cc: Cc: "Eric W. Biederman" Signed-off-by: Aleksa Sarai --- drivers/scsi/scsi_proc.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c index 480a597b3877..8c891ab16b11 100644 --- a/drivers/scsi/scsi_proc.c +++ b/drivers/scsi/scsi_proc.c @@ -51,7 +51,10 @@ static ssize_t proc_scsi_host_write(struct file *file, const char __user *buf, struct Scsi_Host *shost = PDE_DATA(file_inode(file)); ssize_t ret = -ENOMEM; char *page; - + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + if (count > PROC_BLOCK_SIZE) return -EOVERFLOW; @@ -313,6 +316,9 @@ static ssize_t proc_scsi_write(struct file *file, const char __user *buf, char *buffer, *p; int err; + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + if (!buf || length > PAGE_SIZE) return -EINVAL; -- 2.14.3
Re: [PATCH] scsi: require CAP_SYS_ADMIN to write to procfs interface
Previously, the only capability effectively required to operate on the /proc/scsi interface was CAP_DAC_OVERRIDE (or for some other files, having an fsuid of GLOBAL_ROOT_UID was enough). This means that semi-privileged processes could interfere with core components of a system (such as causing a DoS by removing the underlying SCSI device of the host's / mount). Cc: Cc: "Eric W. Biederman" Signed-off-by: Aleksa Sarai --- drivers/scsi/scsi_proc.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c index 480a597b3877..486aedce2f05 100644 --- a/drivers/scsi/scsi_proc.c +++ b/drivers/scsi/scsi_proc.c @@ -51,7 +51,10 @@ static ssize_t proc_scsi_host_write(struct file *file, const char __user *buf, struct Scsi_Host *shost = PDE_DATA(file_inode(file)); ssize_t ret = -ENOMEM; char *page; - + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM did that build without a trailing ';' ? D'oh. Re-sent, thanks. -- Aleksa Sarai Senior Software Engineer (Containers) SUSE Linux GmbH https://www.cyphar.com/
Re: [PATCH] scsi: require CAP_SYS_ADMIN to write to procfs interface
On 11/04/2017 11:59 AM, Aleksa Sarai wrote: > Previously, the only capability effectively required to operate on the > /proc/scsi interface was CAP_DAC_OVERRIDE (or for some other files, > having an fsuid of GLOBAL_ROOT_UID was enough). This means that > semi-privileged processes could interfere with core components of a > system (such as causing a DoS by removing the underlying SCSI device of > the host's / mount). > > Cc: > Cc: "Eric W. Biederman" > Signed-off-by: Aleksa Sarai > --- > drivers/scsi/scsi_proc.c | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c > index 480a597b3877..486aedce2f05 100644 > --- a/drivers/scsi/scsi_proc.c > +++ b/drivers/scsi/scsi_proc.c > @@ -51,7 +51,10 @@ static ssize_t proc_scsi_host_write(struct file *file, > const char __user *buf, > struct Scsi_Host *shost = PDE_DATA(file_inode(file)); > ssize_t ret = -ENOMEM; > char *page; > - > + > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM did that build without a trailing ';' ? > + > if (count > PROC_BLOCK_SIZE) > return -EOVERFLOW; > > @@ -313,6 +316,9 @@ static ssize_t proc_scsi_write(struct file *file, const > char __user *buf, > char *buffer, *p; > int err; > > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + > if (!buf || length > PAGE_SIZE) > return -EINVAL; > > -- ~Randy
[PATCH] scsi: require CAP_SYS_ADMIN to write to procfs interface
Previously, the only capability effectively required to operate on the /proc/scsi interface was CAP_DAC_OVERRIDE (or for some other files, having an fsuid of GLOBAL_ROOT_UID was enough). This means that semi-privileged processes could interfere with core components of a system (such as causing a DoS by removing the underlying SCSI device of the host's / mount). Cc: Cc: "Eric W. Biederman" Signed-off-by: Aleksa Sarai --- drivers/scsi/scsi_proc.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c index 480a597b3877..486aedce2f05 100644 --- a/drivers/scsi/scsi_proc.c +++ b/drivers/scsi/scsi_proc.c @@ -51,7 +51,10 @@ static ssize_t proc_scsi_host_write(struct file *file, const char __user *buf, struct Scsi_Host *shost = PDE_DATA(file_inode(file)); ssize_t ret = -ENOMEM; char *page; - + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM + if (count > PROC_BLOCK_SIZE) return -EOVERFLOW; @@ -313,6 +316,9 @@ static ssize_t proc_scsi_write(struct file *file, const char __user *buf, char *buffer, *p; int err; + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + if (!buf || length > PAGE_SIZE) return -EINVAL; -- 2.14.3
[PATCH] blk-mq: don't handle failure in .get_budget
It is enough to just check if we can get the budget via .get_budget().
And we don't need to deal with device state change in .get_budget().
For SCSI, one issue to be fixed is that we have to call scsi_mq_uninit_cmd()
to free allocated ressources if SCSI device fails to handle the request. And
it isn't enough to simply call blk_mq_end_request() to do that if this request
is marked as RQF_DONTPREP.
Fixes: 0df21c86bdbf(scsi: implement .get_budget and .put_budget for blk-mq)
Signed-off-by: Ming Lei
---
block/blk-mq-sched.c| 14 ++
block/blk-mq.c | 17 -
block/blk-mq.h | 5 ++---
drivers/scsi/scsi_lib.c | 11 +++
include/linux/blk-mq.h | 2 +-
5 files changed, 12 insertions(+), 37 deletions(-)
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 7775f6b12fa9..13a27d4d1671 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -94,23 +94,18 @@ static void blk_mq_do_dispatch_sched(struct blk_mq_hw_ctx
*hctx)
do {
struct request *rq;
- blk_status_t ret;
if (e->type->ops.mq.has_work &&
!e->type->ops.mq.has_work(hctx))
break;
- ret = blk_mq_get_dispatch_budget(hctx);
- if (ret == BLK_STS_RESOURCE)
+ if (!blk_mq_get_dispatch_budget(hctx))
break;
rq = e->type->ops.mq.dispatch_request(hctx);
if (!rq) {
blk_mq_put_dispatch_budget(hctx);
break;
- } else if (ret != BLK_STS_OK) {
- blk_mq_end_request(rq, ret);
- continue;
}
/*
@@ -146,22 +141,17 @@ static void blk_mq_do_dispatch_ctx(struct blk_mq_hw_ctx
*hctx)
do {
struct request *rq;
- blk_status_t ret;
if (!sbitmap_any_bit_set(&hctx->ctx_map))
break;
- ret = blk_mq_get_dispatch_budget(hctx);
- if (ret == BLK_STS_RESOURCE)
+ if (!blk_mq_get_dispatch_budget(hctx))
break;
rq = blk_mq_dequeue_from_ctx(hctx, ctx);
if (!rq) {
blk_mq_put_dispatch_budget(hctx);
break;
- } else if (ret != BLK_STS_OK) {
- blk_mq_end_request(rq, ret);
- continue;
}
/*
diff --git a/block/blk-mq.c b/block/blk-mq.c
index e4d2490f4e7e..f4e1d8447b5d 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1134,13 +1134,8 @@ bool blk_mq_dispatch_rq_list(struct request_queue *q,
struct list_head *list,
}
}
- if (!got_budget) {
- ret = blk_mq_get_dispatch_budget(hctx);
- if (ret == BLK_STS_RESOURCE)
- break;
- if (ret != BLK_STS_OK)
- goto fail_rq;
- }
+ if (!got_budget && !blk_mq_get_dispatch_budget(hctx))
+ break;
list_del_init(&rq->queuelist);
@@ -1167,7 +1162,6 @@ bool blk_mq_dispatch_rq_list(struct request_queue *q,
struct list_head *list,
break;
}
- fail_rq:
if (unlikely(ret != BLK_STS_OK)) {
errors++;
blk_mq_end_request(rq, BLK_STS_IOERR);
@@ -1639,12 +1633,10 @@ static void __blk_mq_try_issue_directly(struct
blk_mq_hw_ctx *hctx,
if (!blk_mq_get_driver_tag(rq, NULL, false))
goto insert;
- ret = blk_mq_get_dispatch_budget(hctx);
- if (ret == BLK_STS_RESOURCE) {
+ if (!blk_mq_get_dispatch_budget(hctx)) {
blk_mq_put_driver_tag(rq);
goto insert;
- } else if (ret != BLK_STS_OK)
- goto fail_rq;
+ }
new_cookie = request_to_qc_t(hctx, rq);
@@ -1662,7 +1654,6 @@ static void __blk_mq_try_issue_directly(struct
blk_mq_hw_ctx *hctx,
__blk_mq_requeue_request(rq);
goto insert;
default:
- fail_rq:
*cookie = BLK_QC_T_NONE;
blk_mq_end_request(rq, ret);
return;
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 522b420dedc0..f97aceff76e9 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -147,14 +147,13 @@ static inline void blk_mq_put_dispatch_budget(struct
blk_mq_hw_ctx *hctx)
q->mq_ops->put_budget(hctx);
}
-static inline blk_status_t blk_mq_get_dispatch_budget(
- struct blk_mq_hw_ctx *hctx)
+static inline bool blk_mq_get_dispatch_budget(struct blk_mq_hw_ctx *hctx)
{
struct request_queue *q = hctx->queue;
if (q->mq_ops->get_budget)
return q->mq_o
Re: [PATCH] SCSI: don't get target/host busy_count in scsi_mq_get_budget()
On 11/03/2017 07:55 PM, Ming Lei wrote: > It is very expensive to atomic_inc/atomic_dec the host wide counter of > host->busy_count, and it should have been avoided via blk-mq's mechanism > of getting driver tag, which uses the more efficient way of sbitmap queue. > > Also we don't check atomic_read(&sdev->device_busy) in scsi_mq_get_budget() > and don't run queue if the counter becomes zero, so IO hang may be caused > if all requests are completed just before the current SCSI device > is added to shost->starved_list. This looks like an improvement. I have added it for 4.15. Bart, does this fix your hang? -- Jens Axboe
[PATCH 4/4] target: Delete an error message for a failed memory allocation in two functions
From: Markus Elfring
Date: Sat, 4 Nov 2017 13:43:22 +0100
Omit an extra message for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/target/target_core_xcopy.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/target/target_core_xcopy.c
b/drivers/target/target_core_xcopy.c
index 1519e399392b..c2b3d3158208 100644
--- a/drivers/target/target_core_xcopy.c
+++ b/drivers/target/target_core_xcopy.c
@@ -681,10 +681,9 @@ static int target_xcopy_read_source(
bool remote_port = (xop->op_origin == XCOL_DEST_RECV_OP);
xpt_cmd = kzalloc(sizeof(*xpt_cmd), GFP_KERNEL);
- if (!xpt_cmd) {
- pr_err("Unable to allocate xcopy_pt_cmd\n");
+ if (!xpt_cmd)
return -ENOMEM;
- }
+
init_completion(&xpt_cmd->xpt_passthrough_sem);
se_cmd = &xpt_cmd->se_cmd;
@@ -743,10 +742,9 @@ static int target_xcopy_write_destination(
bool remote_port = (xop->op_origin == XCOL_SOURCE_RECV_OP);
xpt_cmd = kzalloc(sizeof(*xpt_cmd), GFP_KERNEL);
- if (!xpt_cmd) {
- pr_err("Unable to allocate xcopy_pt_cmd\n");
+ if (!xpt_cmd)
return -ENOMEM;
- }
+
init_completion(&xpt_cmd->xpt_passthrough_sem);
se_cmd = &xpt_cmd->se_cmd;
--
2.15.0
[PATCH 3/4] target: Improve a size determination in three functions
From: Markus Elfring
Date: Sat, 4 Nov 2017 13:35:08 +0100
Replace the specification of data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/target/target_core_xcopy.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/target/target_core_xcopy.c
b/drivers/target/target_core_xcopy.c
index 46cac36014df..1519e399392b 100644
--- a/drivers/target/target_core_xcopy.c
+++ b/drivers/target/target_core_xcopy.c
@@ -680,7 +680,7 @@ static int target_xcopy_read_source(
unsigned char cdb[16];
bool remote_port = (xop->op_origin == XCOL_DEST_RECV_OP);
- xpt_cmd = kzalloc(sizeof(struct xcopy_pt_cmd), GFP_KERNEL);
+ xpt_cmd = kzalloc(sizeof(*xpt_cmd), GFP_KERNEL);
if (!xpt_cmd) {
pr_err("Unable to allocate xcopy_pt_cmd\n");
return -ENOMEM;
@@ -742,7 +742,7 @@ static int target_xcopy_write_destination(
unsigned char cdb[16];
bool remote_port = (xop->op_origin == XCOL_SOURCE_RECV_OP);
- xpt_cmd = kzalloc(sizeof(struct xcopy_pt_cmd), GFP_KERNEL);
+ xpt_cmd = kzalloc(sizeof(*xpt_cmd), GFP_KERNEL);
if (!xpt_cmd) {
pr_err("Unable to allocate xcopy_pt_cmd\n");
return -ENOMEM;
@@ -1010,7 +1010,7 @@ sense_reason_t target_do_xcopy(struct se_cmd *se_cmd)
return TCM_PARAMETER_LIST_LENGTH_ERROR;
}
- xop = kzalloc(sizeof(struct xcopy_op), GFP_KERNEL);
+ xop = kzalloc(sizeof(*xop), GFP_KERNEL);
if (!xop)
goto err;
xop->xop_se_cmd = se_cmd;
--
2.15.0
[PATCH 2/4] target: Combine two condition checks into one statement in target_xcopy_do_work()
From: Markus Elfring Date: Sat, 4 Nov 2017 13:26:46 +0100 The same label was used by goto statements after two condition checks. Thus use a single statement instead. Signed-off-by: Markus Elfring --- drivers/target/target_core_xcopy.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index b06877c57765..46cac36014df 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -802,10 +802,8 @@ static void target_xcopy_do_work(struct work_struct *work) int rc = 0; unsigned short nolb, cur_nolb, max_nolb, copied_nolb = 0; - if (target_parse_xcopy_cmd(xop) != TCM_NO_SENSE) - goto err_free_xop; - - if (WARN_ON_ONCE(!xop->src_dev) || WARN_ON_ONCE(!xop->dst_dev)) + if (target_parse_xcopy_cmd(xop) != TCM_NO_SENSE || + WARN_ON_ONCE(!xop->src_dev) || WARN_ON_ONCE(!xop->dst_dev)) goto err_free_xop; src_dev = xop->src_dev; -- 2.15.0
[PATCH 1/4] target: Use common error handling code in three functions
From: Markus Elfring
Date: Sat, 4 Nov 2017 13:13:20 +0100
Adjust jump targets so that a bit of exception handling can be better
reused at the end of these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/target/target_core_xcopy.c | 49 +++---
1 file changed, 25 insertions(+), 24 deletions(-)
diff --git a/drivers/target/target_core_xcopy.c
b/drivers/target/target_core_xcopy.c
index 9ee89e00cd77..b06877c57765 100644
--- a/drivers/target/target_core_xcopy.c
+++ b/drivers/target/target_core_xcopy.c
@@ -701,11 +701,8 @@ static int target_xcopy_read_source(
rc = target_xcopy_setup_pt_cmd(xpt_cmd, xop, src_dev, &cdb[0],
remote_port, true);
- if (rc < 0) {
- ec_cmd->scsi_status = xpt_cmd->se_cmd.scsi_status;
- transport_generic_free_cmd(se_cmd, 0);
- return rc;
- }
+ if (rc < 0)
+ goto set_status;
xop->xop_data_sg = se_cmd->t_data_sg;
xop->xop_data_nents = se_cmd->t_data_nents;
@@ -713,11 +710,9 @@ static int target_xcopy_read_source(
" memory\n", xop->xop_data_sg, xop->xop_data_nents);
rc = target_xcopy_issue_pt_cmd(xpt_cmd);
- if (rc < 0) {
- ec_cmd->scsi_status = xpt_cmd->se_cmd.scsi_status;
- transport_generic_free_cmd(se_cmd, 0);
- return rc;
- }
+ if (rc < 0)
+ goto set_status;
+
/*
* Clear off the allocated t_data_sg, that has been saved for
* zero-copy WRITE submission reuse in struct xcopy_op..
@@ -726,6 +721,11 @@ static int target_xcopy_read_source(
se_cmd->t_data_nents = 0;
return 0;
+
+set_status:
+ ec_cmd->scsi_status = xpt_cmd->se_cmd.scsi_status;
+ transport_generic_free_cmd(se_cmd, 0);
+ return rc;
}
static int target_xcopy_write_destination(
@@ -775,19 +775,21 @@ static int target_xcopy_write_destination(
src_cmd->t_data_sg = xop->xop_data_sg;
src_cmd->t_data_nents = xop->xop_data_nents;
- transport_generic_free_cmd(se_cmd, 0);
- return rc;
+ goto err_free_cmd;
}
rc = target_xcopy_issue_pt_cmd(xpt_cmd);
if (rc < 0) {
ec_cmd->scsi_status = xpt_cmd->se_cmd.scsi_status;
se_cmd->se_cmd_flags &= ~SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC;
- transport_generic_free_cmd(se_cmd, 0);
- return rc;
+ goto err_free_cmd;
}
return 0;
+
+err_free_cmd:
+ transport_generic_free_cmd(se_cmd, 0);
+ return rc;
}
static void target_xcopy_do_work(struct work_struct *work)
@@ -801,10 +803,10 @@ static void target_xcopy_do_work(struct work_struct *work)
unsigned short nolb, cur_nolb, max_nolb, copied_nolb = 0;
if (target_parse_xcopy_cmd(xop) != TCM_NO_SENSE)
- goto err_free;
+ goto err_free_xop;
if (WARN_ON_ONCE(!xop->src_dev) || WARN_ON_ONCE(!xop->dst_dev))
- goto err_free;
+ goto err_free_xop;
src_dev = xop->src_dev;
dst_dev = xop->dst_dev;
@@ -835,7 +837,7 @@ static void target_xcopy_do_work(struct work_struct *work)
rc = target_xcopy_read_source(ec_cmd, xop, src_dev, src_lba,
cur_nolb);
if (rc < 0)
- goto out;
+ goto err_undepend_device;
src_lba += cur_nolb;
pr_debug("target_xcopy_do_work: Incremented READ src_lba to
%llu\n",
@@ -846,10 +848,8 @@ static void target_xcopy_do_work(struct work_struct *work)
rc = target_xcopy_write_destination(ec_cmd, xop, dst_dev,
dst_lba, cur_nolb);
- if (rc < 0) {
- transport_generic_free_cmd(&xop->src_pt_cmd->se_cmd, 0);
- goto out;
- }
+ if (rc < 0)
+ goto err_free_cmd;
dst_lba += cur_nolb;
pr_debug("target_xcopy_do_work: Incremented WRITE dst_lba to
%llu\n",
@@ -876,10 +876,11 @@ static void target_xcopy_do_work(struct work_struct *work)
target_complete_cmd(ec_cmd, SAM_STAT_GOOD);
return;
-out:
+err_free_cmd:
+ transport_generic_free_cmd(&xop->src_pt_cmd->se_cmd, 0);
+err_undepend_device:
xcopy_pt_undepend_remotedev(xop);
-
-err_free:
+err_free_xop:
kfree(xop);
/*
* Don't override an error scsi status if it has already been set
--
2.15.0
[PATCH 0/4] target: Adjustments for four function implementations
From: Markus Elfring Date: Sat, 4 Nov 2017 14:09:09 +0100 A few update suggestions were taken into account from static source code analysis. Markus Elfring (4): Use common error handling code in three functions Combine two condition checks into one statement in target_xcopy_do_work() Improve a size determination in three functions Delete an error message for a failed memory allocation in two functions drivers/target/target_core_xcopy.c | 69 ++ 1 file changed, 33 insertions(+), 36 deletions(-) -- 2.15.0
