Re: [PATCH V3 08/24] aacraid: Added support for response path
On Mon, Jan 30, 2017 at 08:16:48PM +, Raghava Aditya Renukunta wrote:
>
>
> > -Original Message-
> > From: Johannes Thumshirn [mailto:jthumsh...@suse.de]
> > Sent: Monday, January 30, 2017 1:50 AM
> > To: Raghava Aditya Renukunta
> >
> > Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com; linux-
> > s...@vger.kernel.org; Dave Carroll ; Gana
> > Sridaran ; Scott Benesh
> >
> > Subject: Re: [PATCH V3 08/24] aacraid: Added support for response path
> >
> > EXTERNAL EMAIL
> >
> >
> > On Fri, Jan 27, 2017 at 11:28:37AM -0800, Raghava Aditya Renukunta wrote:
> > > This patch enables the driver to actually process the I/O, or srb replies
> > > from adapter. In addition to any HBA1000 or SmartIOC2000 adapter events.
> > >
> > > Signed-off-by: Raghava Aditya Renukunta
> >
> > > Signed-off-by: Dave Carroll
> > >
> > > ---
> >
> > Sorry for not completely reviewing the patchset before you reposted.
> >
> > > @@ -475,16 +475,26 @@ int aac_get_containers(struct aac_dev *dev)
> > >
> > > if (maximum_num_containers < MAXIMUM_NUM_CONTAINERS)
> > > maximum_num_containers = MAXIMUM_NUM_CONTAINERS;
> > > - fsa_dev_ptr = kzalloc(sizeof(*fsa_dev_ptr) *
> > maximum_num_containers,
> > > - GFP_KERNEL);
> > > - if (!fsa_dev_ptr)
> > > - return -ENOMEM;
> > > + if ((dev->fsa_dev == NULL) ||
> > > + (dev->maximum_num_containers != maximum_num_containers))
> > {
> > > +
> > > + fsa_dev_ptr = dev->fsa_dev;
> >
> > Comparison has precedence over logical OR. See
> > http://en.cppreference.com/w/c/language/operator_precedence
>
> Acknowledged. , I will remove the parentheses.
> The idea was to make it easier to read the code , if there were enclosed in
> Parenthesis (I dabbled a bit in lisp , and for me brackets makes it easier
> to read).
I think it makes it a bit less obvious and yes it kinda looks like LISP,
you're right.
>
> >
> > >
> > > - dev->fsa_dev = fsa_dev_ptr;
> > > - dev->maximum_num_containers = maximum_num_containers;
> > > + dev->fsa_dev = kzalloc(sizeof(*fsa_dev_ptr) *
> > > + maximum_num_containers, GFP_KERNEL);
> >
> > kcalloc()?
>
> Yes kcalloc makes sense, but then an additional mem set would be required.
> Unless there is a kzcalloc or kczalloc?
No kcalloc() (and it's user-space counterpart calloc() give you zeroed out
memory). If you don't want a zero-fill kmalloc_array() would be the way to go.
Thanks for taking care of that,
Johannes
--
Johannes Thumshirn Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
scsi: BUG in scsi_init_io
Hello,
The following program triggers BUG in scsi_init_io:
kernel BUG at drivers/scsi/scsi_lib.c:1043!
invalid opcode: [#1] SMP KASAN
Modules linked in:
CPU: 0 PID: 2899 Comm: a.out Not tainted 4.10.0-rc5+ #201
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: 88006baa4500 task.stack: 880069788000
RIP: 0010:scsi_init_io+0x2a3/0x3d0 drivers/scsi/scsi_lib.c:1043
RSP: 0018:88006978e500 EFLAGS: 00010097
RAX: 88006baa4500 RBX: 8800683f2c00 RCX:
RDX: RSI: 88006afc79c0 RDI: 88006afc7aa0
RBP: 88006978e548 R08: 0001 R09: 0001
R10: R11: 0001 R12: 8800683f2c00
R13: 8800683f2d40 R14: 880068b335d8 R15: 88006afc79c0
FS: 02572880() GS:88006d00() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 20c03000 CR3: 6b04f000 CR4: 001406f0
Call Trace:
scsi_setup_blk_pc_cmnd drivers/scsi/scsi_lib.c:1153 [inline]
scsi_setup_cmnd+0x13b/0x5d0 drivers/scsi/scsi_lib.c:1201
scsi_prep_fn+0x375/0x610 drivers/scsi/scsi_lib.c:1313
blk_peek_request+0x686/0xcc0 block/blk-core.c:2382
scsi_request_fn+0x19e/0x1d70 drivers/scsi/scsi_lib.c:1709
__blk_run_queue_uncond block/blk-core.c:325 [inline]
__blk_run_queue+0xc5/0x130 block/blk-core.c:343
blk_execute_rq_nowait+0x304/0x480 block/blk-exec.c:83
sg_common_write.isra.22+0x10b8/0x1b00 drivers/scsi/sg.c:804
sg_new_write.isra.25+0x5e7/0x990 drivers/scsi/sg.c:747
sg_ioctl+0x244b/0x39a0 drivers/scsi/sg.c:855
vfs_ioctl fs/ioctl.c:43 [inline]
do_vfs_ioctl+0x1bf/0x1790 fs/ioctl.c:683
SYSC_ioctl fs/ioctl.c:698 [inline]
SyS_ioctl+0x8f/0xc0 fs/ioctl.c:689
entry_SYSCALL_64_fastpath+0x1f/0xc2
RIP: 0033:0x434da9
RSP: 002b:7ffd20ad81a8 EFLAGS: 0246 ORIG_RAX: 0010
RAX: ffda RBX: 00401b00 RCX: 00434da9
RDX: 20007000 RSI: 2285 RDI: 0003
RBP: R08: R09:
R10: R11: 0246 R12: 00010004
R13: 00401b00 R14: 00401b90 R15:
Code: df 49 c7 85 e8 00 00 00 00 00 00 00 e8 37 f9 fd ff 48 8b 7d c8
48 81 c7 38 02 00 00 e8 f7 8a f0 ff e9 70 ff ff ff e8 fd f1 a5 fe <0f>
0b e8 f6 f1 a5 fe 48 8b 3d 8f a8 f1 03 be 20 80 08 01 e8 05
RIP: scsi_init_io+0x2a3/0x3d0 drivers/scsi/scsi_lib.c:1043 RSP: 88006978e500
---[ end trace 08eb8aec64134983 ]---
// autogenerated by syzkaller (http://github.com/google/syzkaller)
#define _GNU_SOURCE
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
int main()
{
syscall(__NR_mmap, 0x2000ul, 0xfcul, 0x3ul,
0x32ul, 0xul, 0x800ul, 0,
0, 0);
int fd = syscall(__NR_open, "/dev/sg0", 0x0ul, 0, 0, 0, 0, 0, 0);
(*(uint64_t*)0x2000f000 = (uint64_t)0x20003fc0);
(*(uint64_t*)0x2000f008 = (uint64_t)0x20003fc0);
(*(uint64_t*)0x2000f010 = (uint64_t)0x2000f000);
(*(uint64_t*)0x20003fc0 = (uint64_t)0x7);
(*(uint32_t*)0x20003fc8 = (uint32_t)0x0);
(*(uint32_t*)0x20003fcc = (uint32_t)0x0);
(*(uint16_t*)0x20003fd0 = (uint16_t)0x0);
(*(uint16_t*)0x20003fd2 = (uint16_t)0x0);
(*(uint32_t*)0x20003fd4 = fd);
(*(uint64_t*)0x20003fd8 = (uint64_t)0x2000f000);
(*(uint64_t*)0x20003fe0 = (uint64_t)0x0);
(*(uint64_t*)0x20003fe8 = (uint64_t)0x0);
(*(uint64_t*)0x20003ff0 = (uint64_t)0x20007000);
(*(uint32_t*)0x20003ff8 = (uint32_t)0x1);
(*(uint32_t*)0x20003ffc = fd);
(*(uint64_t*)0x20007000 = (uint64_t)0x0);
(*(uint32_t*)0x20007008 = (uint32_t)0x409);
(*(uint32_t*)0x2000700c = (uint32_t)0x1);
(*(uint64_t*)0x20007010 = (uint64_t)0x20c03000);
(*(uint64_t*)0x20007018 = (uint64_t)0x2000f000);
(memcpy((void*)0x2000f000,
"\x83\x3c\x35\x2f\xff\x00\x00\x00\x00\x00\x00\x7f"
"\xff\x00\x00\x00\x00\x82\x7a\x7f\xa3\xcc\x90\xbe"
"\x3d\xf8\x43\x81\xc5\x02",
30));
(*(uint64_t*)0x20003fc0 = (uint64_t)0x649e);
(*(uint32_t*)0x20003fc8 = (uint32_t)0x0);
(*(uint32_t*)0x20003fcc = (uint32_t)0x0);
(*(uint16_t*)0x20003fd0 = (uint16_t)0x0);
(*(uint16_t*)0x20003fd2 = (uint16_t)0x1);
(*(uint32_t*)0x20003fd4 = fd);
(*(uint64_t*)0x20003fd8 = (uint64_t)0x2000f000);
(*(uint64_t*)0x20003fe0 = (uint64_t)0xaf);
(*(uint64_t*)0x20003fe8 = (uint64_t)0x);
(*(uint64_t*)0x20003ff0 = (uint64_t)0x20378fb0);
(*(uint32_t*)0x20003ff8 = (uint32_t)0x1);
(*(uint32_t*)0x20003ffc = fd);
(memcpy(
(void*)0x2000f000,
"\x05\x60\x1e\xc6\x2e\x5d\xdf\xc8\xcd\xd1\xd8\x2c\x37\x5f\xa2\x63"
"\xec\x39\x1d\x03\xf8\xfd\x1d\xe8\xf6\xfd\x84\x33\xfb\x7a\xd4\xfb"
"\xaf\x30\x6a\x0a\x2a\x43\xd8\xbb\x41\xcc\x7a\x74\x17\xe8\x66\x62"
"\x40\x17\x4d\x14\x34\x9e\x1b\x3b\x43\x50\x22\x95\x54\x05\x1e\xfd"
"\x8f\x9e\xb6\xe8\x93\x5c\xee\x4
Re: scsi: BUG in scsi_init_io
On Tue, Jan 31, 2017 at 09:55:52AM +0100, Dmitry Vyukov wrote: > Hello, > > The following program triggers BUG in scsi_init_io: Well crashing a machine just because of an empty dma transfer is a bit harsh, isn't it? >From 86e6fa5f618fe588b98e923e032f33e075fcd4f4 Mon Sep 17 00:00:00 2001 From: Johannes Thumshirn Date: Tue, 31 Jan 2017 10:16:00 +0100 Subject: [PATCH] scsi: don't BUG_ON() empty DMA transfers Don't crash the machine just because of an empty transfer. Use WARN_ON() combined with returning an error. Signed-off-by: Johannes Thumshirn --- drivers/scsi/scsi_lib.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index e9e1e14..414588a 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1040,7 +1040,8 @@ int scsi_init_io(struct scsi_cmnd *cmd) bool is_mq = (rq->mq_ctx != NULL); int error; - BUG_ON(!blk_rq_nr_phys_segments(rq)); + if (WARN_ON(!blk_rq_nr_phys_segments(rq))) + return -EINVAL; error = scsi_init_sgtable(rq, &cmd->sdb); if (error) -- 2.10.2 -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/10] mpt3sas: implement _dechain_st()
Split off _dechain_st() as separate function.
No functional change.
Signed-off-by: Hannes Reinecke
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 09d0008..120b317 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -2365,6 +2365,19 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
return smid;
}
+static void
+_dechain_st(struct MPT3SAS_ADAPTER *ioc, struct scsiio_tracker *st)
+{
+ struct chain_tracker *chain_req;
+
+ while (!list_empty(&st->chain_list)) {
+ chain_req = list_first_entry(&st->chain_list,
+struct chain_tracker,
+tracker_list);
+ list_move(&chain_req->tracker_list, &ioc->free_chain_list);
+ }
+}
+
/**
* mpt3sas_base_free_smid - put smid back on free_list
* @ioc: per adapter object
@@ -2377,20 +2390,12 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
{
unsigned long flags;
int i;
- struct chain_tracker *chain_req, *next;
spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
if (smid < ioc->hi_priority_smid) {
/* scsiio queue */
i = smid - 1;
- if (!list_empty(&ioc->scsi_lookup[i].chain_list)) {
- list_for_each_entry_safe(chain_req, next,
- &ioc->scsi_lookup[i].chain_list, tracker_list) {
- list_del_init(&chain_req->tracker_list);
- list_add(&chain_req->tracker_list,
- &ioc->free_chain_list);
- }
- }
+ _dechain_st(ioc, &ioc->scsi_lookup[i]);
ioc->scsi_lookup[i].cb_idx = 0xFF;
ioc->scsi_lookup[i].scmd = NULL;
ioc->scsi_lookup[i].direct_io = 0;
--
1.8.5.6
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/10] mpt3sas: full mq support
Hi all, this is a patchset to enable full multiqueue support for the mpt3sas driver. While the HBA only has a single mailbox register for submitting commands, it does have individual receive queues per MSI-X interrupt and as such does benefit from converting it to full multiqueue support. As usual, comments and reviews are welcome. Hannes Reinecke (10): mpt3sas: switch to pci_alloc_irq_vectors mpt3sas: set default value for cb_idx mpt3sas: implement _dechain_st() mpt3sas: separate out _base_recovery_check() mpt3sas: open-code _scsih_scsi_lookup_get() mpt3sas: Introduce mpt3sas_get_st_from_smid() mpt3sas: use hi-priority queue for TMFs mpt3sas: lockless command submission for scsi-mq mpt3sas: Use 'msix_index' as argument for put_smid functions mpt3sas: scsi-mq interrupt steering drivers/scsi/mpt3sas/mpt3sas_base.c | 360 --- drivers/scsi/mpt3sas/mpt3sas_base.h | 24 ++- drivers/scsi/mpt3sas/mpt3sas_config.c| 2 +- drivers/scsi/mpt3sas/mpt3sas_ctl.c | 134 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 303 +++--- drivers/scsi/mpt3sas/mpt3sas_transport.c | 8 +- drivers/scsi/mpt3sas/mpt3sas_warpdrive.c | 4 +- 7 files changed, 623 insertions(+), 212 deletions(-) -- 1.8.5.6 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/10] mpt3sas: scsi-mq interrupt steering
Enable all queues and implement correct I/O steering for scsi-mq.
Signed-off-by: Hannes Reinecke
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 8 ++--
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 19 +++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index f6564a3..84f808d 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -3560,9 +3560,13 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc,
* with some internal commands that could be outstanding
*/
ioc->shost->can_queue = ioc->scsiio_depth - INTERNAL_SCSIIO_CMDS_COUNT;
+ if (ioc->shost->nr_hw_queues > 1) {
+ ioc->shost->nr_hw_queues = ioc->msix_vector_count;
+ ioc->shost->can_queue /= ioc->msix_vector_count;
+ }
dinitprintk(ioc, pr_info(MPT3SAS_FMT
- "scsi host: can_queue depth (%d)\n",
- ioc->name, ioc->shost->can_queue));
+ "scsi host: can_queue depth (%d), nr_hw_queues (%d)\n",
+ ioc->name, ioc->shost->can_queue, ioc->shost->nr_hw_queues));
/* contiguous pool for request and chains, 16 byte align, one extra "
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 8c451aa..e43f928 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -54,6 +54,7 @@
#include
#include
#include
+#include
#include
#include "mpt3sas_base.h"
@@ -4255,6 +4256,11 @@ void _flush_running(struct request *req, void *data,
bool reserved)
mpt3sas_setup_direct_io(ioc, scmd, raid_device, mpi_request,
smid);
+ if (shost_use_blk_mq(ioc->shost) && (ioc->shost->nr_hw_queues > 1)) {
+ u32 unique_tag = blk_mq_unique_tag(scmd->request);
+
+ msix_task = blk_mq_unique_tag_to_hwq(unique_tag);
+ }
if (likely(mpi_request->Function == MPI2_FUNCTION_SCSI_IO_REQUEST)) {
if (sas_target_priv_data->flags & MPT_TARGET_FASTPATH_IO) {
mpi_request->IoFlags = cpu_to_le16(scmd->cmd_len |
@@ -8775,6 +8781,16 @@ static void sas_device_make_active(struct
MPT3SAS_ADAPTER *ioc,
return 1;
}
+static int scsih_map_queues(struct Scsi_Host *shost)
+{
+ struct MPT3SAS_ADAPTER *ioc = shost_priv(shost);
+
+ if (shost->nr_hw_queues > 1)
+ return blk_mq_pci_map_queues(&shost->tag_set, ioc->pdev);
+
+ return -EINVAL;
+}
+
/* shost template for SAS 2.0 HBA devices */
static struct scsi_host_template mpt2sas_driver_template = {
.module = THIS_MODULE,
@@ -8788,6 +8804,7 @@ static void sas_device_make_active(struct MPT3SAS_ADAPTER
*ioc,
.slave_destroy = scsih_slave_destroy,
.scan_finished = scsih_scan_finished,
.scan_start = scsih_scan_start,
+ .map_queues = scsih_map_queues,
.change_queue_depth = scsih_change_queue_depth,
.eh_abort_handler = scsih_abort,
.eh_device_reset_handler= scsih_dev_reset,
@@ -9054,6 +9071,8 @@ static void sas_device_make_active(struct MPT3SAS_ADAPTER
*ioc,
shost->max_lun = max_lun;
shost->transportt = mpt3sas_transport_template;
shost->unique_id = ioc->id;
+ if (shost->use_blk_mq)
+ shost->nr_hw_queues = num_online_cpus();
if (max_sectors != 0x) {
if (max_sectors < 64) {
--
1.8.5.6
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/10] mpt3sas: set default value for cb_idx
No functional change.
Signed-off-by: Hannes Reinecke
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 0185a8d..09d0008 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -873,7 +873,7 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
_base_get_cb_idx(struct MPT3SAS_ADAPTER *ioc, u16 smid)
{
int i;
- u8 cb_idx;
+ u8 cb_idx = 0xFF;
if (smid < ioc->hi_priority_smid) {
i = smid - 1;
@@ -884,8 +884,7 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
} else if (smid <= ioc->hba_queue_depth) {
i = smid - ioc->internal_smid;
cb_idx = ioc->internal_lookup[i].cb_idx;
- } else
- cb_idx = 0xFF;
+ }
return cb_idx;
}
--
1.8.5.6
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/10] mpt3sas: Use 'msix_index' as argument for put_smid functions
Use msix_index as explicit argument for the various put_smid
callbacks.
No functional change.
Signed-off-by: Hannes Reinecke
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 56 +---
drivers/scsi/mpt3sas/mpt3sas_base.h | 18 --
drivers/scsi/mpt3sas/mpt3sas_config.c| 2 +-
drivers/scsi/mpt3sas/mpt3sas_ctl.c | 21 ++--
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 21 ++--
drivers/scsi/mpt3sas/mpt3sas_transport.c | 8 ++---
6 files changed, 71 insertions(+), 55 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 29e139f..f6564a3 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -849,7 +849,7 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
ack_request->EventContext = mpi_reply->EventContext;
ack_request->VF_ID = 0; /* TODO */
ack_request->VP_ID = 0;
- ioc->put_smid_default(ioc, smid);
+ mpt3sas_base_put_smid_default(ioc, smid);
out:
@@ -2290,12 +2290,6 @@ struct scsiio_tracker *
return ioc->reply + (phys_addr - (u32)ioc->reply_dma);
}
-static inline u8
-_base_get_msix_index(struct MPT3SAS_ADAPTER *ioc)
-{
- return ioc->cpu_msix_table[raw_smp_processor_id()];
-}
-
/**
* mpt3sas_base_get_smid - obtain a free smid from internal queue
* @ioc: per adapter object
@@ -2370,7 +2364,7 @@ struct scsiio_tracker *
request->scmd = scmd;
request->cb_idx = cb_idx;
smid = request->smid;
- request->msix_io = _base_get_msix_index(ioc);
+ request->msix_io = mpt3sas_base_get_msix_index(ioc);
list_del(&request->tracker_list);
spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
return smid;
@@ -2538,18 +2532,20 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc,
* @ioc: per adapter object
* @smid: system request message index
* @handle: device handle
+ * @msix_index: MSI-X index
*
* Return nothing.
*/
static void
-_base_put_smid_scsi_io(struct MPT3SAS_ADAPTER *ioc, u16 smid, u16 handle)
+_base_put_smid_scsi_io(struct MPT3SAS_ADAPTER *ioc, u16 smid,
+ u16 handle, u16 msix_index)
{
Mpi2RequestDescriptorUnion_t descriptor;
u64 *request = (u64 *)&descriptor;
descriptor.SCSIIO.RequestFlags = MPI2_REQ_DESCRIPT_FLAGS_SCSI_IO;
- descriptor.SCSIIO.MSIxIndex = _base_get_msix_index(ioc);
+ descriptor.SCSIIO.MSIxIndex = cpu_to_le16(msix_index);
descriptor.SCSIIO.SMID = cpu_to_le16(smid);
descriptor.SCSIIO.DevHandle = cpu_to_le16(handle);
descriptor.SCSIIO.LMID = 0;
@@ -2562,19 +2558,20 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc,
* @ioc: per adapter object
* @smid: system request message index
* @handle: device handle
+ * @msix_index: MSI-X index
*
* Return nothing.
*/
static void
_base_put_smid_fast_path(struct MPT3SAS_ADAPTER *ioc, u16 smid,
- u16 handle)
+ u16 handle, u16 msix_index)
{
Mpi2RequestDescriptorUnion_t descriptor;
u64 *request = (u64 *)&descriptor;
descriptor.SCSIIO.RequestFlags =
MPI25_REQ_DESCRIPT_FLAGS_FAST_PATH_SCSI_IO;
- descriptor.SCSIIO.MSIxIndex = _base_get_msix_index(ioc);
+ descriptor.SCSIIO.MSIxIndex = cpu_to_le16(msix_index);
descriptor.SCSIIO.SMID = cpu_to_le16(smid);
descriptor.SCSIIO.DevHandle = cpu_to_le16(handle);
descriptor.SCSIIO.LMID = 0;
@@ -2598,7 +2595,7 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc,
descriptor.HighPriority.RequestFlags =
MPI2_REQ_DESCRIPT_FLAGS_HIGH_PRIORITY;
- descriptor.HighPriority.MSIxIndex = msix_task;
+ descriptor.HighPriority.MSIxIndex = cpu_to_le16(msix_task);
descriptor.HighPriority.SMID = cpu_to_le16(smid);
descriptor.HighPriority.LMID = 0;
descriptor.HighPriority.Reserved1 = 0;
@@ -2610,17 +2607,18 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc,
* _base_put_smid_default - Default, primarily used for config pages
* @ioc: per adapter object
* @smid: system request message index
+ * @msix_index: MSI-X index
*
* Return nothing.
*/
static void
-_base_put_smid_default(struct MPT3SAS_ADAPTER *ioc, u16 smid)
+_base_put_smid_default(struct MPT3SAS_ADAPTER *ioc, u16 smid, u16 msix_index)
{
Mpi2RequestDescriptorUnion_t descriptor;
u64 *request = (u64 *)&descriptor;
descriptor.Default.RequestFlags = MPI2_REQ_DESCRIPT_FLAGS_DEFAULT_TYPE;
- descriptor.Default.MSIxIndex = _base_get_msix_index(ioc);
+ descriptor.Default.MSIxIndex = cpu_to_le16(msix_index);
descriptor.Default.SMID = cpu_to_le16(smid);
descriptor.Default.LMID = 0;
descriptor.Default.DescriptorTypeDependent = 0;
@@ -2634,18 +2632,19 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc,
* @ioc: per adapter object
* @smid: system request message index
[PATCH 05/10] mpt3sas: open-code _scsih_scsi_lookup_get()
Just a wrapper around the scsi lookup array and only used
in one place, so open-code it.
Signed-off-by: Hannes Reinecke
---
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 15 +--
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index b5c966e..979f95a 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -1061,19 +1061,6 @@ struct _sas_node *
}
/**
- * _scsih_scsi_lookup_get - returns scmd entry
- * @ioc: per adapter object
- * @smid: system request message index
- *
- * Returns the smid stored scmd pointer.
- */
-static struct scsi_cmnd *
-_scsih_scsi_lookup_get(struct MPT3SAS_ADAPTER *ioc, u16 smid)
-{
- return ioc->scsi_lookup[smid - 1].scmd;
-}
-
-/**
* _scsih_scsi_lookup_get_clear - returns scmd entry
* @ioc: per adapter object
* @smid: system request message index
@@ -6101,7 +6088,7 @@ static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
for (smid = 1; smid <= ioc->scsiio_depth; smid++) {
if (ioc->shost_recovery)
goto out;
- scmd = _scsih_scsi_lookup_get(ioc, smid);
+ scmd = ioc->scsi_lookup[smid - 1].scmd;
if (!scmd)
continue;
sdev = scmd->device;
--
1.8.5.6
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/10] mpt3sas: lockless command submission for scsi-mq
Enable lockless command submission for scsi-mq by moving the
command structure into the payload for struct request.
Signed-off-by: Hannes Reinecke
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 125 ++
drivers/scsi/mpt3sas/mpt3sas_base.h | 2 +
drivers/scsi/mpt3sas/mpt3sas_ctl.c | 111
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 246 +--
4 files changed, 427 insertions(+), 57 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 942fb7e..29e139f 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -867,6 +867,20 @@ struct scsiio_tracker *
{
WARN_ON(!smid);
WARN_ON(smid >= ioc->hi_priority_smid);
+ if (shost_use_blk_mq(ioc->shost)) {
+ u16 hwq = (smid - 1) % ioc->shost->nr_hw_queues;
+ u16 tag = (smid - 1) / ioc->shost->nr_hw_queues;
+ struct blk_mq_tag_set *tagset = &ioc->shost->tag_set;
+ struct request *req;
+
+ req = blk_mq_tag_to_rq(tagset->tags[hwq], tag);
+ if (req) {
+ struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req);
+
+ return scsi_cmd_priv(cmd);
+ } else
+ return NULL;
+ }
return &ioc->scsi_lookup[smid - 1];
}
@@ -2329,6 +2343,20 @@ struct scsiio_tracker *
struct scsiio_tracker *request;
u16 smid;
+ if (shost_use_blk_mq(ioc->shost)) {
+ unsigned int unique_tag = blk_mq_unique_tag(scmd->request);
+ u16 hwq = blk_mq_unique_tag_to_hwq(unique_tag);
+ u16 tag = blk_mq_unique_tag_to_tag(unique_tag);
+ u16 smid = (tag * ioc->shost->nr_hw_queues) + hwq + 1;
+
+ request = scsi_cmd_priv(scmd);
+ request->scmd = scmd;
+ request->cb_idx = cb_idx;
+ request->msix_io = hwq;
+ request->smid = smid;
+ INIT_LIST_HEAD(&request->chain_list);
+ return request->smid;
+ }
spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
if (list_empty(&ioc->free_list)) {
spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
@@ -2403,6 +2431,23 @@ struct scsiio_tracker *
}
}
+void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc,
+ struct scsiio_tracker *st)
+{
+ if (WARN_ON(st->smid == 0))
+ return;
+ st->cb_idx = 0xFF;
+ st->scmd = NULL;
+ st->direct_io = 0;
+ if (!list_empty(&st->chain_list)) {
+ unsigned long flags;
+
+ spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
+ _dechain_st(ioc, st);
+ spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
+ }
+}
+
/**
* mpt3sas_base_free_smid - put smid back on free_list
* @ioc: per adapter object
@@ -2416,6 +2461,19 @@ struct scsiio_tracker *
unsigned long flags;
int i;
+ if (shost_use_blk_mq(ioc->shost) && smid < ioc->hi_priority_smid) {
+ struct scsiio_tracker *st;
+
+ st = mpt3sas_get_st_from_smid(ioc, smid);
+ if (WARN_ON(!st)) {
+ _base_recovery_check(ioc);
+ return;
+ }
+ mpt3sas_base_clear_st(ioc, st);
+ _base_recovery_check(ioc);
+ return;
+ }
+
spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
if (smid < ioc->hi_priority_smid) {
/* scsiio queue */
@@ -3559,14 +3617,23 @@ struct scsiio_tracker *
ioc->name, (unsigned long long) ioc->request_dma));
total_sz += sz;
- sz = ioc->scsiio_depth * sizeof(struct scsiio_tracker);
- ioc->scsi_lookup_pages = get_order(sz);
- ioc->scsi_lookup = (struct scsiio_tracker *)__get_free_pages(
- GFP_KERNEL, ioc->scsi_lookup_pages);
- if (!ioc->scsi_lookup) {
- pr_err(MPT3SAS_FMT "scsi_lookup: get_free_pages failed,
sz(%d)\n",
- ioc->name, (int)sz);
- goto out;
+ /*
+* Don't need to allocate memory for scsiio_tracker array if we
+* are using scsi-mq, we embed it in the scsi_cmnd for that case.
+*/
+ if (!shost_use_blk_mq(ioc->shost)) {
+ sz = ioc->scsiio_depth * sizeof(struct scsiio_tracker);
+ ioc->scsi_lookup_pages = get_order(sz);
+ ioc->scsi_lookup = (struct scsiio_tracker *)__get_free_pages(
+ GFP_KERNEL, ioc->scsi_lookup_pages);
+ if (!ioc->scsi_lookup) {
+ pr_err(MPT3SAS_FMT "scsi_lookup: get_free_pages "
+ "failed, sz(%d)\n", ioc->name, (int)sz);
+ goto out;
+ }
+ } else {
+ ioc->scsi_lookup_pages = 0;
+
[PATCH 01/10] mpt3sas: switch to pci_alloc_irq_vectors
Cleanup the MSI-X handling allowing us to use
the PCI-layer provided vector allocation.
Signed-off-by: Hannes Reinecke
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 100 +---
drivers/scsi/mpt3sas/mpt3sas_base.h | 2 -
2 files changed, 46 insertions(+), 56 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index f00ef88..0185a8d 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1129,7 +1129,7 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
/* TMs are on msix_index == 0 */
if (reply_q->msix_index == 0)
continue;
- synchronize_irq(reply_q->vector);
+ synchronize_irq(pci_irq_vector(ioc->pdev, reply_q->msix_index));
}
}
@@ -1818,11 +1818,8 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
list_for_each_entry_safe(reply_q, next, &ioc->reply_queue_list, list) {
list_del(&reply_q->list);
- if (smp_affinity_enable) {
- irq_set_affinity_hint(reply_q->vector, NULL);
- free_cpumask_var(reply_q->affinity_hint);
- }
- free_irq(reply_q->vector, reply_q);
+ free_irq(pci_irq_vector(ioc->pdev, reply_q->msix_index),
+reply_q);
kfree(reply_q);
}
}
@@ -1831,13 +1828,13 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
* _base_request_irq - request irq
* @ioc: per adapter object
* @index: msix index into vector table
- * @vector: irq vector
*
* Inserting respective reply_queue into the list.
*/
static int
-_base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8 index, u32 vector)
+_base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8 index)
{
+ struct pci_dev *pdev = ioc->pdev;
struct adapter_reply_queue *reply_q;
int r;
@@ -1849,14 +1846,6 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
}
reply_q->ioc = ioc;
reply_q->msix_index = index;
- reply_q->vector = vector;
-
- if (smp_affinity_enable) {
- if (!zalloc_cpumask_var(&reply_q->affinity_hint, GFP_KERNEL)) {
- kfree(reply_q);
- return -ENOMEM;
- }
- }
atomic_set(&reply_q->busy, 0);
if (ioc->msix_enable)
@@ -1865,12 +1854,11 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
else
snprintf(reply_q->name, MPT_NAME_LENGTH, "%s%d",
ioc->driver_name, ioc->id);
- r = request_irq(vector, _base_interrupt, IRQF_SHARED, reply_q->name,
- reply_q);
+ r = request_irq(pci_irq_vector(pdev, index), _base_interrupt,
+ IRQF_SHARED, reply_q->name, reply_q);
if (r) {
pr_err(MPT3SAS_FMT "unable to allocate interrupt %d!\n",
- reply_q->name, vector);
- free_cpumask_var(reply_q->affinity_hint);
+ reply_q->name, pci_irq_vector(pdev, index));
kfree(reply_q);
return -EBUSY;
}
@@ -1906,6 +1894,21 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
if (!nr_msix)
return;
+ if (smp_affinity_enable) {
+ list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
+ const cpumask_t *mask = pci_irq_get_affinity(ioc->pdev,
+ reply_q->msix_index);
+ if (!mask) {
+ pr_warn(MPT3SAS_FMT "no affinity for msi %x\n",
+ ioc->name, reply_q->msix_index);
+ continue;
+ }
+
+ for_each_cpu(cpu, mask)
+ ioc->cpu_msix_table[cpu] = reply_q->msix_index;
+ }
+ return;
+ }
cpu = cpumask_first(cpu_online_mask);
list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
@@ -1919,18 +1922,9 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
group++;
for (i = 0 ; i < group ; i++) {
- ioc->cpu_msix_table[cpu] = index;
- if (smp_affinity_enable)
- cpumask_or(reply_q->affinity_hint,
- reply_q->affinity_hint, get_cpu_mask(cpu));
+ ioc->cpu_msix_table[cpu] = reply_q->msix_index;
cpu = cpumask_next(cpu, cpu_online_mask);
}
- if (smp_affinity_enable)
- if (irq_set_affinity_hint(reply_q->vector,
- reply_q->affinity_hint))
- dinitprintk(ioc, pr_info(MPT3SAS_FMT
-
[PATCH 07/10] mpt3sas: use hi-priority queue for TMFs
When sending a TMF via the ioctl interface we should be using
the hi-priority queue instead of the scsi queue to be consistent
with overall TMF usage.
Signed-off-by: Hannes Reinecke
---
drivers/scsi/mpt3sas/mpt3sas_ctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
index 95f0f24..e952175 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
@@ -720,7 +720,7 @@ enum block_state {
}
} else {
- smid = mpt3sas_base_get_smid_scsiio(ioc, ioc->ctl_cb_idx, NULL);
+ smid = mpt3sas_base_get_smid_hpr(ioc, ioc->ctl_cb_idx);
if (!smid) {
pr_err(MPT3SAS_FMT "%s: failed obtaining a smid\n",
ioc->name, __func__);
--
1.8.5.6
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/10] mpt3sas: separate out _base_recovery_check()
No functional change.
Signed-off-by: Hannes Reinecke
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 120b317..dd14596 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -2366,6 +2366,19 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
}
static void
+_base_recovery_check(struct MPT3SAS_ADAPTER *ioc)
+{
+ /*
+* See _wait_for_commands_to_complete() call with regards to this code.
+*/
+ if (ioc->shost_recovery && ioc->pending_io_count) {
+ if (ioc->pending_io_count == 1)
+ wake_up(&ioc->reset_wq);
+ ioc->pending_io_count = 0;
+ }
+}
+
+static void
_dechain_st(struct MPT3SAS_ADAPTER *ioc, struct scsiio_tracker *st)
{
struct chain_tracker *chain_req;
@@ -2402,15 +2415,7 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
list_add(&ioc->scsi_lookup[i].tracker_list, &ioc->free_list);
spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
- /*
-* See _wait_for_commands_to_complete() call with regards
-* to this code.
-*/
- if (ioc->shost_recovery && ioc->pending_io_count) {
- if (ioc->pending_io_count == 1)
- wake_up(&ioc->reset_wq);
- ioc->pending_io_count--;
- }
+ _base_recovery_check(ioc);
return;
} else if (smid < ioc->internal_smid) {
/* hi-priority */
--
1.8.5.6
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/10] mpt3sas: Introduce mpt3sas_get_st_from_smid()
Abstract accesses to the scsi_lookup array by introducing
mpt3sas_get_st_from_smid().
Signed-off-by: Hannes Reinecke
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 20
drivers/scsi/mpt3sas/mpt3sas_base.h | 2 ++
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 2 +-
drivers/scsi/mpt3sas/mpt3sas_warpdrive.c | 4 +++-
4 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index dd14596..942fb7e 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -862,6 +862,14 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
return 1;
}
+struct scsiio_tracker *
+mpt3sas_get_st_from_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid)
+{
+ WARN_ON(!smid);
+ WARN_ON(smid >= ioc->hi_priority_smid);
+ return &ioc->scsi_lookup[smid - 1];
+}
+
/**
* _base_get_cb_idx - obtain the callback index
* @ioc: per adapter object
@@ -876,8 +884,11 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
u8 cb_idx = 0xFF;
if (smid < ioc->hi_priority_smid) {
- i = smid - 1;
- cb_idx = ioc->scsi_lookup[i].cb_idx;
+ struct scsiio_tracker *st;
+
+ st = mpt3sas_get_st_from_smid(ioc, smid);
+ if (st)
+ cb_idx = st->cb_idx;
} else if (smid < ioc->internal_smid) {
i = smid - ioc->hi_priority_smid;
cb_idx = ioc->hpr_lookup[i].cb_idx;
@@ -1249,6 +1260,7 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
_base_get_chain_buffer_tracker(struct MPT3SAS_ADAPTER *ioc, u16 smid)
{
struct chain_tracker *chain_req;
+ struct scsiio_tracker *st;
unsigned long flags;
spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
@@ -1261,8 +1273,8 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
chain_req = list_entry(ioc->free_chain_list.next,
struct chain_tracker, tracker_list);
list_del_init(&chain_req->tracker_list);
- list_add_tail(&chain_req->tracker_list,
- &ioc->scsi_lookup[smid - 1].chain_list);
+ st = mpt3sas_get_st_from_smid(ioc, smid);
+ list_add_tail(&chain_req->tracker_list, &st->chain_list);
spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
return chain_req;
}
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h
b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 505574e..876e7b4 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -1246,6 +1246,8 @@ __le32 mpt3sas_base_get_sense_buffer_dma(struct
MPT3SAS_ADAPTER *ioc,
u16 mpt3sas_base_get_smid_hpr(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx);
u16 mpt3sas_base_get_smid_scsiio(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx,
struct scsi_cmnd *scmd);
+struct scsiio_tracker * mpt3sas_get_st_from_smid(struct MPT3SAS_ADAPTER *ioc,
+ u16 smid);
u16 mpt3sas_base_get_smid(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx);
void mpt3sas_base_free_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 979f95a..ca925ef 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -2250,7 +2250,7 @@ struct _sas_node *
}
if (type == MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK)
- scsi_lookup = &ioc->scsi_lookup[smid_task - 1];
+ scsi_lookup = mpt3sas_get_st_from_smid(ioc, smid_task);
dtmprintk(ioc, pr_info(MPT3SAS_FMT
"sending tm: handle(0x%04x), task_type(0x%02x), smid(%d)\n",
diff --git a/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c
b/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c
index 540bd50..06e3f7d 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c
@@ -270,7 +270,9 @@
inline u8
mpt3sas_scsi_direct_io_get(struct MPT3SAS_ADAPTER *ioc, u16 smid)
{
- return ioc->scsi_lookup[smid - 1].direct_io;
+ struct scsiio_tracker *st = mpt3sas_get_st_from_smid(ioc, smid);
+
+ return st ? st->direct_io : 0;
}
/**
--
1.8.5.6
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: scsi: BUG in scsi_init_io
On Tue, Jan 31, 2017 at 10:20 AM, Johannes Thumshirn wrote:
> On Tue, Jan 31, 2017 at 09:55:52AM +0100, Dmitry Vyukov wrote:
>> Hello,
>>
>> The following program triggers BUG in scsi_init_io:
>
> Well crashing a machine just because of an empty dma transfer is a bit harsh,
> isn't it?
>
> From 86e6fa5f618fe588b98e923e032f33e075fcd4f4 Mon Sep 17 00:00:00 2001
> From: Johannes Thumshirn
> Date: Tue, 31 Jan 2017 10:16:00 +0100
> Subject: [PATCH] scsi: don't BUG_ON() empty DMA transfers
>
> Don't crash the machine just because of an empty transfer. Use WARN_ON()
> combined with returning an error.
>
> Signed-off-by: Johannes Thumshirn
> ---
> drivers/scsi/scsi_lib.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index e9e1e14..414588a 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1040,7 +1040,8 @@ int scsi_init_io(struct scsi_cmnd *cmd)
> bool is_mq = (rq->mq_ctx != NULL);
> int error;
>
> - BUG_ON(!blk_rq_nr_phys_segments(rq));
> + if (WARN_ON(!blk_rq_nr_phys_segments(rq)))
> + return -EINVAL;
Please-please-please, let's not use WARN for something that is not a
kernel bug and is user-triggerable. This makes it impossible to
automate kernel testing and requires hiring an army of people doing
mechanical job of sorting out WARNING reports into kernel-bugs and
non-kernel-bugs.
If the message is absolutely necessary (while kernel does not
generally explain every EINVAL on console), the following will do:
if (!blk_rq_nr_phys_segments(rq)) {
pr_err("you are doing something wrong\n");
return -EINVAL;
}
> error = scsi_init_sgtable(rq, &cmd->sdb);
> if (error)
> --
> 2.10.2
>
>
> --
> Johannes Thumshirn Storage
> jthumsh...@suse.de+49 911 74053 689
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
> Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: scsi: BUG in scsi_init_io
On Tue, Jan 31, 2017 at 10:50:49AM +0100, Dmitry Vyukov wrote:
> On Tue, Jan 31, 2017 at 10:20 AM, Johannes Thumshirn
> wrote:
> > On Tue, Jan 31, 2017 at 09:55:52AM +0100, Dmitry Vyukov wrote:
[...]
> Please-please-please, let's not use WARN for something that is not a
> kernel bug and is user-triggerable. This makes it impossible to
> automate kernel testing and requires hiring an army of people doing
> mechanical job of sorting out WARNING reports into kernel-bugs and
> non-kernel-bugs.
> If the message is absolutely necessary (while kernel does not
> generally explain every EINVAL on console), the following will do:
>
> if (!blk_rq_nr_phys_segments(rq)) {
> pr_err("you are doing something wrong\n");
> return -EINVAL;
> }
Yes I understand that. OTOH having the WARN helps you finding the caller because
of to the stack trace. But arguably that could be accomplished with function
graph tracing as well. I'll re-send a v2 as a proper patch.
Thanks,
Johannes
--
Johannes Thumshirn Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/10] mpt3sas: full mq support
On Tue, Jan 31, 2017 at 10:25:50AM +0100, Hannes Reinecke wrote: > Hi all, > > this is a patchset to enable full multiqueue support for the mpt3sas driver. > While the HBA only has a single mailbox register for submitting commands, > it does have individual receive queues per MSI-X interrupt and as such > does benefit from converting it to full multiqueue support. Explanation and numbers on why this would be beneficial, please. We should not need multiple submissions queues for a single register to benefit from multiple completion queues. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/10] mpt3sas: scsi-mq interrupt steering
We should not need multiple submission queues for IRQ steering. We just need to build the cpu to vector reverse map that blk_mq_pci_map_queues builds. So maybe we need to refactor the API for ->map_queues a bit so that it's not tied to submission queues. In the meantime you can just opencode that loop. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: scsi: BUG in scsi_init_io
On Tue, Jan 31, 2017 at 10:58 AM, Johannes Thumshirn wrote:
>
> [...]
>
>> Please-please-please, let's not use WARN for something that is not a
>> kernel bug and is user-triggerable. This makes it impossible to
>> automate kernel testing and requires hiring an army of people doing
>> mechanical job of sorting out WARNING reports into kernel-bugs and
>> non-kernel-bugs.
>> If the message is absolutely necessary (while kernel does not
>> generally explain every EINVAL on console), the following will do:
>>
>> if (!blk_rq_nr_phys_segments(rq)) {
>> pr_err("you are doing something wrong\n");
>> return -EINVAL;
>> }
>
> Yes I understand that. OTOH having the WARN helps you finding the caller
> because
> of to the stack trace. But arguably that could be accomplished with function
> graph tracing as well. I'll re-send a v2 as a proper patch.
Thank you very much.
Stack trace can be done with dump_stack() if necessary, e.g.
pr_err("you are doing something wrong here:\n");
dump_stack();
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/10] mpt3sas: full mq support
On 01/31/2017 11:02 AM, Christoph Hellwig wrote: > On Tue, Jan 31, 2017 at 10:25:50AM +0100, Hannes Reinecke wrote: >> Hi all, >> >> this is a patchset to enable full multiqueue support for the mpt3sas driver. >> While the HBA only has a single mailbox register for submitting commands, >> it does have individual receive queues per MSI-X interrupt and as such >> does benefit from converting it to full multiqueue support. > > Explanation and numbers on why this would be beneficial, please. > We should not need multiple submissions queues for a single register > to benefit from multiple completion queues. > Well, the actual throughput very strongly depends on the blk-mq-sched patches from Jens. As this is barely finished I didn't post any numbers yet. However: With multiqueue support: 4k seq read : io=60573MB, bw=1009.2MB/s, iops=258353, runt= 60021msec With scsi-mq on 1 queue: 4k seq read : io=17369MB, bw=296291KB/s, iops=74072, runt= 60028msec So yes, there _is_ a benefit. (Which is actually quite cool, as these tests were done on a SAS3 HBA, so we're getting close to the theoretical maximum of 1.2GB/s). (Unlike the single-queue case :-) Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi_dh_emc: fix uninitialized return value ret
From: Colin Ian King
ret is uninitialized on a successful execution of clarrion_std_inquiry
and a garbage return value is being returted. Fix this by setting ret
to zero on the success exit path.
Found by CoverityScan, CID#1398889 ("Uninitialized scaler variable")
Fixes: b78205c93297b8219 ("scsi_dh_emc: switch to scsi_execute_req_flags()")
Signed-off-by: Colin Ian King
---
drivers/scsi/device_handler/scsi_dh_emc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c
b/drivers/scsi/device_handler/scsi_dh_emc.c
index 4a7679f..6647da2 100644
--- a/drivers/scsi/device_handler/scsi_dh_emc.c
+++ b/drivers/scsi/device_handler/scsi_dh_emc.c
@@ -376,6 +376,7 @@ static int clariion_std_inquiry(struct scsi_device *sdev,
sdev_printk(KERN_INFO, sdev,
"%s: detected Clariion %s, flags %x\n",
CLARIION_NAME, sp_model, csdev->flags);
+ err = 0;
out:
return err;
}
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi_dh_emc: fix uninitialized return value ret
Am 31.01.2017 13:22, schrieb Colin King:
> From: Colin Ian King
>
> ret is uninitialized on a successful execution of clarrion_std_inquiry
> and a garbage return value is being returted. Fix this by setting ret
> to zero on the success exit path.
>
> Found by CoverityScan, CID#1398889 ("Uninitialized scaler variable")
>
> Fixes: b78205c93297b8219 ("scsi_dh_emc: switch to scsi_execute_req_flags()")
> Signed-off-by: Colin Ian King
> ---
> drivers/scsi/device_handler/scsi_dh_emc.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c
> b/drivers/scsi/device_handler/scsi_dh_emc.c
> index 4a7679f..6647da2 100644
> --- a/drivers/scsi/device_handler/scsi_dh_emc.c
> +++ b/drivers/scsi/device_handler/scsi_dh_emc.c
> @@ -376,6 +376,7 @@ static int clariion_std_inquiry(struct scsi_device *sdev,
> sdev_printk(KERN_INFO, sdev,
> "%s: detected Clariion %s, flags %x\n",
> CLARIION_NAME, sp_model, csdev->flags);
> + err = 0;
> out:
> return err;
> }
I am not sure IMHO err is set in the first line err = send_inquiry_cmd(sdev,
0, csdev);
but this looks strange:
if (!strlen(sp_model) || !strncmp(sp_model, "FC",2))
IMHO we can drop strlen() and the compare should be agaist:
#define emc_default_str "FC (Legacy)"
just my 2 cents,
re,
wh
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] mpt3sas: lockless command submission for scsi-mq
On Tue, Jan 31, 2017 at 10:25:58AM +0100, Hannes Reinecke wrote:
> Enable lockless command submission for scsi-mq by moving the
> command structure into the payload for struct request.
We support cmd_size for both the mq and !mq code path, so this
could be simplified by always taking your new block-mq path.
> 4 files changed, 427 insertions(+), 57 deletions(-)
The amount of new code scare me. Especially compared to the very
shport changelog.
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
> b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index 942fb7e..29e139f 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -867,6 +867,20 @@ struct scsiio_tracker *
> {
> WARN_ON(!smid);
> WARN_ON(smid >= ioc->hi_priority_smid);
> + if (shost_use_blk_mq(ioc->shost)) {
> + u16 hwq = (smid - 1) % ioc->shost->nr_hw_queues;
> + u16 tag = (smid - 1) / ioc->shost->nr_hw_queues;
> + struct blk_mq_tag_set *tagset = &ioc->shost->tag_set;
> + struct request *req;
> +
> + req = blk_mq_tag_to_rq(tagset->tags[hwq], tag);
> + if (req) {
> + struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req);
> +
> + return scsi_cmd_priv(cmd);
> + } else
> + return NULL;
> + }
This looks like it basically duplicates scsi_host_find_tag().
> + if (shost_use_blk_mq(ioc->shost)) {
> + unsigned int unique_tag = blk_mq_unique_tag(scmd->request);
> + u16 hwq = blk_mq_unique_tag_to_hwq(unique_tag);
> + u16 tag = blk_mq_unique_tag_to_tag(unique_tag);
> + u16 smid = (tag * ioc->shost->nr_hw_queues) + hwq + 1;
> +
> + request = scsi_cmd_priv(scmd);
> + request->scmd = scmd;
no need for a backpointer, blk_mq_rq_from_pdu gets your
back to the request, and scsi_cmd_priv back to driver private data.
> + request->cb_idx = cb_idx;
> + request->msix_io = hwq;
> + request->smid = smid;
why do we need to store the smid?
> @@ -562,12 +642,7 @@ enum block_state {
> _ctl_set_task_mid(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command
> *karg,
I think this function just needs to go away instantly.
Trying to send task management requests from userspace for any
active smid is plain dangerous.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] mpt3sas: lockless command submission for scsi-mq
On 01/31/2017 02:22 PM, Christoph Hellwig wrote:
> On Tue, Jan 31, 2017 at 10:25:58AM +0100, Hannes Reinecke wrote:
>> Enable lockless command submission for scsi-mq by moving the
>> command structure into the payload for struct request.
>
> We support cmd_size for both the mq and !mq code path, so this
> could be simplified by always taking your new block-mq path.
>
Yes, they are with your latest patchset.
But that sort of overlapped with my patches and is hasn't been merged
yet. So I haven't integrated that.
So what is the status?
Should I rebase my patch on top of those patches?
>> 4 files changed, 427 insertions(+), 57 deletions(-)
>
> The amount of new code scare me. Especially compared to the very
> short changelog.
>
Well, these are mainly duplicated code paths as I left the original code
in place.
Simply because the infrastructure is very much in flux and converting it
all in one go might be giving negative results :-)
>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
>> b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> index 942fb7e..29e139f 100644
>> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> @@ -867,6 +867,20 @@ struct scsiio_tracker *
>> {
>> WARN_ON(!smid);
>> WARN_ON(smid >= ioc->hi_priority_smid);
>> +if (shost_use_blk_mq(ioc->shost)) {
>> +u16 hwq = (smid - 1) % ioc->shost->nr_hw_queues;
>> +u16 tag = (smid - 1) / ioc->shost->nr_hw_queues;
>> +struct blk_mq_tag_set *tagset = &ioc->shost->tag_set;
>> +struct request *req;
>> +
>> +req = blk_mq_tag_to_rq(tagset->tags[hwq], tag);
>> +if (req) {
>> +struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req);
>> +
>> +return scsi_cmd_priv(cmd);
>> +} else
>> +return NULL;
>> +}
>
> This looks like it basically duplicates scsi_host_find_tag().
>
Yes, but scsi_host_find_tag() still relies on req->special, which it
really shouldn't after your patches; scsi_cmd_priv() should be used there.
I got a patch series to remove the last vestigies from using ->special
in the SCSI midlayer; probably time to post it.
>> +if (shost_use_blk_mq(ioc->shost)) {
>> +unsigned int unique_tag = blk_mq_unique_tag(scmd->request);
>> +u16 hwq = blk_mq_unique_tag_to_hwq(unique_tag);
>> +u16 tag = blk_mq_unique_tag_to_tag(unique_tag);
>> +u16 smid = (tag * ioc->shost->nr_hw_queues) + hwq + 1;
>> +
>> +request = scsi_cmd_priv(scmd);
>> +request->scmd = scmd;
>
> no need for a backpointer, blk_mq_rq_from_pdu gets your
> back to the request, and scsi_cmd_priv back to driver private data.
>
I know. Compability with original code only.
>> +request->cb_idx = cb_idx;
>> +request->msix_io = hwq;
>> +request->smid = smid;
>
> why do we need to store the smid?
>
Sanity checking.
Far too many instances where I got the tag <-> smid mapping wrong during
development.
But yeah, if we move the entire driver over and rip out the original
code path this can go.
>> @@ -562,12 +642,7 @@ enum block_state {
>> _ctl_set_task_mid(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command
>> *karg,
>
> I think this function just needs to go away instantly.
> Trying to send task management requests from userspace for any
> active smid is plain dangerous.
>
Hey, I didn't to it. I just did the conversion.
But indeed, it's of questionable value.
I can remove it with the next iteration.
So, I'm happy to rebase it on top of your BLOCK_PC changes. Just tell me
which tree to use.
Then indeed most of the duplicate code-paths could go away.
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] mpt3sas: lockless command submission for scsi-mq
On Tue, Jan 31, 2017 at 02:46:19PM +0100, Hannes Reinecke wrote: > Yes, they are with your latest patchset. No, this has been the case since blk-mq support wa added to scsi. > Yes, but scsi_host_find_tag() still relies on req->special, which it > really shouldn't after your patches; scsi_cmd_priv() should be used there. Both are perfectly fine for now. > > I think this function just needs to go away instantly. > > Trying to send task management requests from userspace for any > > active smid is plain dangerous. > > > Hey, I didn't to it. I just did the conversion. > > But indeed, it's of questionable value. > > I can remove it with the next iteration. I think we need to sort out the mpt-specific passthrough ioctls one way or another. A lot of the mess in this patch comes from the fact that no all I/O goes through the block/scsi layer. Once that is sorted out things become a lot simpler. > So, I'm happy to rebase it on top of your BLOCK_PC changes. Just tell me > which tree to use. You don't need them. cmd_size has been there for a long time for SCSI. The only thing my patches do is to add support to legacy block code and use that new simpler and more efficient implementation in SCSI instead of the old hown-grown implementation in SCSI. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: scsi: BUG in scsi_init_io
On Tue, 2017-01-31 at 10:50 +0100, Dmitry Vyukov wrote: > On Tue, Jan 31, 2017 at 10:20 AM, Johannes Thumshirn < > jthumsh...@suse.de> wrote: > > On Tue, Jan 31, 2017 at 09:55:52AM +0100, Dmitry Vyukov wrote: > > > Hello, > > > > > > The following program triggers BUG in scsi_init_io: > > > > Well crashing a machine just because of an empty dma transfer is a > > bit harsh, > > isn't it? > > > > From 86e6fa5f618fe588b98e923e032f33e075fcd4f4 Mon Sep 17 00:00:00 > > 2001 > > From: Johannes Thumshirn > > Date: Tue, 31 Jan 2017 10:16:00 +0100 > > Subject: [PATCH] scsi: don't BUG_ON() empty DMA transfers > > > > Don't crash the machine just because of an empty transfer. Use > > WARN_ON() > > combined with returning an error. > > > > Signed-off-by: Johannes Thumshirn > > --- > > drivers/scsi/scsi_lib.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > > index e9e1e14..414588a 100644 > > --- a/drivers/scsi/scsi_lib.c > > +++ b/drivers/scsi/scsi_lib.c > > @@ -1040,7 +1040,8 @@ int scsi_init_io(struct scsi_cmnd *cmd) > > bool is_mq = (rq->mq_ctx != NULL); > > int error; > > > > - BUG_ON(!blk_rq_nr_phys_segments(rq)); > > + if (WARN_ON(!blk_rq_nr_phys_segments(rq))) > > + return -EINVAL; > > > Please-please-please, let's not use WARN for something that is not a > kernel bug and is user-triggerable. It is a kernel bug and it should not be user triggerable, so it should have a warn_on or bug_on. It means something called a data setup function with no data. There's actually a root cause that patches like this won't fix, can we find it? James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/10] ide: don't abuse cmd_type
Currently the legacy ide driver defines several request types of it's own,
which is in the way of removing that field entirely.
Instead add a type field to struct ide_request and use that to distinguish
the different types of IDE-internal requests.
It's a bit of a mess, but so is the surrounding code..
Signed-off-by: Christoph Hellwig
---
drivers/ide/ide-atapi.c| 18 ++-
drivers/ide/ide-cd.c | 25 -
drivers/ide/ide-cd_ioctl.c | 1 +
drivers/ide/ide-devsets.c | 1 +
drivers/ide/ide-disk.c | 6 +++--
drivers/ide/ide-eh.c | 4 ++--
drivers/ide/ide-floppy.c | 18 ++-
drivers/ide/ide-io.c | 8 +++
drivers/ide/ide-ioctls.c | 4 +++-
drivers/ide/ide-park.c | 2 ++
drivers/ide/ide-pm.c | 16 -
drivers/ide/ide-tape.c | 6 +++--
drivers/ide/ide-taskfile.c | 3 ++-
include/linux/ide.h| 56 +-
14 files changed, 117 insertions(+), 51 deletions(-)
diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index 7c826ec..a8c650e 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -95,6 +95,7 @@ int ide_queue_pc_tail(ide_drive_t *drive, struct gendisk
*disk,
rq = blk_get_request(drive->queue, READ, __GFP_RECLAIM);
scsi_req_init(rq);
rq->cmd_type = REQ_TYPE_DRV_PRIV;
+ ide_req(rq)->type = ATA_PRIV_MISC;
rq->special = (char *)pc;
if (buf && bufflen) {
@@ -193,7 +194,7 @@ void ide_prep_sense(ide_drive_t *drive, struct request *rq)
BUG_ON(sense_len > sizeof(*sense));
- if (rq->cmd_type == REQ_TYPE_ATA_SENSE || drive->sense_rq_armed)
+ if (ata_sense_request(rq) || drive->sense_rq_armed)
return;
memset(sense, 0, sizeof(*sense));
@@ -211,7 +212,8 @@ void ide_prep_sense(ide_drive_t *drive, struct request *rq)
}
sense_rq->rq_disk = rq->rq_disk;
- sense_rq->cmd_type = REQ_TYPE_ATA_SENSE;
+ sense_rq->cmd_type = REQ_TYPE_DRV_PRIV;
+ ide_req(sense_rq)->type = ATA_PRIV_SENSE;
sense_rq->rq_flags |= RQF_PREEMPT;
req->cmd[0] = GPCMD_REQUEST_SENSE;
@@ -313,10 +315,14 @@ int ide_cd_get_xferlen(struct request *rq)
switch (rq->cmd_type) {
case REQ_TYPE_FS:
return 32768;
- case REQ_TYPE_ATA_SENSE:
case REQ_TYPE_BLOCK_PC:
- case REQ_TYPE_ATA_PC:
return blk_rq_bytes(rq);
+ case REQ_TYPE_DRV_PRIV:
+ switch (ide_req(rq)->type) {
+ case ATA_PRIV_PC:
+ case ATA_PRIV_SENSE:
+ return blk_rq_bytes(rq);
+ }
default:
return 0;
}
@@ -377,7 +383,7 @@ int ide_check_ireason(ide_drive_t *drive, struct request
*rq, int len,
drive->name, __func__, ireason);
}
- if (dev_is_idecd(drive) && rq->cmd_type == REQ_TYPE_ATA_PC)
+ if (dev_is_idecd(drive) && ata_pc_request(rq))
rq->rq_flags |= RQF_FAILED;
return 1;
@@ -480,7 +486,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
if (uptodate == 0)
drive->failed_pc = NULL;
- if (rq->cmd_type == REQ_TYPE_DRV_PRIV) {
+ if (ata_misc_request(rq)) {
rq->errors = 0;
error = 0;
} else {
diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 6eb9872..207af78 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -210,7 +210,7 @@ static void cdrom_analyze_sense_data(ide_drive_t *drive,
static void ide_cd_complete_failed_rq(ide_drive_t *drive, struct request *rq)
{
/*
-* For REQ_TYPE_ATA_SENSE, "rq->special" points to the original
+* For ATA_PRIV_SENSE, "rq->special" points to the original
* failed request. Also, the sense data should be read
* directly from rq which might be different from the original
* sense buffer if it got copied during mapping.
@@ -282,7 +282,7 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
"stat 0x%x",
rq->cmd[0], rq->cmd_type, err, stat);
- if (rq->cmd_type == REQ_TYPE_ATA_SENSE) {
+ if (ata_sense_request(rq)) {
/*
* We got an error trying to get sense info from the drive
* (probably while trying to recover from a former error).
@@ -438,7 +438,8 @@ int ide_cd_queue_pc(ide_drive_t *drive, const unsigned char
*cmd,
rq = blk_get_request(drive->queue, write, __GFP_RECLAIM);
scsi_req_init(rq);
memcpy(scsi_req(rq)->cmd, cmd, BLK_MAX_CDB);
- rq->cmd_type = REQ_TYPE_ATA_PC;
+ rq->cmd_type = REQ_TYPE_DRV_PRIV;
+ ide_req(rq)->type = ATA_PRIV_PC;
rq->rq_fla
[PATCH 10/10] block: fold cmd_type into the REQ_OP_ space
Instead of keeping two levels of indirection for requests types, fold it
all into the operations. The little caveat here is that previously
cmd_type only applied to struct request, while the request and bio op
fields were set to plain REQ_OP_READ/WRITE even for passthrough
operations.
Instead this patch adds new REQ_OP_* for SCSI passthrough and driver
private requests, althought it has to add two for each so that we
can communicate the data in/out nature of the request.
Signed-off-by: Christoph Hellwig
---
block/bio.c| 10 -
block/blk-core.c | 6 +--
block/blk-flush.c | 1 -
block/blk-map.c| 13 ---
block/blk-mq-debugfs.c | 4 +-
block/bsg.c| 17 -
block/scsi_ioctl.c | 9 +++--
drivers/block/cciss.c | 13 +--
drivers/block/floppy.c | 4 +-
drivers/block/hd.c | 45 +++---
drivers/block/mg_disk.c| 31 +---
drivers/block/nbd.c| 19 ++
drivers/block/null_blk.c | 4 +-
drivers/block/osdblk.c | 6 ---
drivers/block/paride/pd.c | 15 +++-
drivers/block/pktcdvd.c| 2 +-
drivers/block/ps3disk.c| 15 +---
drivers/block/rbd.c| 22 ++-
drivers/block/sx8.c| 4 +-
drivers/block/virtio_blk.c | 61 +++---
drivers/block/xen-blkfront.c | 2 +-
drivers/block/xsysace.c| 2 +-
drivers/cdrom/cdrom.c | 2 +-
drivers/cdrom/gdrom.c | 29 ---
drivers/ide/ide-atapi.c| 21 ++-
drivers/ide/ide-cd.c | 76 ++
drivers/ide/ide-cd_ioctl.c | 3 +-
drivers/ide/ide-devsets.c | 3 +-
drivers/ide/ide-disk.c | 9 ++---
drivers/ide/ide-eh.c | 2 +-
drivers/ide/ide-floppy.c | 19 +-
drivers/ide/ide-io.c | 2 +-
drivers/ide/ide-ioctls.c | 6 +--
drivers/ide/ide-park.c | 6 +--
drivers/ide/ide-pm.c | 10 ++---
drivers/ide/ide-tape.c | 5 +--
drivers/ide/ide-taskfile.c | 6 +--
drivers/mtd/mtd_blkdevs.c | 13 +++
drivers/mtd/ubi/block.c| 15
drivers/nvme/host/core.c | 26 +
drivers/nvme/host/rdma.c | 2 +-
drivers/scsi/osd/osd_initiator.c | 3 +-
drivers/scsi/osst.c| 3 +-
drivers/scsi/scsi_error.c | 2 +-
drivers/scsi/scsi_lib.c| 30 +++
drivers/scsi/sg.c | 3 +-
drivers/scsi/sr.c | 9 +++--
drivers/scsi/st.c | 6 +--
drivers/target/target_core_pscsi.c | 3 +-
fs/nfsd/blocklayout.c | 2 +-
include/linux/blk_types.h | 7
include/linux/blkdev.h | 22 +--
include/linux/ide.h| 14 +++
53 files changed, 338 insertions(+), 326 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 2b37502..9a2dd71 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1227,9 +1227,6 @@ struct bio *bio_copy_user_iov(struct request_queue *q,
if (!bio)
goto out_bmd;
- if (iter->type & WRITE)
- bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
-
ret = 0;
if (map_data) {
@@ -1394,12 +1391,6 @@ struct bio *bio_map_user_iov(struct request_queue *q,
kfree(pages);
- /*
-* set data direction, and check if mapped pages need bouncing
-*/
- if (iter->type & WRITE)
- bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
-
bio_set_flag(bio, BIO_USER_MAPPED);
/*
@@ -1590,7 +1581,6 @@ struct bio *bio_copy_kern(struct request_queue *q, void
*data, unsigned int len,
bio->bi_private = data;
} else {
bio->bi_end_io = bio_copy_kern_endio;
- bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
}
return bio;
diff --git a/block/blk-core.c b/block/blk-core.c
index f5c9f40..e1fc4f7f 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -158,8 +158,8 @@ static void req_bio_endio(struct request *rq, struct bio
*bio,
void blk_dump_rq_flags(struct request *rq, char *msg)
{
- printk(KERN_INFO "%s: dev %s: type=%x, flags=%llx\n", msg,
- rq->rq_disk ? rq->rq_disk->disk_name : "?", rq->cmd_type,
+ printk(KERN_INFO "%s: dev %s: flags=%llx\n", msg,
+ rq->rq_disk ? rq->rq_disk->disk_name : "?",
(unsigned long long) rq->cmd_flags);
printk(KERN_INFO " sector %llu, nr/cnr %u/%u\n",
@@ -1593,7 +1593,6 @@ unsigned int blk_plug_queued_count(struct request_queue
*q)
void init_request_from_bio(struct request *req, struct bio *bio)
{
- req->cmd_type = REQ_TYPE_FS;
[PATCH 08/10] block: introduce blk_rq_is_passthrough
This can be used to check for fs vs non-fs requests and basically
removes all knowledge of BLOCK_PC specific from the block layer,
as well as preparing for removing the cmd_type field in struct request.
Signed-off-by: Christoph Hellwig
---
block/blk-core.c | 8
block/blk-exec.c | 2 +-
block/blk.h | 2 +-
block/elevator.c | 4 ++--
block/mq-deadline.c | 2 +-
drivers/ata/libata-scsi.c | 2 +-
drivers/block/cciss.c | 36 +--
drivers/nvme/host/fc.c| 2 +-
drivers/nvme/host/pci.c | 4 ++--
drivers/nvme/host/rdma.c | 4 ++--
drivers/nvme/target/loop.c| 2 +-
drivers/scsi/hpsa.c | 4 ++--
drivers/scsi/scsi.c | 2 +-
drivers/scsi/scsi_error.c | 4 ++--
drivers/scsi/scsi_lib.c | 6 +++---
drivers/scsi/smartpqi/smartpqi_init.c | 2 +-
drivers/scsi/sun3_scsi.c | 2 +-
include/linux/blkdev.h| 16 +++-
include/linux/blktrace_api.h | 4 ++--
include/scsi/scsi_cmnd.h | 2 +-
kernel/trace/blktrace.c | 2 +-
21 files changed, 59 insertions(+), 53 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index d0c50fc..f5c9f40 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2506,10 +2506,10 @@ bool blk_update_request(struct request *req, int error,
unsigned int nr_bytes)
* TODO: tj: This is too subtle. It would be better to let
* low level drivers do what they see fit.
*/
- if (req->cmd_type == REQ_TYPE_FS)
+ if (!blk_rq_is_passthrough(req))
req->errors = 0;
- if (error && req->cmd_type == REQ_TYPE_FS &&
+ if (error && !blk_rq_is_passthrough(req) &&
!(req->rq_flags & RQF_QUIET)) {
char *error_type;
@@ -2581,7 +2581,7 @@ bool blk_update_request(struct request *req, int error,
unsigned int nr_bytes)
req->__data_len -= total_bytes;
/* update sector only for requests with clear definition of sector */
- if (req->cmd_type == REQ_TYPE_FS)
+ if (!blk_rq_is_passthrough(req))
req->__sector += total_bytes >> 9;
/* mixed attributes always follow the first bio */
@@ -2659,7 +2659,7 @@ void blk_finish_request(struct request *req, int error)
BUG_ON(blk_queued_rq(req));
- if (unlikely(laptop_mode) && req->cmd_type == REQ_TYPE_FS)
+ if (unlikely(laptop_mode) && !blk_rq_is_passthrough(req))
laptop_io_completion(&req->q->backing_dev_info);
blk_delete_timer(req);
diff --git a/block/blk-exec.c b/block/blk-exec.c
index ed51800..8cd0e9b 100644
--- a/block/blk-exec.c
+++ b/block/blk-exec.c
@@ -51,7 +51,7 @@ void blk_execute_rq_nowait(struct request_queue *q, struct
gendisk *bd_disk,
int where = at_head ? ELEVATOR_INSERT_FRONT : ELEVATOR_INSERT_BACK;
WARN_ON(irqs_disabled());
- WARN_ON(rq->cmd_type == REQ_TYPE_FS);
+ WARN_ON(!blk_rq_is_passthrough(rq));
rq->rq_disk = bd_disk;
rq->end_io = done;
diff --git a/block/blk.h b/block/blk.h
index 9a716b5..c1bd4bf 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -249,7 +249,7 @@ static inline int blk_do_io_stat(struct request *rq)
{
return rq->rq_disk &&
(rq->rq_flags & RQF_IO_STAT) &&
- (rq->cmd_type == REQ_TYPE_FS);
+ !blk_rq_is_passthrough(rq);
}
/*
diff --git a/block/elevator.c b/block/elevator.c
index ef7f594..dba9be8 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -634,7 +634,7 @@ void __elv_add_request(struct request_queue *q, struct
request *rq, int where)
if (rq->rq_flags & RQF_SOFTBARRIER) {
/* barriers are scheduling boundary, update end_sector */
- if (rq->cmd_type == REQ_TYPE_FS) {
+ if (!blk_rq_is_passthrough(rq)) {
q->end_sector = rq_end_sector(rq);
q->boundary_rq = rq;
}
@@ -676,7 +676,7 @@ void __elv_add_request(struct request_queue *q, struct
request *rq, int where)
if (elv_attempt_insert_merge(q, rq))
break;
case ELEVATOR_INSERT_SORT:
- BUG_ON(rq->cmd_type != REQ_TYPE_FS);
+ BUG_ON(blk_rq_is_passthrough(rq));
rq->rq_flags |= RQF_SORTED;
q->nr_sorted++;
if (rq_mergeable(rq)) {
diff --git a/block/mq-deadline.c b/block/mq-deadline.c
index d93ec71..4958353 100644
--- a/block/mq-deadline.c
+++ b/block/mq-deadline.c
@@ -398,7 +398,7 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx,
struct request *rq,
if (blk_mq_sched_bypass_insert(hctx, rq))
return;
- if (at_head || rq->cmd_type != REQ_TYPE_FS) {
+
Re: [PATCH 09/10] ide: don't abuse cmd_type
From: Christoph Hellwig Date: Tue, 31 Jan 2017 16:57:30 +0100 > Currently the legacy ide driver defines several request types of it's own, > which is in the way of removing that field entirely. > > Instead add a type field to struct ide_request and use that to distinguish > the different types of IDE-internal requests. > > It's a bit of a mess, but so is the surrounding code.. > > Signed-off-by: Christoph Hellwig Acked-by: David S. Miller -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/10] ѕd: remove pointless REQ_TYPE_FS check
->done can only be called for fs requests, so no need to check again here. Signed-off-by: Christoph Hellwig --- drivers/scsi/sd.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index c779986..bd2fb4d 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1735,9 +1735,6 @@ static unsigned int sd_completed_bytes(struct scsi_cmnd *scmd) unsigned int transferred = scsi_bufflen(scmd) - scsi_get_resid(scmd); unsigned int good_bytes; - if (scmd->request->cmd_type != REQ_TYPE_FS) - return 0; - info_valid = scsi_get_sense_info_fld(scmd->sense_buffer, SCSI_SENSE_BUFFERSIZE, &bad_lba); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/10] mmc: remove pointless request type check in mmc_prep_request
The block layer won't send requests the driver isn't asking for,
so remove this check.
Signed-off-by: Christoph Hellwig
---
drivers/mmc/core/queue.c | 9 -
1 file changed, 9 deletions(-)
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index a6496d8..033f641 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -30,15 +30,6 @@ static int mmc_prep_request(struct request_queue *q, struct
request *req)
{
struct mmc_queue *mq = q->queuedata;
- /*
-* We only like normal block requests and discards.
-*/
- if (req->cmd_type != REQ_TYPE_FS && req_op(req) != REQ_OP_DISCARD &&
- req_op(req) != REQ_OP_SECURE_ERASE) {
- blk_dump_rq_flags(req, "MMC bad request");
- return BLKPREP_KILL;
- }
-
if (mq && (mmc_card_removed(mq->card) || mmc_access_rpmb(mq)))
return BLKPREP_KILL;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/10] ms_block: remove pointless prep_fn
This driver will never see non-fs requests, and doesn't do anything
else in the prep_fn.
Signed-off-by: Christoph Hellwig
---
drivers/memstick/core/ms_block.c | 11 ---
1 file changed, 11 deletions(-)
diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index f3512404..99e651c 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -2000,16 +2000,6 @@ static int msb_bd_getgeo(struct block_device *bdev,
return 0;
}
-static int msb_prepare_req(struct request_queue *q, struct request *req)
-{
- if (req->cmd_type != REQ_TYPE_FS) {
- blk_dump_rq_flags(req, "MS unsupported request");
- return BLKPREP_KILL;
- }
- req->rq_flags |= RQF_DONTPREP;
- return BLKPREP_OK;
-}
-
static void msb_submit_req(struct request_queue *q)
{
struct memstick_dev *card = q->queuedata;
@@ -2132,7 +2122,6 @@ static int msb_init_disk(struct memstick_dev *card)
}
msb->queue->queuedata = card;
- blk_queue_prep_rq(msb->queue, msb_prepare_req);
blk_queue_bounce_limit(msb->queue, limit);
blk_queue_max_hw_sectors(msb->queue, MS_BLOCK_MAX_PAGES);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/10] mspro_block: remove pointless prep_fn
This driver will never see non-fs requests, and doesn't do anything
else in the prep_fn.
Signed-off-by: Christoph Hellwig
---
drivers/memstick/core/mspro_block.c | 13 -
1 file changed, 13 deletions(-)
diff --git a/drivers/memstick/core/mspro_block.c
b/drivers/memstick/core/mspro_block.c
index fa0746d..c00d8a2 100644
--- a/drivers/memstick/core/mspro_block.c
+++ b/drivers/memstick/core/mspro_block.c
@@ -827,18 +827,6 @@ static void mspro_block_start(struct memstick_dev *card)
spin_unlock_irqrestore(&msb->q_lock, flags);
}
-static int mspro_block_prepare_req(struct request_queue *q, struct request
*req)
-{
- if (req->cmd_type != REQ_TYPE_FS) {
- blk_dump_rq_flags(req, "MSPro unsupported request");
- return BLKPREP_KILL;
- }
-
- req->rq_flags |= RQF_DONTPREP;
-
- return BLKPREP_OK;
-}
-
static void mspro_block_submit_req(struct request_queue *q)
{
struct memstick_dev *card = q->queuedata;
@@ -1228,7 +1216,6 @@ static int mspro_block_init_disk(struct memstick_dev
*card)
}
msb->queue->queuedata = card;
- blk_queue_prep_rq(msb->queue, mspro_block_prepare_req);
blk_queue_bounce_limit(msb->queue, limit);
blk_queue_max_hw_sectors(msb->queue, MSPRO_BLOCK_MAX_PAGES);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/10] nbd: move request validity checking into nbd_send_cmd
This is where we do the rest of the request handling, which will
become much simpler soon, too.
Signed-off-by: Christoph Hellwig
---
drivers/block/nbd.c | 21 ++---
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index d4c6281..8bce1c7c 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -271,6 +271,9 @@ static int nbd_send_cmd(struct nbd_device *nbd, struct
nbd_cmd *cmd, int index)
u32 type;
u32 tag = blk_mq_unique_tag(req);
+ if (req->cmd_type != REQ_TYPE_FS)
+ return -EIO;
+
if (req_op(req) == REQ_OP_DISCARD)
type = NBD_CMD_TRIM;
else if (req_op(req) == REQ_OP_FLUSH)
@@ -280,6 +283,13 @@ static int nbd_send_cmd(struct nbd_device *nbd, struct
nbd_cmd *cmd, int index)
else
type = NBD_CMD_READ;
+ if (rq_data_dir(req) == WRITE &&
+ (nbd->flags & NBD_FLAG_READ_ONLY)) {
+ dev_err_ratelimited(disk_to_dev(nbd->disk),
+ "Write on read-only\n");
+ return -EIO;
+ }
+
memset(&request, 0, sizeof(request));
request.magic = htonl(NBD_REQUEST_MAGIC);
request.type = htonl(type);
@@ -503,17 +513,6 @@ static void nbd_handle_cmd(struct nbd_cmd *cmd, int index)
goto error_out;
}
- if (req->cmd_type != REQ_TYPE_FS)
- goto error_out;
-
- if (req->cmd_type == REQ_TYPE_FS &&
- rq_data_dir(req) == WRITE &&
- (nbd->flags & NBD_FLAG_READ_ONLY)) {
- dev_err_ratelimited(disk_to_dev(nbd->disk),
- "Write on read-only\n");
- goto error_out;
- }
-
req->errors = 0;
nsock = nbd->socks[index];
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/10] scm_blk: remove unneeded REQ_TYPE_FS check
Signed-off-by: Christoph Hellwig
---
drivers/s390/block/scm_blk.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/s390/block/scm_blk.c b/drivers/s390/block/scm_blk.c
index 9f16ea6..152de68 100644
--- a/drivers/s390/block/scm_blk.c
+++ b/drivers/s390/block/scm_blk.c
@@ -300,13 +300,6 @@ static void scm_blk_request(struct request_queue *rq)
struct request *req;
while ((req = blk_peek_request(rq))) {
- if (req->cmd_type != REQ_TYPE_FS) {
- blk_start_request(req);
- blk_dump_rq_flags(req, KMSG_COMPONENT " bad request");
- __blk_end_request_all(req, -EIO);
- continue;
- }
-
if (!scm_permit_request(bdev, req))
goto out;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/10] nbd: remove REQ_TYPE_DRV_PRIV leftovers
Disconnects don't use block layer requests these days, so all handling of private requests is dead code. Signed-off-by: Christoph Hellwig --- drivers/block/nbd.c | 10 +- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 9fd06ee..d4c6281 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -193,13 +193,6 @@ static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req, set_bit(NBD_TIMEDOUT, &nbd->runtime_flags); req->errors++; - /* -* If our disconnect packet times out then we're already holding the -* config_lock and could deadlock here, so just set an error and return, -* we'll handle shutting everything down later. -*/ - if (req->cmd_type == REQ_TYPE_DRV_PRIV) - return BLK_EH_HANDLED; mutex_lock(&nbd->config_lock); sock_shutdown(nbd); mutex_unlock(&nbd->config_lock); @@ -510,8 +503,7 @@ static void nbd_handle_cmd(struct nbd_cmd *cmd, int index) goto error_out; } - if (req->cmd_type != REQ_TYPE_FS && - req->cmd_type != REQ_TYPE_DRV_PRIV) + if (req->cmd_type != REQ_TYPE_FS) goto error_out; if (req->cmd_type == REQ_TYPE_FS && -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
remove the cmd_type field from struct request
Hi Jens, this series gets rid of the cmd_type field in struct request and instead folds it into the REQ_OP* space. This reduces the size of struct request, and leads to a single op namespace that drivers can easily switch on. Except for the legacy ide driver which has a mess of different request types [1] this also nicely cleans up the code. Note that the patches are on top of the "make SCSI passthrough support optional" series I sent out on Saturday. To make life easier I also have a git tree available here: git://git.infradead.org/users/hch/block.git cmd_type http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/cmd_type [1] which were a pain in the ass to untangle and debug during development, it's really time for it to die.. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert "sd: remove __data_len hack for WRITE SAME"
Acked-by: Lee Duncan -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 2/2] qedf: Add QLogic FastLinQ offload FCoE driver framework.
On Mon, 30 Jan 2017, 10:34am -, Hannes Reinecke wrote:
> On 01/25/2017 09:33 PM, Dupuis, Chad wrote:
> > From: "Dupuis, Chad"
> >
> > The QLogic FastLinQ Driver for FCoE (qedf) is the FCoE specific module for
> > 41000
> > Series Converged Network Adapters by QLogic. This patch consists of
> > following
> > changes:
> >
> > - MAINTAINERS Makefile and Kconfig changes for qedf
> > - PCI driver registration
> > - libfc/fcoe host level initialization
> > - SCSI host template initialization and callbacks
> > - Debugfs and log level infrastructure
> > - Link handling
> > - Firmware interface structures
> > - QED core module initialization
> > - Light L2 interface callbacks
> > - I/O request initialization
> > - Firmware I/O completion handling
> > - Firmware ELS request/response handling
> > - FIP request/response handled by the driver itself
> >
> > Signed-off-by: Nilesh Javali
> > Signed-off-by: Manish Rangankar
> > Signed-off-by: Saurav Kashyap
> > Signed-off-by: Arun Easi
> > Signed-off-by: Chad Dupuis
> > ---
> [ .. ]
> > +#define QEDF_IO_WORK_MIN 64
> > + mempool_t *io_mempool;
> > + struct workqueue_struct *dpc_wq;
> > +
> > + u32 slow_sge_ios;
> > + u32 fast_sge_ios;
> > + u32 single_sge_ios;
> > +
> > + uint8_t *grcdump;
> > + uint32_t grcdump_size;
> > +
> > + struct qedf_io_log io_trace_buf[QEDF_IO_TRACE_SIZE];
> > + spinlock_t io_trace_lock;
> > + uint16_t io_trace_idx;
> > +
> > + bool stop_io_on_error;
> > +
> > + u32 flogi_cnt;
> > + u32 flogi_failed;
> > +
> > + /* Used for fc statistics */
> > + u64 input_requests;
> > + u64 output_requests;
> > + u64 control_requests;
> > + u64 packet_aborts;
> > + u64 alloc_failures;
> > +};
> > +
> > +/*
> > + * 4 regs size $$KEEP_ENDIANNESS$$
> > + */
> > +
> What is this supposed to mean?
> Does it refer to the next structure?
I think it's a superfluous comment. I'll remove it.
>
> > +struct io_bdt {
> > + struct qedf_ioreq *io_req;
> > + struct fcoe_sge *bd_tbl;
> > + dma_addr_t bd_tbl_dma;
> > + u16 bd_valid;
> > +};
> > +
> [ .. ]
>
> > +
> > +static inline void qedf_stop_all_io(struct qedf_ctx *qedf)
> > +{
> > + set_bit(QEDF_UNLOADING, &qedf->flags);
> > +}
> > +
> This looks like a misnomer.
> Why is 'UNLOADING' equivalent to stopping all I/O?
> I could imagine quite some situations where I would want to stop I/O
> without unloading the driver.
>
> Please use better naming here; either name the bit 'QEDF_STOP_IO' or
> something or call the function 'qedf_is_unloading'.
>
This is more a debugfs mechanism to be able to stop i/o from a script or
other outside entity. I was piggy-backing off of functionality that stops
I/O submission while the driver is unloading but I'll add a define
specific to debug stopping of I/O.
>
> [ .. ]
> > +/*
> > + * In instances where an ELS command times out we may need to restart the
> > + * rport by logging out and then logging back in.
> > + */
> > +void qedf_restart_rport(struct qedf_rport *fcport)
> > +{
> > + struct fc_lport *lport;
> > + struct fc_rport_priv *rdata;
> > + u32 port_id;
> > +
> > + if (!fcport)
> > + return;
> > +
> > + rdata = fcport->rdata;
> > + if (rdata) {
> > + lport = fcport->qedf->lport;
> > + port_id = rdata->ids.port_id;
> > + QEDF_ERR(&(fcport->qedf->dbg_ctx),
> > + "LOGO port_id=%x.\n", port_id);
> > + mutex_lock(&lport->disc.disc_mutex);
> > + fc_rport_logoff(rdata);
> > + /* Recreate the rport and log back in */
> > + rdata = fc_rport_create(lport, port_id);
> > + if (rdata)
> > + fc_rport_login(rdata);
> > + mutex_unlock(&lport->disc.disc_mutex);
> > + }
> > +}
> > +
> Please don't hold the discovery mutex when calling fc_rport_logoff().
> Calling rport_logoff might call into exch_mgr_reset() which in turn
> might need to take the discover mutex.
Looking at other code it looked like this mutex was held when logging off
the port and then logging on. I'll remove it however.
>
> [ .. ]
> > + if (opcode == ELS_LS_RJT) {
> > + rjt = fc_frame_payload_get(fp, sizeof(*rjt));
> > + QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_ELS,
> > + "Received LS_RJT for REC: er_reason=0x%x, "
> > + "er_explan=0x%x.\n", rjt->er_reason, rjt->er_explan);
> > + /*
> > +* The following response(s) mean that we need to reissue the
> > +* request on another exchange. We need to do this without
> > +* informing the upper layers lest it cause an application
> > +* error.
> > +*/
> > + if ((rjt->er_reason == ELS_RJT_LOGIC ||
> > + rjt->er_reason == ELS_RJT_UNAB) &&
> > + rjt->er_explan == ELS_EXPL_OXID_RXID) {
> > + QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_ELS,
> > + "Handle CMD LOST case.\n");
> > +
RE: [PATCH 00/10] mpt3sas: full mq support
> -Original Message- > From: Hannes Reinecke [mailto:h...@suse.de] > Sent: Tuesday, January 31, 2017 4:47 PM > To: Christoph Hellwig > Cc: Martin K. Petersen; James Bottomley; linux-scsi@vger.kernel.org; Sathya > Prakash; Kashyap Desai; mpt-fusionlinux@broadcom.com > Subject: Re: [PATCH 00/10] mpt3sas: full mq support > > On 01/31/2017 11:02 AM, Christoph Hellwig wrote: > > On Tue, Jan 31, 2017 at 10:25:50AM +0100, Hannes Reinecke wrote: > >> Hi all, > >> > >> this is a patchset to enable full multiqueue support for the mpt3sas > driver. > >> While the HBA only has a single mailbox register for submitting > >> commands, it does have individual receive queues per MSI-X interrupt > >> and as such does benefit from converting it to full multiqueue support. > > > > Explanation and numbers on why this would be beneficial, please. > > We should not need multiple submissions queues for a single register > > to benefit from multiple completion queues. > > > Well, the actual throughput very strongly depends on the blk-mq-sched > patches from Jens. > As this is barely finished I didn't post any numbers yet. > > However: > With multiqueue support: > 4k seq read : io=60573MB, bw=1009.2MB/s, iops=258353, runt= 60021msec > With scsi-mq on 1 queue: > 4k seq read : io=17369MB, bw=296291KB/s, iops=74072, runt= 60028msec So > yes, there _is_ a benefit. > > (Which is actually quite cool, as these tests were done on a SAS3 HBA, so > we're getting close to the theoretical maximum of 1.2GB/s). > (Unlike the single-queue case :-) Hannes - Can you share detail about setup ? How many drives do you have and how is connection (enclosure -> drives. ??) ? To me it looks like current mpt3sas driver might be taking more hit in spinlock operation (penalty on NUMA arch is more compare to single core server) unlike we have in megaraid_sas driver use of shared blk tag. I mean " [PATCH 08/10] mpt3sas: lockless command submission for scsi-mq" patch is improving performance removing spinlock overhead and attempting to get request using blk_tags. Are you seeing performance improvement if you hard code nr_hw_queues = 1 in below code changes part of "[PATCH 10/10] mpt3sas: scsi-mq interrupt steering" @@ -9054,6 +9071,8 @@ static void sas_device_make_active(struct MPT3SAS_ADAPTER *ioc, shost->max_lun = max_lun; shost->transportt = mpt3sas_transport_template; shost->unique_id = ioc->id; + if (shost->use_blk_mq) + shost->nr_hw_queues = num_online_cpus(); Thanks, Kashyap > > Cheers, > > Hannes > -- > Dr. Hannes Reinecke Teamlead Storage & Networking > h...@suse.de +49 911 74053 688 > SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg > GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 > (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: remove the cmd_type field from struct request
On 01/31/2017 07:57 AM, Christoph Hellwig wrote: > [1] which were a pain in the ass to untangle and debug during development, > it's really time for it to die.. Outside of the patch series in question, how to we expedite the euthanasia of IDE? What explicit features/support are we missing through libata that would need to be added, before we can git rm drivers/ide/ ? -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 2/2] qedf: Add QLogic FastLinQ offload FCoE driver framework.
On Mon, 30 Jan 2017, 10:34am -, Hannes Reinecke wrote:
> On 01/25/2017 09:33 PM, Dupuis, Chad wrote:
> > +static int qedf_request_msix_irq(struct qedf_ctx *qedf)
> > +{
> > + int i, rc, cpu;
> > +
> > + cpu = cpumask_first(cpu_online_mask);
> > + for (i = 0; i < qedf->num_queues; i++) {
> > + rc = request_irq(qedf->int_info.msix[i].vector,
> > + qedf_msix_handler, 0, "qedf", &qedf->fp_array[i]);
> > +
> > + if (rc) {
> > + QEDF_WARN(&(qedf->dbg_ctx), "request_irq failed.\n");
> > + qedf_sync_free_irqs(qedf);
> > + return rc;
> > + }
> > +
> > + qedf->int_info.used_cnt++;
> > + rc = irq_set_affinity_hint(qedf->int_info.msix[i].vector,
> > + get_cpu_mask(cpu));
> > + cpu = cpumask_next(cpu, cpu_online_mask);
> > + }
> > +
> > + return 0;
> > +}
> > +
> Please use pci_alloc_irq_vectors() here; that avoid you having to do SMP
> affinity setting yourself.
This wil be difficult to coordinate with three other drivers (qedi, qede
and qedr) using the same vector allocation code in the qed module.
>
> Cheers,
>
> Hannes
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: remove the cmd_type field from struct request
On Tue, 2017-01-31 at 10:02 -0800, Jens Axboe wrote: > On 01/31/2017 07:57 AM, Christoph Hellwig wrote: > > [1] which were a pain in the ass to untangle and debug during > > development, it's really time for it to die.. > > Outside of the patch series in question, how to we expedite the > euthanasia of IDE? What explicit features/support are we missing > through libata that would need to be added, before we can git rm > drivers/ide/? I thought the primary objection was actually embedded in that libata with its reliance on SCSI was just too large a dependency, so they have to keep using drivers/ide. Perhaps nvme and flash is obviating this problem and we can ask them again, though? James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: remove the cmd_type field from struct request
On 01/31/2017 09:51 PM, James Bottomley wrote: [1] which were a pain in the ass to untangle and debug during development, it's really time for it to die.. Outside of the patch series in question, how to we expedite the euthanasia of IDE? What explicit features/support are we missing through libata that would need to be added, before we can git rm drivers/ide/? I thought the primary objection was actually embedded in that libata Back at MontaVista, even embedded uses switched to libata years ago... DaveM thinks we still can't prove that libata works everywhere the IDE works. Besided, there are still a number of non-x86 drivers not converted over to libata (like DaVinci, etc.)... with its reliance on SCSI was just too large a dependency, so they have to keep using drivers/ide. Do you remember how many years ago a libata's own block driver was promised to you? ;-) I'd gladly wrote one, getting tired of my current OSS work... :-) Perhaps nvme and flash is obviating this problem and we can ask them again, though? What's wrong with those? I'm not really following NVMe... James MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4.10-rc3 00/13] net: dsa: remove unnecessary phy.h include
Including phy.h and phy_fixed.h into net/dsa.h causes phy*.h to be an unnecessary dependency for quite a large amount of the kernel. There's very little which actually requires definitions from phy.h in net/dsa.h - the include itself only wants the declaration of a couple of structures and IFNAMSIZ. Add linux/if.h for IFNAMSIZ, declarations for the structures, phy.h to mv88e6xxx.h as it needs it for phy_interface_t, and remove both phy.h and phy_fixed.h from net/dsa.h. This patch reduces from around 800 files rebuilt to around 40 - even with ccache, the time difference is noticable. In order to make this change, several drivers need to be updated to include necessary headers that they were picking up through this include. This has resulted in a much larger patch series. I'm assuming the 0-day builder has had 24 hours with this series, and hasn't reported any further issues with it - the last issue was two weeks ago (before I became ill) which I fixed over the last weekend. I'm hoping this doesn't conflict with what's already in net-next... arch/mips/cavium-octeon/octeon-platform.c | 4 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 1 + drivers/net/ethernet/broadcom/bgmac.c | 2 ++ drivers/net/ethernet/cadence/macb.h | 2 ++ drivers/net/ethernet/cavium/liquidio/lio_main.c | 1 + drivers/net/ethernet/cavium/liquidio/lio_vf_main.c| 1 + drivers/net/ethernet/cavium/liquidio/octeon_console.c | 1 + drivers/net/ethernet/freescale/fman/fman_memac.c | 1 + drivers/net/ethernet/marvell/mvneta.c | 1 + drivers/net/ethernet/qualcomm/emac/emac-sgmii.c | 1 + drivers/net/usb/lan78xx.c | 1 + drivers/net/wireless/ath/ath5k/ahb.c | 2 +- drivers/target/iscsi/iscsi_target_login.c | 1 + include/net/dsa.h | 6 -- net/core/netprio_cgroup.c | 1 + net/sunrpc/xprtrdma/svc_rdma_backchannel.c| 1 + 16 files changed, 20 insertions(+), 7 deletions(-) -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4.10-rc3 13/13] net: dsa: remove unnecessary phy*.h includes
Including phy.h and phy_fixed.h into net/dsa.h causes phy*.h to be an
unnecessary dependency for quite a large amount of the kernel. There's
very little which actually requires definitions from phy.h in net/dsa.h
- the include itself only wants the declaration of a couple of
structures and IFNAMSIZ.
Add linux/if.h for IFNAMSIZ, declarations for the structures, phy.h to
mv88e6xxx.h as it needs it for phy_interface_t, and remove both phy.h
and phy_fixed.h from net/dsa.h.
This patch reduces from around 800 files rebuilt to around 40 - even
with ccache, the time difference is noticable.
Tested-by: Vivien Didelot
Reviewed-by: Florian Fainelli
Signed-off-by: Russell King
---
drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 1 +
include/net/dsa.h | 6 --
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index af54baea47cf..3a949095068a 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#ifndef UINT64_MAX
#define UINT64_MAX (u64)(~((u64)0))
diff --git a/include/net/dsa.h b/include/net/dsa.h
index b122196d5a1f..887b2f98f9ea 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -11,15 +11,17 @@
#ifndef __LINUX_NET_DSA_H
#define __LINUX_NET_DSA_H
+#include
#include
#include
#include
#include
#include
-#include
-#include
#include
+struct phy_device;
+struct fixed_phy_status;
+
enum dsa_tag_protocol {
DSA_TAG_PROTO_NONE = 0,
DSA_TAG_PROTO_DSA,
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4.10-rc3 04/13] net: lan78xx: fix build errors when linux/phy*.h is removed from net/dsa.h
drivers/net/usb/lan78xx.c:394:33: sparse: expected ; at end of declaration drivers/net/usb/lan78xx.c:394:33: sparse: Expected } at end of struct-union-enum-specifier drivers/net/usb/lan78xx.c:394:33: sparse: got interface drivers/net/usb/lan78xx.c:403:1: sparse: Expected ; at the end of type declaration drivers/net/usb/lan78xx.c:403:1: sparse: got } Add linux/phy.h to lan78xx.c Signed-off-by: Russell King --- drivers/net/usb/lan78xx.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index 08f8703e4d54..9889a70ff4f6 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -35,6 +35,7 @@ #include #include #include +#include #include "lan78xx.h" #define DRIVER_AUTHOR "WOOJUNG HUH " -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4.10-rc3 12/13] net: ath5k: fix build errors when linux/phy*.h is removed from net/dsa.h
Fix these errors reported by the 0-day builder by replacing the linux/export.h include with linux/module.h. In file included from include/linux/platform_device.h:14:0, from drivers/net/wireless/ath/ath5k/ahb.c:20: include/linux/device.h:1463:1: warning: data definition has no type or storage class module_init(__driver##_init); \ ^ include/linux/platform_device.h:228:2: note: in expansion of macro 'module_driver' module_driver(__platform_driver, platform_driver_register, \ ^ drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 'module_platform_driver' module_platform_driver(ath_ahb_driver); ^~ include/linux/device.h:1463:1: error: type defaults to 'int' in declaration of 'module_init' [-Werror=implicit-int] module_init(__driver##_init); \ ^ include/linux/platform_device.h:228:2: note: in expansion of macro 'module_driver' module_driver(__platform_driver, platform_driver_register, \ ^ drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 'module_platform_driver' module_platform_driver(ath_ahb_driver); ^~ drivers/net/wireless/ath/ath5k/ahb.c:233:1: warning: parameter names (without types) in function declaration In file included from include/linux/platform_device.h:14:0, from drivers/net/wireless/ath/ath5k/ahb.c:20: include/linux/device.h:1468:1: warning: data definition has no type or storage class module_exit(__driver##_exit); ^ include/linux/platform_device.h:228:2: note: in expansion of macro 'module_driver' module_driver(__platform_driver, platform_driver_register, \ ^ drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 'module_platform_driver' module_platform_driver(ath_ahb_driver); ^~ include/linux/device.h:1468:1: error: type defaults to 'int' in declaration of 'module_exit' [-Werror=implicit-int] module_exit(__driver##_exit); ^ include/linux/platform_device.h:228:2: note: in expansion of macro 'module_driver' module_driver(__platform_driver, platform_driver_register, \ ^ drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 'module_platform_driver' module_platform_driver(ath_ahb_driver); ^~ drivers/net/wireless/ath/ath5k/ahb.c:233:1: warning: parameter names (without types) in function declaration In file included from include/linux/platform_device.h:14:0, from drivers/net/wireless/ath/ath5k/ahb.c:20: drivers/net/wireless/ath/ath5k/ahb.c:233:24: warning: 'ath_ahb_driver_exit' defined but not used [-Wunused-function] module_platform_driver(ath_ahb_driver); ^ include/linux/device.h:1464:20: note: in definition of macro 'module_driver' static void __exit __driver##_exit(void) \ ^~~~ drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 'module_platform_driver' module_platform_driver(ath_ahb_driver); ^~ drivers/net/wireless/ath/ath5k/ahb.c:233:24: warning: 'ath_ahb_driver_init' defined but not used [-Wunused-function] module_platform_driver(ath_ahb_driver); ^ include/linux/device.h:1459:19: note: in definition of macro 'module_driver' static int __init __driver##_init(void) \ ^~~~ drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 'module_platform_driver' module_platform_driver(ath_ahb_driver); ^~ Signed-off-by: Russell King --- drivers/net/wireless/ath/ath5k/ahb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath5k/ahb.c b/drivers/net/wireless/ath/ath5k/ahb.c index 2ca88b593e4c..c0794f5988b3 100644 --- a/drivers/net/wireless/ath/ath5k/ahb.c +++ b/drivers/net/wireless/ath/ath5k/ahb.c @@ -16,10 +16,10 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ +#include #include #include #include -#include #include #include "ath5k.h" #include "debug.h" -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4.10-rc3 05/13] net: bgmac: fix build errors when linux/phy*.h is removed from net/dsa.h
drivers/net/ethernet/broadcom/bgmac.c:1015:17: error: dereferencing pointer to incomplete type 'struct mii_bus' drivers/net/ethernet/broadcom/bgmac.c:1185:2: error: implicit declaration of function 'phy_start' [-Werror=implicit-function-declaration] drivers/net/ethernet/broadcom/bgmac.c:1198:2: error: implicit declaration of function 'phy_stop' [-Werror=implicit-function-declaration] drivers/net/ethernet/broadcom/bgmac.c:1239:9: error: implicit declaration of function 'phy_mii_ioctl' [-Werror=implicit-function-declaration] drivers/net/ethernet/broadcom/bgmac.c:1389:28: error: 'phy_ethtool_get_link_ksettings' undeclared here (not in a function) drivers/net/ethernet/broadcom/bgmac.c:1390:28: error: 'phy_ethtool_set_link_ksettings' undeclared here (not in a function) drivers/net/ethernet/broadcom/bgmac.c:1403:13: error: dereferencing pointer to incomplete type 'struct phy_device' drivers/net/ethernet/broadcom/bgmac.c:1417:3: error: implicit declaration of function 'phy_print_status' [-Werror=implicit-function-declaration] drivers/net/ethernet/broadcom/bgmac.c:1424:26: error: storage size of 'fphy_status' isn't known drivers/net/ethernet/broadcom/bgmac.c:1424:9: error: variable 'fphy_status' has initializer but incomplete type drivers/net/ethernet/broadcom/bgmac.c:1425:11: warning: excess elements in struct initializer drivers/net/ethernet/broadcom/bgmac.c:1425:3: error: unknown field 'link' specified in initializer drivers/net/ethernet/broadcom/bgmac.c:1426:12: note: in expansion of macro 'SPEED_1000' drivers/net/ethernet/broadcom/bgmac.c:1426:3: error: unknown field 'speed' specified in initializer drivers/net/ethernet/broadcom/bgmac.c:1427:13: note: in expansion of macro 'DUPLEX_FULL' drivers/net/ethernet/broadcom/bgmac.c:1427:3: error: unknown field 'duplex' specified in initializer drivers/net/ethernet/broadcom/bgmac.c:1432:12: error: implicit declaration of function 'fixed_phy_register' [-Werror=implicit-function-declaration] drivers/net/ethernet/broadcom/bgmac.c:1432:31: error: 'PHY_POLL' undeclared (first use in this function) drivers/net/ethernet/broadcom/bgmac.c:1438:8: error: implicit declaration of function 'phy_connect_direct' [-Werror=implicit-function-declaration] drivers/net/ethernet/broadcom/bgmac.c:1439:6: error: 'PHY_INTERFACE_MODE_MII' undeclared (first use in this function) drivers/net/ethernet/broadcom/bgmac.c:1521:2: error: implicit declaration of function 'phy_disconnect' [-Werror=implicit-function-declaration] drivers/net/ethernet/broadcom/bgmac.c:1541:15: error: expected declaration specifiers or '...' before string constant Add linux/phy.h to bgmac.c Signed-off-by: Russell King --- drivers/net/ethernet/broadcom/bgmac.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c index 0e066dc6b8cc..58a2bd3c0458 100644 --- a/drivers/net/ethernet/broadcom/bgmac.c +++ b/drivers/net/ethernet/broadcom/bgmac.c @@ -12,6 +12,8 @@ #include #include #include +#include +#include #include "bgmac.h" static bool bgmac_wait_value(struct bgmac *bgmac, u16 reg, u32 mask, -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4.10-rc3 11/13] net: liquidio: fix build errors when linux/phy*.h is removed from net/dsa.h
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:30: error: expected declaration specifiers or '...' before string constant drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:30: warning: data definition has no type or storage class drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:30: error: type defaults to 'int' in declaration of 'MODULE_AUTHOR' drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:30: error: function declaration isn't a prototype drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:31: error: expected declaration specifiers or '...' before string constant drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:31: warning: data definition has no type or storage class drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:31: error: type defaults to 'int' in declaration of 'MODULE_DESCRIPTION' drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:31: error: function declaration isn't a prototype drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:32: error: expected declaration specifiers or '...' before string constant drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:32: warning: data definition has no type or storage class drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:32: error: type defaults to 'int' in declaration of 'MODULE_LICENSE' drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:32: error: function declaration isn't a prototype drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:33: error: expected declaration specifiers or '...' before string constant drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:33: warning: data definition has no type or storage class drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:33: error: type defaults to 'int' in declaration of 'MODULE_VERSION' drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:33: error: function declaration isn't a prototype drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:36: error: expected ')' before 'int' drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:37: error: expected ')' before string constant drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:325: warning: data definition has no type or storage class drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:325: error: type defaults to 'int' in declaration of 'MODULE_DEVICE_TABLE' drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:325: warning: parameter names (without types) in function declaration drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3250: warning: data definition has no type or storage class drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3250: error: type defaults to 'int' in declaration of 'module_init' drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3250: warning: parameter names (without types) in function declaration drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3251: warning: data definition has no type or storage class drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3251: error: type defaults to 'int' in declaration of 'module_exit' drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3251: warning: parameter names (without types) in function declaration drivers/net/ethernet/cavium/liquidio/lio_main.c:36: error: expected declaration specifiers or '...' before string constant drivers/net/ethernet/cavium/liquidio/lio_main.c:36: warning: data definition has no type or storage class drivers/net/ethernet/cavium/liquidio/lio_main.c:36: error: type defaults to 'int' in declaration of 'MODULE_AUTHOR' drivers/net/ethernet/cavium/liquidio/lio_main.c:36: error: function declaration isn't a prototype drivers/net/ethernet/cavium/liquidio/lio_main.c:37: error: expected declaration specifiers or '...' before string constant drivers/net/ethernet/cavium/liquidio/lio_main.c:37: warning: data definition has no type or storage class drivers/net/ethernet/cavium/liquidio/lio_main.c:37: error: type defaults to 'int' in declaration of 'MODULE_DESCRIPTION' drivers/net/ethernet/cavium/liquidio/lio_main.c:37: error: function declaration isn't a prototype drivers/net/ethernet/cavium/liquidio/lio_main.c:38: error: expected declaration specifiers or '...' before string constant drivers/net/ethernet/cavium/liquidio/lio_main.c:38: warning: data definition has no type or storage class drivers/net/ethernet/cavium/liquidio/lio_main.c:38: error: type defaults to 'int' in declaration of 'MODULE_LICENSE' drivers/net/ethernet/cavium/liquidio/lio_main.c:38: error: function declaration isn't a prototype drivers/net/ethernet/cavium/liquidio/lio_main.c:39: error: expected declaration specifiers or '...' before string constant drivers/net/ethernet/cavium/liquidio/lio_main.c:39: warning: data definition has no type or storage class drivers/net/ethernet/cavium/liquidio/lio_main.c:39: error: type defaults to 'int' in declaration of 'MODULE_VERSION' drivers/net/ethernet/cavium/liquidio/lio_main.c:39: error: function declaration isn't a prototype drivers/net/ethernet/cavium/liquidio/lio_main.c:40: error: expected declaration specifiers or '...' be
[PATCH 4.10-rc3 10/13] MIPS: Octeon: Remove unnecessary MODULE_*()
octeon-platform.c can not be built as a module for two reasons:
(a) the Makefile doesn't allow it:
obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o
(b) the multiple *_initcall() statements, each of which are translated
to a module_init() call when attempting a module build, become
aliases to init_module(). Having more than one alias will cause a
build error.
Hence, rather than adding a linux/module.h include, remove the redundant
MODULE_*() from this file.
Acked-by: David Daney
Signed-off-by: Russell King
---
arch/mips/cavium-octeon/octeon-platform.c | 4
1 file changed, 4 deletions(-)
diff --git a/arch/mips/cavium-octeon/octeon-platform.c
b/arch/mips/cavium-octeon/octeon-platform.c
index 37a932d9148c..8297ce714c5e 100644
--- a/arch/mips/cavium-octeon/octeon-platform.c
+++ b/arch/mips/cavium-octeon/octeon-platform.c
@@ -1060,7 +1060,3 @@ static int __init octeon_publish_devices(void)
return of_platform_bus_probe(NULL, octeon_ids, NULL);
}
arch_initcall(octeon_publish_devices);
-
-MODULE_AUTHOR("David Daney ");
-MODULE_LICENSE("GPL");
-MODULE_DESCRIPTION("Platform driver for Octeon SOC");
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4.10-rc3 01/13] net: sunrpc: fix build errors when linux/phy*.h is removed from net/dsa.h
Removing linux/phy.h from net/dsa.h reveals a build error in the sunrpc code: net/sunrpc/xprtrdma/svc_rdma_backchannel.c: In function 'xprt_rdma_bc_put': net/sunrpc/xprtrdma/svc_rdma_backchannel.c:277:2: error: implicit declaration of function 'module_put' [-Werror=implicit-function-declaration] net/sunrpc/xprtrdma/svc_rdma_backchannel.c: In function 'xprt_setup_rdma_bc': net/sunrpc/xprtrdma/svc_rdma_backchannel.c:348:7: error: implicit declaration of function 'try_module_get' [-Werror=implicit-function-declaration] Fix this by adding linux/module.h to svc_rdma_backchannel.c Signed-off-by: Russell King --- net/sunrpc/xprtrdma/svc_rdma_backchannel.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c index 288e35c2d8f4..cb1e48e54eb1 100644 --- a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c +++ b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c @@ -4,6 +4,7 @@ * Support for backward direction RPCs on RPC/RDMA (server-side). */ +#include #include #include "xprt_rdma.h" -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4.10-rc3 09/13] iscsi: fix build errors when linux/phy*.h is removed from net/dsa.h
drivers/target/iscsi/iscsi_target_login.c:1135:7: error: implicit declaration of function 'try_module_get' [-Werror=implicit-function-declaration] Add linux/module.h to iscsi_target_login.c. Signed-off-by: Russell King --- drivers/target/iscsi/iscsi_target_login.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c index 450f51deb2a2..eab274d17b5c 100644 --- a/drivers/target/iscsi/iscsi_target_login.c +++ b/drivers/target/iscsi/iscsi_target_login.c @@ -17,6 +17,7 @@ **/ #include +#include #include #include #include -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4.10-rc3 08/13] net: emac: fix build errors when linux/phy*.h is removed from net/dsa.h
drivers/net/ethernet/qualcomm/emac/emac-sgmii.c:58:12: error: dereferencing pointer to incomplete type 'struct phy_device' Add linux/phy.h to emac-sgmii.c Signed-off-by: Russell King --- drivers/net/ethernet/qualcomm/emac/emac-sgmii.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c b/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c index bf722a9bb09d..5e31fb7e4ab8 100644 --- a/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c +++ b/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c @@ -16,6 +16,7 @@ #include #include #include +#include #include "emac.h" #include "emac-mac.h" #include "emac-sgmii.h" -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4.10-rc3 06/13] net: fman: fix build errors when linux/phy*.h is removed from net/dsa.h
drivers/net/ethernet/freescale/fman/fman_memac.c:519:21: error: dereferencing pointer to incomplete type 'struct fixed_phy_status' Add linux/phy_fixed.h to fman_memac.c Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fman/fman_memac.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c b/drivers/net/ethernet/freescale/fman/fman_memac.c index 71a5ded9d1de..cd6a53eaf161 100644 --- a/drivers/net/ethernet/freescale/fman/fman_memac.c +++ b/drivers/net/ethernet/freescale/fman/fman_memac.c @@ -38,6 +38,7 @@ #include #include #include +#include #include /* PCS registers */ -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4.10-rc3 07/13] net: mvneta: fix build errors when linux/phy*.h is removed from net/dsa.h
drivers/net/ethernet/marvell/mvneta.c:2694:26: error: storage size of 'status' isn't known drivers/net/ethernet/marvell/mvneta.c:2695:26: error: storage size of 'changed' isn't known drivers/net/ethernet/marvell/mvneta.c:2695:9: error: variable 'changed' has initializer but incomplete type drivers/net/ethernet/marvell/mvneta.c:2709:2: error: implicit declaration of function 'fixed_phy_update_state' [-Werror=implicit-function-declaration] Add linux/phy_fixed.h to mvneta.c Signed-off-by: Russell King --- drivers/net/ethernet/marvell/mvneta.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c index e05e22705cf7..eb0eb3e62ca0 100644 --- a/drivers/net/ethernet/marvell/mvneta.c +++ b/drivers/net/ethernet/marvell/mvneta.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4.10-rc3 03/13] net: macb: fix build errors when linux/phy*.h is removed from net/dsa.h
drivers/net/ethernet/cadence/macb.h:862:33: sparse: expected ; at end of declaration drivers/net/ethernet/cadence/macb.h:862:33: sparse: Expected } at end of struct-union-enum-specifier drivers/net/ethernet/cadence/macb.h:862:33: sparse: got phy_interface drivers/net/ethernet/cadence/macb.h:877:1: sparse: Expected ; at the end of type declaration drivers/net/ethernet/cadence/macb.h:877:1: sparse: got } In file included from drivers/net/ethernet/cadence/macb_pci.c:29:0: drivers/net/ethernet/cadence/macb.h:862:2: error: unknown type name 'phy_interface_t' phy_interface_t phy_interface; ^~~ Add linux/phy.h to macb.h Signed-off-by: Russell King --- drivers/net/ethernet/cadence/macb.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h index d67adad67be1..383da8cf5f6d 100644 --- a/drivers/net/ethernet/cadence/macb.h +++ b/drivers/net/ethernet/cadence/macb.h @@ -10,6 +10,8 @@ #ifndef _MACB_H #define _MACB_H +#include + #define MACB_GREGS_NBR 16 #define MACB_GREGS_VERSION 2 #define MACB_MAX_QUEUES 8 -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4.10-rc3 02/13] net: cgroups: fix build errors when linux/phy*.h is removed from net/dsa.h
net/core/netprio_cgroup.c:303:16: error: expected declaration specifiers or
'...' before string constant
MODULE_LICENSE("GPL v2");
^~~~
Add linux/module.h to fix this.
Signed-off-by: Russell King
---
net/core/netprio_cgroup.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c
index 2ec86fc552df..756637dc7a57 100644
--- a/net/core/netprio_cgroup.c
+++ b/net/core/netprio_cgroup.c
@@ -13,6 +13,7 @@
#include
#include
+#include
#include
#include
#include
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4.10-rc3 09/13] iscsi: fix build errors when linux/phy*.h is removed from net/dsa.h
On Tue, 2017-01-31 at 19:19 +, Russell King wrote: > drivers/target/iscsi/iscsi_target_login.c:1135:7: error: implicit declaration > of function 'try_module_get' [-Werror=implicit-function-declaration] > > Add linux/module.h to iscsi_target_login.c. > > Signed-off-by: Russell King > --- > drivers/target/iscsi/iscsi_target_login.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/target/iscsi/iscsi_target_login.c > b/drivers/target/iscsi/iscsi_target_login.c > index 450f51deb2a2..eab274d17b5c 100644 > --- a/drivers/target/iscsi/iscsi_target_login.c > +++ b/drivers/target/iscsi/iscsi_target_login.c > @@ -17,6 +17,7 @@ > > **/ > > #include > +#include > #include > #include > #include Reviewed-by: Bart Van Assche -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4.10-rc3 01/13] net: sunrpc: fix build errors when linux/phy*.h is removed from net/dsa.h
Hi Russell, On 01/31/2017 02:18 PM, Russell King wrote: > Removing linux/phy.h from net/dsa.h reveals a build error in the sunrpc > code: > > net/sunrpc/xprtrdma/svc_rdma_backchannel.c: In function 'xprt_rdma_bc_put': > net/sunrpc/xprtrdma/svc_rdma_backchannel.c:277:2: error: implicit declaration > of function 'module_put' [-Werror=implicit-function-declaration] > net/sunrpc/xprtrdma/svc_rdma_backchannel.c: In function 'xprt_setup_rdma_bc': > net/sunrpc/xprtrdma/svc_rdma_backchannel.c:348:7: error: implicit declaration > of function 'try_module_get' [-Werror=implicit-function-declaration] > > Fix this by adding linux/module.h to svc_rdma_backchannel.c > > Signed-off-by: Russell King This patch looks okay to me: Acked-by: Anna Schumaker > --- > net/sunrpc/xprtrdma/svc_rdma_backchannel.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c > b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c > index 288e35c2d8f4..cb1e48e54eb1 100644 > --- a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c > +++ b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c > @@ -4,6 +4,7 @@ > * Support for backward direction RPCs on RPC/RDMA (server-side). > */ > > +#include > #include > #include "xprt_rdma.h" > > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4.10-rc3 08/13] net: emac: fix build errors when linux/phy*.h is removed from net/dsa.h
On 01/31/2017 01:19 PM, Russell King wrote: drivers/net/ethernet/qualcomm/emac/emac-sgmii.c:58:12: error: dereferencing pointer to incomplete type 'struct phy_device' Add linux/phy.h to emac-sgmii.c Signed-off-by: Russell King --- drivers/net/ethernet/qualcomm/emac/emac-sgmii.c | 1 + The version of emac-sgmii.c on net-next does not need this fixed. I already removed all references to phy_device in commit "net: qcom/emac: always use autonegotiation to configure the SGMII link". -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: remove the cmd_type field from struct request
On 01/31/2017 07:57 AM, Christoph Hellwig wrote: > Hi Jens, > > this series gets rid of the cmd_type field in struct request and > instead folds it into the REQ_OP* space. This reduces the size of > struct request, and leads to a single op namespace that drivers > can easily switch on. Except for the legacy ide driver which has > a mess of different request types [1] this also nicely cleans up the > code. > > Note that the patches are on top of the > > "make SCSI passthrough support optional" > > series I sent out on Saturday. To make life easier I also have a git > tree available here: > > git://git.infradead.org/users/hch/block.git cmd_type > > > http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/cmd_type Looks good to me, applied for 4.11. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: remove the cmd_type field from struct request
On 01/31/2017 10:58 AM, Sergei Shtylyov wrote: > On 01/31/2017 09:51 PM, James Bottomley wrote: > [1] which were a pain in the ass to untangle and debug during development, it's really time for it to die.. >>> >>> Outside of the patch series in question, how to we expedite the >>> euthanasia of IDE? What explicit features/support are we missing >>> through libata that would need to be added, before we can git rm >>> drivers/ide/? >> >> I thought the primary objection was actually embedded in that libata > > Back at MontaVista, even embedded uses switched to libata years ago... Indeed, that would be my assumption as well, not too worried about that side. > DaveM thinks we still can't prove that libata works everywhere the IDE works. > Besided, there are still a number of non-x86 drivers not converted over to > libata (like DaVinci, etc.)... That argument is getting harder to buy. IDE has become a considerable maintenance burden - not for Dave, but for the rest of us that have to carry a subsystem forward. Personally I would _love_ to kill IDE at some point in the future, where that future hopefully isn't too far off. But if we have hardware that is being used and where IDE works and libata support does not exist, then weneed to fix that first. > Do you remember how many years ago a libata's own block driver was > promised to you? ;-) > I'd gladly wrote one, getting tired of my current OSS work... :-) Pretty sure I told Jeff originally that libata should only go into the kernel, if there was a plan to make it independent of SCSI. A promise was made that of course it would, but that promise was never held, unfortunately. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [dm-devel] split scsi passthrough fields out of struct request V2
On Mon, 2017-01-30 at 17:38 -0800, Jens Axboe wrote: > That's a known bug in mainline. Pull it into 4.10-rc6, > or use my for-next where everything is already merged. Hello Jens, With your for-next branch (commit c2e60b3a2602) I haven't hit any block layer crashes so far. The only issue I encountered that is new is a memory leak triggered by the SG-IO code. These memory leak reports started to appear after I started testing the mq-deadline scheduler. kmemleak reported the following call stack multiple times after my tests had finished: unreferenced object 0x88041119e528 (size 192): comm "multipathd", pid 2353, jiffies 4295128020 (age 1332.440s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 12 01 00 00 00 00 00 00 backtrace: [] kmemleak_alloc+0x45/0xa0 [] __kmalloc+0x15d/0x2f0 [] bio_alloc_bioset+0x185/0x1f0 [] bio_map_user_iov+0x124/0x400 [] blk_rq_map_user_iov+0x11a/0x210 [] blk_rq_map_user+0x4d/0x60 [] sg_io+0x3d4/0x410 [] scsi_cmd_ioctl+0x300/0x490 [] scsi_cmd_blk_ioctl+0x3d/0x50 [] sd_ioctl+0x80/0x100 [] blkdev_ioctl+0x51e/0x9f0 [] block_ioctl+0x38/0x40 [] do_vfs_ioctl+0x8f/0x700 [] SyS_ioctl+0x3c/0x70 [] entry_SYSCALL_64_fastpath+0x18/0xad Bart.
Re: [dm-devel] split scsi passthrough fields out of struct request V2
On Tue, 2017-01-31 at 13:34 -0800, Bart Van Assche wrote: > On Mon, 2017-01-30 at 17:38 -0800, Jens Axboe wrote: > > That's a known bug in mainline. Pull it into 4.10-rc6, > > or use my for-next where everything is already merged. > > Hello Jens, > > With your for-next branch (commit c2e60b3a2602) I haven't hit any block > layer crashes so far. The only issue I encountered that is new is a > memory leak triggered by the SG-IO code. These memory leak reports > started to appear after I started testing the mq-deadline scheduler. > kmemleak reported the following call stack multiple times after my tests > had finished: > > unreferenced object 0x88041119e528 (size 192): > comm "multipathd", pid 2353, jiffies 4295128020 (age 1332.440s) > hex dump (first 32 bytes): > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 00 00 00 00 00 00 00 00 12 01 00 00 00 00 00 00 > backtrace: > [] kmemleak_alloc+0x45/0xa0 > [] __kmalloc+0x15d/0x2f0 > [] bio_alloc_bioset+0x185/0x1f0 > [] bio_map_user_iov+0x124/0x400 > [] blk_rq_map_user_iov+0x11a/0x210 > [] blk_rq_map_user+0x4d/0x60 > [] sg_io+0x3d4/0x410 > [] scsi_cmd_ioctl+0x300/0x490 > [] scsi_cmd_blk_ioctl+0x3d/0x50 > [] sd_ioctl+0x80/0x100 > [] blkdev_ioctl+0x51e/0x9f0 > [] block_ioctl+0x38/0x40 > [] do_vfs_ioctl+0x8f/0x700 > [] SyS_ioctl+0x3c/0x70 > [] entry_SYSCALL_64_fastpath+0x18/0xad After I repeated my test the above findings were confirmed: no memory leaks were reported by kmemleak after a test with I/O scheduler "none" and the above call stack was reported 44 times by kmemleak after a test with I/O scheduler "mq-deadline". Bart.
Re: [dm-devel] split scsi passthrough fields out of struct request V2
On 01/31/2017 01:55 PM, Bart Van Assche wrote: > On Tue, 2017-01-31 at 13:34 -0800, Bart Van Assche wrote: >> On Mon, 2017-01-30 at 17:38 -0800, Jens Axboe wrote: >>> That's a known bug in mainline. Pull it into 4.10-rc6, >>> or use my for-next where everything is already merged. >> >> Hello Jens, >> >> With your for-next branch (commit c2e60b3a2602) I haven't hit any block >> layer crashes so far. The only issue I encountered that is new is a >> memory leak triggered by the SG-IO code. These memory leak reports >> started to appear after I started testing the mq-deadline scheduler. >> kmemleak reported the following call stack multiple times after my tests >> had finished: >> >> unreferenced object 0x88041119e528 (size 192): >> comm "multipathd", pid 2353, jiffies 4295128020 (age 1332.440s) >> hex dump (first 32 bytes): >> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> 00 00 00 00 00 00 00 00 12 01 00 00 00 00 00 00 >> backtrace: >> [] kmemleak_alloc+0x45/0xa0 >> [] __kmalloc+0x15d/0x2f0 >> [] bio_alloc_bioset+0x185/0x1f0 >> [] bio_map_user_iov+0x124/0x400 >> [] blk_rq_map_user_iov+0x11a/0x210 >> [] blk_rq_map_user+0x4d/0x60 >> [] sg_io+0x3d4/0x410 >> [] scsi_cmd_ioctl+0x300/0x490 >> [] scsi_cmd_blk_ioctl+0x3d/0x50 >> [] sd_ioctl+0x80/0x100 >> [] blkdev_ioctl+0x51e/0x9f0 >> [] block_ioctl+0x38/0x40 >> [] do_vfs_ioctl+0x8f/0x700 >> [] SyS_ioctl+0x3c/0x70 >> [] entry_SYSCALL_64_fastpath+0x18/0xad > > After I repeated my test the above findings were confirmed: no memory leaks > were reported by kmemleak after a test with I/O scheduler "none" and the > above call stack was reported 44 times by kmemleak after a test with I/O > scheduler "mq-deadline". Interesting, I'll check this. Doesn't make any sense why the scheduler would be implicated in that, given how we run completions now. But if it complains, then something must be up. -- Jens Axboe
[Bug 193041] iSCSI initiator: mkfs.ext4 + mount triggers kernel oops
https://bugzilla.kernel.org/show_bug.cgi?id=193041 Bart Van Assche changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |CODE_FIX --- Comment #1 from Bart Van Assche --- Fixed in v4.10-rc6 by commit 08965c2eba135bdfb6e86cf25308e01421c7e0ce (Revert "sd: remove __data_len hack for WRITE SAME"). -- You are receiving this mail because: You are the assignee for the bug.
Helpdesk
Dear E-mail User Please follow the link to update your webmail account http://ulvnw9j29.ukit.me Thank You
Re: [PATCH 0/6] ncr5380: Miscellaneous minor patches
> "Finn" == Finn Thain writes: Finn> This series removes some unused code and related comments, Finn> addresses the warnings generated by 'make W=1' and 'make C=1' and Finn> fixes a theoretical bug in the bus reset method in atari_scsi. Finn> There's also a patch to add a missing error check during target Finn> selection. The only target I tested was a QUANTUM DAYTONA514S disk Finn> as that's all I have access to right now. Some testing with other Finn> targets would be prudent. Applied to 4.11/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH 4.10-rc3 00/13] net: dsa: remove unnecessary phy.h include
On 01/31/2017 11:17 AM, Russell King - ARM Linux wrote: > Including phy.h and phy_fixed.h into net/dsa.h causes phy*.h to be an > unnecessary dependency for quite a large amount of the kernel. There's > very little which actually requires definitions from phy.h in net/dsa.h > - the include itself only wants the declaration of a couple of > structures and IFNAMSIZ. > > Add linux/if.h for IFNAMSIZ, declarations for the structures, phy.h to > mv88e6xxx.h as it needs it for phy_interface_t, and remove both phy.h > and phy_fixed.h from net/dsa.h. > > This patch reduces from around 800 files rebuilt to around 40 - even > with ccache, the time difference is noticable. > > In order to make this change, several drivers need to be updated to > include necessary headers that they were picking up through this > include. This has resulted in a much larger patch series. > > I'm assuming the 0-day builder has had 24 hours with this series, and > hasn't reported any further issues with it - the last issue was two > weeks ago (before I became ill) which I fixed over the last weekend. > > I'm hoping this doesn't conflict with what's already in net-next... For the entire series: Acked-by: Florian Fainelli Thanks a lot for doing that. > > arch/mips/cavium-octeon/octeon-platform.c | 4 > drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 1 + > drivers/net/ethernet/broadcom/bgmac.c | 2 ++ > drivers/net/ethernet/cadence/macb.h | 2 ++ > drivers/net/ethernet/cavium/liquidio/lio_main.c | 1 + > drivers/net/ethernet/cavium/liquidio/lio_vf_main.c| 1 + > drivers/net/ethernet/cavium/liquidio/octeon_console.c | 1 + > drivers/net/ethernet/freescale/fman/fman_memac.c | 1 + > drivers/net/ethernet/marvell/mvneta.c | 1 + > drivers/net/ethernet/qualcomm/emac/emac-sgmii.c | 1 + > drivers/net/usb/lan78xx.c | 1 + > drivers/net/wireless/ath/ath5k/ahb.c | 2 +- > drivers/target/iscsi/iscsi_target_login.c | 1 + > include/net/dsa.h | 6 -- > net/core/netprio_cgroup.c | 1 + > net/sunrpc/xprtrdma/svc_rdma_backchannel.c| 1 + > 16 files changed, 20 insertions(+), 7 deletions(-) > -- Florian
Re: [PATCH] storvsc: remove bogus code to transfer struct scatterlist
> "Christoph" == Christoph Hellwig writes: Christoph> Remove a piece of code in storvsc_queuecommand that tries to Christoph> pass the physical address of the kernel struct scatterlist Christoph> pointer to the host. Christoph> Fortunately the code can't ever be reached anyway. Applied to 4.11/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH V2] hpsa: remove coalescing settings for ioaccel2
> "Don" == Don Brace writes: Don> - Setting coalescing has a significant negative impact on low Don> queue-depth performance. - Does not help high queue-depth Don> performance. Applied to 4.11/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering
Re: remove the cmd_type field from struct request
> "Christoph" == Christoph Hellwig writes: Christoph> this series gets rid of the cmd_type field in struct Christoph> request and instead folds it into the REQ_OP* space. This Christoph> reduces the size of struct request, and leads to a single op Christoph> namespace that drivers can easily switch on. Very nice cleanup! Reviewed-by: Martin K. Petersen -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v3 0/4] mpt3sas driver Enhancements and defect fixes:
> "Chaitra" == Chaitra P B writes: Chaitra> Here is the change list: Posting 4 patches for mpt3sas driver Chaitra> enhancement and defect fixes. * Handle cable event for Chaitra> notifying degraded speed. * Performance improvement for Chaitra> Crusader. * Fix Firmware fault state 0x2100 during heavy 4K RR Chaitra> FIO stress test. * Updated driver version to 15.100.00.00 Applied to 4.11/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] scsi_debug: Add OPTIMAL TRANSFER LENGTH GRANULARITY option.
> "Lukas" == Lukas Herbolt writes: Applied to 4.11/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v3] mpt3sas: Force request partial completion alignment
> "Guilherme" == Guilherme G Piccoli writes: Guilherme> The firmware or device, possibly under a heavy I/O load, can Guilherme> return on a partial unaligned boundary. Scsi-ml expects these Guilherme> requests to be completed on an alignment boundary. Scsi-ml Guilherme> blindly requeues the I/O without checking the alignment Guilherme> boundary of the I/O request for the remaining bytes. This Guilherme> leads to errors, since devices cannot perform non-aligned Guilherme> read/write operations. Guilherme> This patch fixes the issue in the driver. It aligns unaligned Guilherme> completions of FS requests, by truncating them to the nearest Guilherme> alignment boundary. Applied to 4.10/scsi-fixes. -- Martin K. Petersen Oracle Linux Engineering
pri hiadani partnera!
Ahoj, Som zastupca investicneho zaujmu o zaujem zahranicnych investícii zahrnajuce velky objem financnych prostriedkov, u ktorych sa snazime svoju ucast ako v zamori zastupcu. Moj klient, ktory je rodak z Thajska, ma nejake peniaze zo svojich obchodnych uspor chce investovať na základe kvalifikovanych zahranicnych partnerstva legitimne transakcie. uvedte, o rychlu odpoved, ak mate zaujem, takze ja vam moze poskytnut dalsie podrobnosti spoluprace. S pozdravom, E-mail: xiasu...@gmail.com Barr. Su
Re: [PATCH 4.10-rc3 12/13] net: ath5k: fix build errors when linux/phy*.h is removed from net/dsa.h
Russell King writes: > Fix these errors reported by the 0-day builder by replacing the > linux/export.h include with linux/module.h. > > In file included from include/linux/platform_device.h:14:0, > from drivers/net/wireless/ath/ath5k/ahb.c:20: > include/linux/device.h:1463:1: warning: data definition has no type or > storage class > module_init(__driver##_init); \ > ^ > include/linux/platform_device.h:228:2: note: in expansion of macro > 'module_driver' > module_driver(__platform_driver, platform_driver_register, \ > ^ > drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro > 'module_platform_driver' > module_platform_driver(ath_ahb_driver); > ^~ > include/linux/device.h:1463:1: error: type defaults to 'int' in declaration > of 'module_init' [-Werror=implicit-int] > module_init(__driver##_init); \ > ^ > include/linux/platform_device.h:228:2: note: in expansion of macro > 'module_driver' > module_driver(__platform_driver, platform_driver_register, \ > ^ > drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro > 'module_platform_driver' > module_platform_driver(ath_ahb_driver); > ^~ > drivers/net/wireless/ath/ath5k/ahb.c:233:1: warning: parameter names (without > types) in function declaration > In file included from include/linux/platform_device.h:14:0, > from drivers/net/wireless/ath/ath5k/ahb.c:20: > include/linux/device.h:1468:1: warning: data definition has no type or > storage class > module_exit(__driver##_exit); > ^ > include/linux/platform_device.h:228:2: note: in expansion of macro > 'module_driver' > module_driver(__platform_driver, platform_driver_register, \ > ^ > drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro > 'module_platform_driver' > module_platform_driver(ath_ahb_driver); > ^~ > include/linux/device.h:1468:1: error: type defaults to 'int' in declaration > of 'module_exit' [-Werror=implicit-int] > module_exit(__driver##_exit); > ^ > include/linux/platform_device.h:228:2: note: in expansion of macro > 'module_driver' > module_driver(__platform_driver, platform_driver_register, \ > ^ > drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro > 'module_platform_driver' > module_platform_driver(ath_ahb_driver); > ^~ > drivers/net/wireless/ath/ath5k/ahb.c:233:1: warning: parameter names (without > types) in function declaration > In file included from include/linux/platform_device.h:14:0, > from drivers/net/wireless/ath/ath5k/ahb.c:20: > drivers/net/wireless/ath/ath5k/ahb.c:233:24: warning: 'ath_ahb_driver_exit' > defined but not used [-Wunused-function] > module_platform_driver(ath_ahb_driver); > ^ > include/linux/device.h:1464:20: note: in definition of macro 'module_driver' > static void __exit __driver##_exit(void) \ > ^~~~ > drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro > 'module_platform_driver' > module_platform_driver(ath_ahb_driver); > ^~ > drivers/net/wireless/ath/ath5k/ahb.c:233:24: warning: 'ath_ahb_driver_init' > defined but not used [-Wunused-function] > module_platform_driver(ath_ahb_driver); > ^ > include/linux/device.h:1459:19: note: in definition of macro 'module_driver' > static int __init __driver##_init(void) \ >^~~~ > drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro > 'module_platform_driver' > module_platform_driver(ath_ahb_driver); > ^~ > > Signed-off-by: Russell King Looks good to me: Acked-by: Kalle Valo I assume Dave will take this so I'll drop the patch from my queue. -- Kalle Valo
Re: [PATCH 00/10] mpt3sas: full mq support
On 01/31/2017 06:54 PM, Kashyap Desai wrote: -Original Message- From: Hannes Reinecke [mailto:h...@suse.de] Sent: Tuesday, January 31, 2017 4:47 PM To: Christoph Hellwig Cc: Martin K. Petersen; James Bottomley; linux-scsi@vger.kernel.org; Sathya Prakash; Kashyap Desai; mpt-fusionlinux@broadcom.com Subject: Re: [PATCH 00/10] mpt3sas: full mq support On 01/31/2017 11:02 AM, Christoph Hellwig wrote: On Tue, Jan 31, 2017 at 10:25:50AM +0100, Hannes Reinecke wrote: Hi all, this is a patchset to enable full multiqueue support for the mpt3sas driver. While the HBA only has a single mailbox register for submitting commands, it does have individual receive queues per MSI-X interrupt and as such does benefit from converting it to full multiqueue support. Explanation and numbers on why this would be beneficial, please. We should not need multiple submissions queues for a single register to benefit from multiple completion queues. Well, the actual throughput very strongly depends on the blk-mq-sched patches from Jens. As this is barely finished I didn't post any numbers yet. However: With multiqueue support: 4k seq read : io=60573MB, bw=1009.2MB/s, iops=258353, runt= 60021msec With scsi-mq on 1 queue: 4k seq read : io=17369MB, bw=296291KB/s, iops=74072, runt= 60028msec So yes, there _is_ a benefit. (Which is actually quite cool, as these tests were done on a SAS3 HBA, so we're getting close to the theoretical maximum of 1.2GB/s). (Unlike the single-queue case :-) Hannes - Can you share detail about setup ? How many drives do you have and how is connection (enclosure -> drives. ??) ? To me it looks like current mpt3sas driver might be taking more hit in spinlock operation (penalty on NUMA arch is more compare to single core server) unlike we have in megaraid_sas driver use of shared blk tag. The tests were done with a single LSI SAS3008 connected to a NetApp E-series (2660), using 4 LUNs under MD-RAID0. Megaraid_sas is even worse here; due to the odd nature of the 'fusion' implementation we're ending up having _two_ sets of tags, making it really hard to use scsi-mq here. (Not that I didn't try; but lacking a proper backend it's really hard to evaluate the benefit of those ... spinning HDDs simply don't cut it here) I mean " [PATCH 08/10] mpt3sas: lockless command submission for scsi-mq" patch is improving performance removing spinlock overhead and attempting to get request using blk_tags. Are you seeing performance improvement if you hard code nr_hw_queues = 1 in below code changes part of "[PATCH 10/10] mpt3sas: scsi-mq interrupt steering" No. The numbers posted above are generated with exactly that patch; the first line is running with nr_hw_queues=32 and the second line with nr_hw_queues=1. Curiously, though, patch 8/10 also reduces the 'can_queue' value by dividing it by the number of CPUs (required for blk tag space scaling). If I _increase_ can_queue after setting up the tagspace to the original value performance _drops_ again. Most unexpected; I'll be doing more experimenting there. Full results will be presented at VAULT, btw :-) Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
Re: remove the cmd_type field from struct request
On 01/31/2017 07:02 PM, Jens Axboe wrote: On 01/31/2017 07:57 AM, Christoph Hellwig wrote: [1] which were a pain in the ass to untangle and debug during development, it's really time for it to die.. Outside of the patch series in question, how to we expedite the euthanasia of IDE? What explicit features/support are we missing through libata that would need to be added, before we can git rm drivers/ide/ ? There is only a single driver (sgi_ide) which hasn't been moved over to libata. But this is for an old Itanium-based server only, and even SGI didn't press us to have this ported. (And we have disabled the IDE drivers since SLES11, where we still support Itanium.) So they can be safely assumed defunct at this time. I'm all for removing them. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
RE: [PATCH 00/10] mpt3sas: full mq support
> -Original Message- > From: Hannes Reinecke [mailto:h...@suse.de] > Sent: Wednesday, February 01, 2017 12:21 PM > To: Kashyap Desai; Christoph Hellwig > Cc: Martin K. Petersen; James Bottomley; linux-scsi@vger.kernel.org; > Sathya > Prakash Veerichetty; PDL-MPT-FUSIONLINUX; Sreekanth Reddy > Subject: Re: [PATCH 00/10] mpt3sas: full mq support > > On 01/31/2017 06:54 PM, Kashyap Desai wrote: > >> -Original Message- > >> From: Hannes Reinecke [mailto:h...@suse.de] > >> Sent: Tuesday, January 31, 2017 4:47 PM > >> To: Christoph Hellwig > >> Cc: Martin K. Petersen; James Bottomley; linux-scsi@vger.kernel.org; > > Sathya > >> Prakash; Kashyap Desai; mpt-fusionlinux@broadcom.com > >> Subject: Re: [PATCH 00/10] mpt3sas: full mq support > >> > >> On 01/31/2017 11:02 AM, Christoph Hellwig wrote: > >>> On Tue, Jan 31, 2017 at 10:25:50AM +0100, Hannes Reinecke wrote: > Hi all, > > this is a patchset to enable full multiqueue support for the > mpt3sas > >> driver. > While the HBA only has a single mailbox register for submitting > commands, it does have individual receive queues per MSI-X > interrupt and as such does benefit from converting it to full > multiqueue > > support. > >>> > >>> Explanation and numbers on why this would be beneficial, please. > >>> We should not need multiple submissions queues for a single register > >>> to benefit from multiple completion queues. > >>> > >> Well, the actual throughput very strongly depends on the blk-mq-sched > >> patches from Jens. > >> As this is barely finished I didn't post any numbers yet. > >> > >> However: > >> With multiqueue support: > >> 4k seq read : io=60573MB, bw=1009.2MB/s, iops=258353, runt= > 60021msec > >> With scsi-mq on 1 queue: > >> 4k seq read : io=17369MB, bw=296291KB/s, iops=74072, runt= 60028msec > >> So yes, there _is_ a benefit. > >> > >> (Which is actually quite cool, as these tests were done on a SAS3 > >> HBA, > > so > >> we're getting close to the theoretical maximum of 1.2GB/s). > >> (Unlike the single-queue case :-) > > > > Hannes - > > > > Can you share detail about setup ? How many drives do you have and how > > is connection (enclosure -> drives. ??) ? > > To me it looks like current mpt3sas driver might be taking more hit in > > spinlock operation (penalty on NUMA arch is more compare to single > > core > > server) unlike we have in megaraid_sas driver use of shared blk tag. > > > The tests were done with a single LSI SAS3008 connected to a NetApp E- > series (2660), using 4 LUNs under MD-RAID0. > > Megaraid_sas is even worse here; due to the odd nature of the 'fusion' > implementation we're ending up having _two_ sets of tags, making it really > hard to use scsi-mq here. Current megaraid_sas as single submission queue exposed to the blk-mq will not encounter similar performance issue. We may not see significant improvement of performance if we attempt the same for megaraid_sas driver. We had similar discussion for megaraid_sas and hpsa. http://www.spinics.net/lists/linux-scsi/msg101838.html I am seeing this patch series is similar attempt for mpt3sas..Am I missing anything ? Megaraid_sas driver just do indexing from blk_tag and fire IO quick enough unlike mpt3sas where we have lock contention @driver level as bottleneck. > (Not that I didn't try; but lacking a proper backend it's really hard to > evaluate > the benefit of those ... spinning HDDs simply don't cut it here) > > > I mean " [PATCH 08/10] mpt3sas: lockless command submission for scsi- > mq" > > patch is improving performance removing spinlock overhead and > > attempting to get request using blk_tags. > > Are you seeing performance improvement if you hard code nr_hw_queues > > = 1 in below code changes part of "[PATCH 10/10] mpt3sas: scsi-mq > > interrupt steering" > > > No. The numbers posted above are generated with exactly that patch; the > first line is running with nr_hw_queues=32 and the second line with > nr_hw_queues=1. Thanks Hannes. That clarifies. Can you share script you have used ? If my understanding correct, you will see theoretical maximum of 1.2GBp/s if you restrict your work load to single numa node. This is just for understanding if driver spinlocks are adding overhead. We have seen such overhead on multi-socket server and it is reasonable to reduce lock in mpt3sas driver, but only concern is exposing HBA for multiple submission queue to blk-mq is really not required and trying to figure out if we have any side effect of doing that. > > Curiously, though, patch 8/10 also reduces the 'can_queue' value by > dividing > it by the number of CPUs (required for blk tag space scaling). > If I _increase_ can_queue after setting up the tagspace to the original > value > performance _drops_ again. > Most unexpected; I'll be doing more experimenting there. > > Full results will be presented at VAULT, btw :-) > > Cheers, > > Hannes > -- > Dr. Hannes Reinecke zSeries & Storage > h...
Re: [PATCH 00/10] mpt3sas: full mq support
On 02/01/2017 08:07 AM, Kashyap Desai wrote: -Original Message- From: Hannes Reinecke [mailto:h...@suse.de] Sent: Wednesday, February 01, 2017 12:21 PM To: Kashyap Desai; Christoph Hellwig Cc: Martin K. Petersen; James Bottomley; linux-scsi@vger.kernel.org; Sathya Prakash Veerichetty; PDL-MPT-FUSIONLINUX; Sreekanth Reddy Subject: Re: [PATCH 00/10] mpt3sas: full mq support On 01/31/2017 06:54 PM, Kashyap Desai wrote: -Original Message- From: Hannes Reinecke [mailto:h...@suse.de] Sent: Tuesday, January 31, 2017 4:47 PM To: Christoph Hellwig Cc: Martin K. Petersen; James Bottomley; linux-scsi@vger.kernel.org; Sathya Prakash; Kashyap Desai; mpt-fusionlinux@broadcom.com Subject: Re: [PATCH 00/10] mpt3sas: full mq support On 01/31/2017 11:02 AM, Christoph Hellwig wrote: On Tue, Jan 31, 2017 at 10:25:50AM +0100, Hannes Reinecke wrote: Hi all, this is a patchset to enable full multiqueue support for the mpt3sas driver. While the HBA only has a single mailbox register for submitting commands, it does have individual receive queues per MSI-X interrupt and as such does benefit from converting it to full multiqueue support. Explanation and numbers on why this would be beneficial, please. We should not need multiple submissions queues for a single register to benefit from multiple completion queues. Well, the actual throughput very strongly depends on the blk-mq-sched patches from Jens. As this is barely finished I didn't post any numbers yet. However: With multiqueue support: 4k seq read : io=60573MB, bw=1009.2MB/s, iops=258353, runt= 60021msec With scsi-mq on 1 queue: 4k seq read : io=17369MB, bw=296291KB/s, iops=74072, runt= 60028msec So yes, there _is_ a benefit. (Which is actually quite cool, as these tests were done on a SAS3 HBA, so we're getting close to the theoretical maximum of 1.2GB/s). (Unlike the single-queue case :-) Hannes - Can you share detail about setup ? How many drives do you have and how is connection (enclosure -> drives. ??) ? To me it looks like current mpt3sas driver might be taking more hit in spinlock operation (penalty on NUMA arch is more compare to single core server) unlike we have in megaraid_sas driver use of shared blk tag. The tests were done with a single LSI SAS3008 connected to a NetApp E- series (2660), using 4 LUNs under MD-RAID0. Megaraid_sas is even worse here; due to the odd nature of the 'fusion' implementation we're ending up having _two_ sets of tags, making it really hard to use scsi-mq here. Current megaraid_sas as single submission queue exposed to the blk-mq will not encounter similar performance issue. We may not see significant improvement of performance if we attempt the same for megaraid_sas driver. We had similar discussion for megaraid_sas and hpsa. http://www.spinics.net/lists/linux-scsi/msg101838.html I am seeing this patch series is similar attempt for mpt3sas..Am I missing anything ? No, you don't. That is precisely the case. The different here is that mpt3sas is actually exposing hardware capabilities, whereas with megaraid_sas (and hpsa) we're limited by the hardware implementation to a single completion queue shared between HBA and OS. With mpt3sas we're having per-interrupt completion queues (well, for newer firmware :-) so we can take advantage of scsi-mq. (And if someone had done a _proper_ design of the megaraid_sas_fusion thing by exposing several submission and completion queues for megaraid_sas itself instead of bolting the existing megaraid_sas single queue approach ontop of the mpt3sas multiqueue design we could have done the same thing there ... sigh) Megaraid_sas driver just do indexing from blk_tag and fire IO quick enough unlike mpt3sas where we have lock contention @driver level as bottleneck. (Not that I didn't try; but lacking a proper backend it's really hard to evaluate the benefit of those ... spinning HDDs simply don't cut it here) I mean " [PATCH 08/10] mpt3sas: lockless command submission for scsi- mq" patch is improving performance removing spinlock overhead and attempting to get request using blk_tags. Are you seeing performance improvement if you hard code nr_hw_queues = 1 in below code changes part of "[PATCH 10/10] mpt3sas: scsi-mq interrupt steering" No. The numbers posted above are generated with exactly that patch; the first line is running with nr_hw_queues=32 and the second line with nr_hw_queues=1. Thanks Hannes. That clarifies. Can you share script you have used ? If my understanding correct, you will see theoretical maximum of 1.2GBp/s if you restrict your work load to single numa node. This is just for understanding if driver spinlocks are adding overhead. We have seen such overhead on multi-socket server and it is reasonable to reduce lock in mpt3sas driver, but only concern is exposing HBA for multiple submission queue to blk-mq is really not required and trying to figure out if we have any side effect of doing that. Well, the HBA ha
