Re: [PATCH v5] target: add emulate_pr backstore attr to toggle PR support
Looks good, Reviewed-by: Christoph Hellwig
[PATCH v2 0/9] qedi bug fixes
Martin, Please consider below patch set for next 'scsi-fixes' submission. Thanks, Nilesh Manish Rangankar (3): qedi: Check for session online before getting iSCSI TLV data. qedi: Add packet filter in light L2 Rx path. qedi: Move LL2 producer index processing in BH. Nilesh Javali (6): qedi: Cleanup redundant QEDI_PAGE_SIZE macro definition qedi: Fix spelling mistake "OUSTANDING" -> "OUTSTANDING" qedi: Replace PAGE_SIZE with QEDI_PAGE_SIZE qedi: Allocate IRQs based on msix_cnt qedi: add module param to set ping packet size qedi: Update driver version to 8.33.0.21 drivers/scsi/qedi/qedi.h | 7 +--- drivers/scsi/qedi/qedi_main.c| 87 +--- drivers/scsi/qedi/qedi_version.h | 4 +- 3 files changed, 68 insertions(+), 30 deletions(-) -- 1.8.3.1
[PATCH v2 1/9] qedi: Cleanup redundant QEDI_PAGE_SIZE macro definition
Remove redundant macro definition. Signed-off-by: Nilesh Javali --- drivers/scsi/qedi/qedi.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/scsi/qedi/qedi.h b/drivers/scsi/qedi/qedi.h index a6f96b3..e966855 100644 --- a/drivers/scsi/qedi/qedi.h +++ b/drivers/scsi/qedi/qedi.h @@ -64,11 +64,9 @@ #define TX_RX_RING 16 #define RX_RING(TX_RX_RING - 1) #define LL2_SINGLE_BUF_SIZE0x400 -#define QEDI_PAGE_SIZE 4096 #define QEDI_PAGE_ALIGN(addr) ALIGN(addr, QEDI_PAGE_SIZE) #define QEDI_PAGE_MASK (~((QEDI_PAGE_SIZE) - 1)) -#define QEDI_PAGE_SIZE 4096 #define QEDI_HW_DMA_BOUNDARY 0xfff #define QEDI_PATH_HANDLE 0xFE000UL -- 1.8.3.1
[PATCH v2 3/9] qedi: Replace PAGE_SIZE with QEDI_PAGE_SIZE
Use QEDI_PAGE_SIZE for enablement of module on systems with 64K page size.
Signed-off-by: Nilesh Javali
---
drivers/scsi/qedi/qedi_main.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
index 0f8eb5f..a1225ae 100644
--- a/drivers/scsi/qedi/qedi_main.c
+++ b/drivers/scsi/qedi/qedi_main.c
@@ -796,7 +796,7 @@ static int qedi_set_iscsi_pf_param(struct qedi_ctx *qedi)
int rval = 0;
- num_sq_pages = (MAX_OUTSTANDING_TASKS_PER_CON * 8) / PAGE_SIZE;
+ num_sq_pages = (MAX_OUTSTANDING_TASKS_PER_CON * 8) / QEDI_PAGE_SIZE;
qedi->num_queues = MIN_NUM_CPUS_MSIX(qedi);
@@ -834,7 +834,7 @@ static int qedi_set_iscsi_pf_param(struct qedi_ctx *qedi)
qedi->pf_params.iscsi_pf_params.max_fin_rt = 2;
for (log_page_size = 0 ; log_page_size < 32 ; log_page_size++) {
- if ((1 << log_page_size) == PAGE_SIZE)
+ if ((1 << log_page_size) == QEDI_PAGE_SIZE)
break;
}
qedi->pf_params.iscsi_pf_params.log_page_size = log_page_size;
@@ -1376,7 +1376,7 @@ static void qedi_free_bdq(struct qedi_ctx *qedi)
int i;
if (qedi->bdq_pbl_list)
- dma_free_coherent(&qedi->pdev->dev, PAGE_SIZE,
+ dma_free_coherent(&qedi->pdev->dev, QEDI_PAGE_SIZE,
qedi->bdq_pbl_list, qedi->bdq_pbl_list_dma);
if (qedi->bdq_pbl)
@@ -1437,7 +1437,7 @@ static int qedi_alloc_bdq(struct qedi_ctx *qedi)
/* Alloc dma memory for BDQ page buffer list */
qedi->bdq_pbl_mem_size = QEDI_BDQ_NUM * sizeof(struct scsi_bd);
- qedi->bdq_pbl_mem_size = ALIGN(qedi->bdq_pbl_mem_size, PAGE_SIZE);
+ qedi->bdq_pbl_mem_size = ALIGN(qedi->bdq_pbl_mem_size, QEDI_PAGE_SIZE);
qedi->rq_num_entries = qedi->bdq_pbl_mem_size / sizeof(struct scsi_bd);
QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_CONN, "rq_num_entries = %d.\n",
@@ -1472,7 +1472,8 @@ static int qedi_alloc_bdq(struct qedi_ctx *qedi)
}
/* Allocate list of PBL pages */
- qedi->bdq_pbl_list = dma_zalloc_coherent(&qedi->pdev->dev, PAGE_SIZE,
+ qedi->bdq_pbl_list = dma_zalloc_coherent(&qedi->pdev->dev,
+QEDI_PAGE_SIZE,
&qedi->bdq_pbl_list_dma,
GFP_KERNEL);
if (!qedi->bdq_pbl_list) {
@@ -1485,13 +1486,14 @@ static int qedi_alloc_bdq(struct qedi_ctx *qedi)
* Now populate PBL list with pages that contain pointers to the
* individual buffers.
*/
- qedi->bdq_pbl_list_num_entries = qedi->bdq_pbl_mem_size / PAGE_SIZE;
+ qedi->bdq_pbl_list_num_entries = qedi->bdq_pbl_mem_size /
+QEDI_PAGE_SIZE;
list = (u64 *)qedi->bdq_pbl_list;
page = qedi->bdq_pbl_list_dma;
for (i = 0; i < qedi->bdq_pbl_list_num_entries; i++) {
*list = qedi->bdq_pbl_dma;
list++;
- page += PAGE_SIZE;
+ page += QEDI_PAGE_SIZE;
}
return 0;
--
1.8.3.1
[PATCH v2 2/9] qedi: Fix spelling mistake "OUSTANDING" -> "OUTSTANDING"
Fix trivial spelling mistake within macro definition.
Signed-off-by: Nilesh Javali
---
drivers/scsi/qedi/qedi.h | 4 ++--
drivers/scsi/qedi/qedi_main.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/qedi/qedi.h b/drivers/scsi/qedi/qedi.h
index e966855..6fa02c5 100644
--- a/drivers/scsi/qedi/qedi.h
+++ b/drivers/scsi/qedi/qedi.h
@@ -45,7 +45,7 @@
#define QEDI_MAX_TASK_NUM 0x0FFF
#define QEDI_MAX_ISCSI_CONNS_PER_HBA 1024
#define QEDI_ISCSI_MAX_BDS_PER_CMD 255 /* Firmware max BDs is 255 */
-#define MAX_OUSTANDING_TASKS_PER_CON 1024
+#define MAX_OUTSTANDING_TASKS_PER_CON 1024
#define QEDI_MAX_BD_LEN0x
#define QEDI_BD_SPLIT_SZ 0x1000
@@ -144,7 +144,7 @@ struct skb_work_list {
};
/* Queue sizes in number of elements */
-#define QEDI_SQ_SIZE MAX_OUSTANDING_TASKS_PER_CON
+#define QEDI_SQ_SIZE MAX_OUTSTANDING_TASKS_PER_CON
#define QEDI_CQ_SIZE 2048
#define QEDI_CMDQ_SIZE QEDI_MAX_ISCSI_TASK
#define QEDI_PROTO_CQ_PROD_IDX 0
diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
index 105b0e4..0f8eb5f 100644
--- a/drivers/scsi/qedi/qedi_main.c
+++ b/drivers/scsi/qedi/qedi_main.c
@@ -796,7 +796,7 @@ static int qedi_set_iscsi_pf_param(struct qedi_ctx *qedi)
int rval = 0;
- num_sq_pages = (MAX_OUSTANDING_TASKS_PER_CON * 8) / PAGE_SIZE;
+ num_sq_pages = (MAX_OUTSTANDING_TASKS_PER_CON * 8) / PAGE_SIZE;
qedi->num_queues = MIN_NUM_CPUS_MSIX(qedi);
--
1.8.3.1
[PATCH v2 7/9] qedi: add module param to set ping packet size
Default packet size is 0x400. For jumbo packets set to 0x2400. Signed-off-by: Nilesh Javali --- drivers/scsi/qedi/qedi.h | 1 - drivers/scsi/qedi/qedi_main.c | 13 + 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/qedi/qedi.h b/drivers/scsi/qedi/qedi.h index 6fa02c5..a26bb506 100644 --- a/drivers/scsi/qedi/qedi.h +++ b/drivers/scsi/qedi/qedi.h @@ -63,7 +63,6 @@ #define QEDI_LOCAL_PORT_INVALID0x #define TX_RX_RING 16 #define RX_RING(TX_RX_RING - 1) -#define LL2_SINGLE_BUF_SIZE0x400 #define QEDI_PAGE_ALIGN(addr) ALIGN(addr, QEDI_PAGE_SIZE) #define QEDI_PAGE_MASK (~((QEDI_PAGE_SIZE) - 1)) diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c index 2621dee..8942f62 100644 --- a/drivers/scsi/qedi/qedi_main.c +++ b/drivers/scsi/qedi/qedi_main.c @@ -44,6 +44,11 @@ MODULE_PARM_DESC(qedi_io_tracing, " Enable logging of SCSI requests/completions into trace buffer. (default off)."); +uint qedi_ll2_buf_size = 0x400; +module_param(qedi_ll2_buf_size, uint, 0644); +MODULE_PARM_DESC(qedi_ll2_buf_size, +"parameter to set ping packet size, default - 0x400, Jumbo packets - 0x2400."); + const struct qed_iscsi_ops *qedi_ops; static struct scsi_transport_template *qedi_scsi_transport; static struct pci_driver qedi_pci_driver; @@ -228,7 +233,7 @@ static int __qedi_alloc_uio_rings(struct qedi_uio_dev *udev) } /* Allocating memory for Tx/Rx pkt buffer */ - udev->ll2_buf_size = TX_RX_RING * LL2_SINGLE_BUF_SIZE; + udev->ll2_buf_size = TX_RX_RING * qedi_ll2_buf_size; udev->ll2_buf_size = QEDI_PAGE_ALIGN(udev->ll2_buf_size); udev->ll2_buf = (void *)__get_free_pages(GFP_KERNEL | __GFP_COMP | __GFP_ZERO, 2); @@ -283,7 +288,7 @@ static int qedi_alloc_uio_rings(struct qedi_ctx *qedi) qedi->udev = udev; udev->tx_pkt = udev->ll2_buf; - udev->rx_pkt = udev->ll2_buf + LL2_SINGLE_BUF_SIZE; + udev->rx_pkt = udev->ll2_buf + qedi_ll2_buf_size; return 0; err_uctrl: @@ -752,8 +757,8 @@ static int qedi_ll2_process_skb(struct qedi_ctx *qedi, struct sk_buff *skb, udev = qedi->udev; uctrl = udev->uctrl; - pkt = udev->rx_pkt + (uctrl->hw_rx_prod * LL2_SINGLE_BUF_SIZE); - len = min_t(u32, skb->len, (u32)LL2_SINGLE_BUF_SIZE); + pkt = udev->rx_pkt + (uctrl->hw_rx_prod * qedi_ll2_buf_size); + len = min_t(u32, skb->len, (u32)qedi_ll2_buf_size); memcpy(pkt, skb->data, len); memset(&rxbd, 0, sizeof(rxbd)); -- 1.8.3.1
[PATCH v2 9/9] qedi: Update driver version to 8.33.0.21
Signed-off-by: Nilesh Javali --- drivers/scsi/qedi/qedi_version.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/qedi/qedi_version.h b/drivers/scsi/qedi/qedi_version.h index 8a0e523..41bcbba 100644 --- a/drivers/scsi/qedi/qedi_version.h +++ b/drivers/scsi/qedi/qedi_version.h @@ -7,8 +7,8 @@ * this source tree. */ -#define QEDI_MODULE_VERSION"8.33.0.20" +#define QEDI_MODULE_VERSION"8.33.0.21" #define QEDI_DRIVER_MAJOR_VER 8 #define QEDI_DRIVER_MINOR_VER 33 #define QEDI_DRIVER_REV_VER0 -#define QEDI_DRIVER_ENG_VER20 +#define QEDI_DRIVER_ENG_VER21 -- 1.8.3.1
[PATCH v2 8/9] qedi: Move LL2 producer index processing in BH.
From: Manish Rangankar
1. Removed logic to update HW producer index in interrupt context.
2. Update HW producer index after UIO ring and buffer gets initialized.
Signed-off-by: Manish Rangankar
---
drivers/scsi/qedi/qedi_main.c | 31 +++
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
index 8942f62..053a947 100644
--- a/drivers/scsi/qedi/qedi_main.c
+++ b/drivers/scsi/qedi/qedi_main.c
@@ -665,7 +665,6 @@ static int qedi_ll2_rx(void *cookie, struct sk_buff *skb,
u32 arg1, u32 arg2)
struct qedi_uio_ctrl *uctrl;
struct skb_work_list *work;
struct ethhdr *eh;
- u32 prod;
if (!qedi) {
QEDI_ERR(NULL, "qedi is NULL\n");
@@ -724,17 +723,10 @@ static int qedi_ll2_rx(void *cookie, struct sk_buff *skb,
u32 arg1, u32 arg2)
spin_lock_bh(&qedi->ll2_lock);
list_add_tail(&work->list, &qedi->ll2_skb_list);
+ spin_unlock_bh(&qedi->ll2_lock);
- ++uctrl->hw_rx_prod_cnt;
- prod = (uctrl->hw_rx_prod + 1) % RX_RING;
- if (prod != uctrl->host_rx_cons) {
- uctrl->hw_rx_prod = prod;
- spin_unlock_bh(&qedi->ll2_lock);
- wake_up_process(qedi->ll2_recv_thread);
- return 0;
- }
+ wake_up_process(qedi->ll2_recv_thread);
- spin_unlock_bh(&qedi->ll2_lock);
return 0;
}
@@ -749,6 +741,7 @@ static int qedi_ll2_process_skb(struct qedi_ctx *qedi,
struct sk_buff *skb,
u32 rx_bd_prod;
void *pkt;
int len = 0;
+ u32 prod;
if (!qedi) {
QEDI_ERR(NULL, "qedi is NULL\n");
@@ -757,12 +750,16 @@ static int qedi_ll2_process_skb(struct qedi_ctx *qedi,
struct sk_buff *skb,
udev = qedi->udev;
uctrl = udev->uctrl;
- pkt = udev->rx_pkt + (uctrl->hw_rx_prod * qedi_ll2_buf_size);
+
+ ++uctrl->hw_rx_prod_cnt;
+ prod = (uctrl->hw_rx_prod + 1) % RX_RING;
+
+ pkt = udev->rx_pkt + (prod * qedi_ll2_buf_size);
len = min_t(u32, skb->len, (u32)qedi_ll2_buf_size);
memcpy(pkt, skb->data, len);
memset(&rxbd, 0, sizeof(rxbd));
- rxbd.rx_pkt_index = uctrl->hw_rx_prod;
+ rxbd.rx_pkt_index = prod;
rxbd.rx_pkt_len = len;
rxbd.vlan_id = vlan_id;
@@ -773,6 +770,16 @@ static int qedi_ll2_process_skb(struct qedi_ctx *qedi,
struct sk_buff *skb,
memcpy(p_rxbd, &rxbd, sizeof(rxbd));
+ QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_LL2,
+ "hw_rx_prod [%d] prod [%d] hw_rx_bd_prod [%d] rx_pkt_idx [%d]
rx_len [%d].\n",
+ uctrl->hw_rx_prod, prod, uctrl->hw_rx_bd_prod,
+ rxbd.rx_pkt_index, rxbd.rx_pkt_len);
+ QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_LL2,
+ "host_rx_cons [%d] hw_rx_bd_cons [%d].\n",
+ uctrl->host_rx_cons, uctrl->host_rx_bd_cons);
+
+ uctrl->hw_rx_prod = prod;
+
/* notify the iscsiuio about new packet */
uio_event_notify(&udev->qedi_uinfo);
--
1.8.3.1
[PATCH v2 4/9] qedi: Allocate IRQs based on msix_cnt
The driver load on some systems failed with error,
[0004:01:00.5]:[qedi_request_msix_irq:2524]:8: request_irq failed.
Allocate the IRQs based on MSIX count obtained from qed module
instead of number of queues.
Signed-off-by: Nilesh Javali
---
drivers/scsi/qedi/qedi_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
index a1225ae..5308e6b 100644
--- a/drivers/scsi/qedi/qedi_main.c
+++ b/drivers/scsi/qedi/qedi_main.c
@@ -1298,7 +1298,7 @@ static int qedi_request_msix_irq(struct qedi_ctx *qedi)
int i, rc, cpu;
cpu = cpumask_first(cpu_online_mask);
- for (i = 0; i < MIN_NUM_CPUS_MSIX(qedi); i++) {
+ for (i = 0; i < qedi->int_info.msix_cnt; i++) {
rc = request_irq(qedi->int_info.msix[i].vector,
qedi_msix_handler, 0, "qedi",
&qedi->fp_array[i]);
--
1.8.3.1
[PATCH v2 6/9] qedi: Add packet filter in light L2 Rx path.
From: Manish Rangankar
Add packet filter to avoid unnecessary packet processing in iscsiuio.
Signed-off-by: Manish Rangankar
---
drivers/scsi/qedi/qedi_main.c | 24
1 file changed, 24 insertions(+)
diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
index 713db9c..2621dee 100644
--- a/drivers/scsi/qedi/qedi_main.c
+++ b/drivers/scsi/qedi/qedi_main.c
@@ -659,6 +659,7 @@ static int qedi_ll2_rx(void *cookie, struct sk_buff *skb,
u32 arg1, u32 arg2)
struct qedi_uio_dev *udev;
struct qedi_uio_ctrl *uctrl;
struct skb_work_list *work;
+ struct ethhdr *eh;
u32 prod;
if (!qedi) {
@@ -673,6 +674,29 @@ static int qedi_ll2_rx(void *cookie, struct sk_buff *skb,
u32 arg1, u32 arg2)
return 0;
}
+ eh = (struct ethhdr *)skb->data;
+ /* Undo VLAN encapsulation */
+ if (eh->h_proto == htons(ETH_P_8021Q)) {
+ memmove((u8 *)eh + VLAN_HLEN, eh, ETH_ALEN * 2);
+ eh = (struct ethhdr *)skb_pull(skb, VLAN_HLEN);
+ skb_reset_mac_header(skb);
+ }
+
+ /* Filter out non FIP/FCoE frames here to free them faster */
+ if (eh->h_proto != htons(ETH_P_ARP) &&
+ eh->h_proto != htons(ETH_P_IP) &&
+ eh->h_proto != htons(ETH_P_IPV6)) {
+ QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_LL2,
+ "Dropping frame ethertype [0x%x] len [0x%x].\n",
+ eh->h_proto, skb->len);
+ kfree_skb(skb);
+ return 0;
+ }
+
+ QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_LL2,
+ "Allowed frame ethertype [0x%x] len [0x%x].\n",
+ eh->h_proto, skb->len);
+
udev = qedi->udev;
uctrl = udev->uctrl;
--
1.8.3.1
[PATCH v2 5/9] qedi: Check for session online before getting iSCSI TLV data.
From: Manish Rangankar
The kernel panic was observed after switch side perturbation,
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [] strcmp+0x20/0x40
PGD 0 Oops: [#1] SMP
CPU: 8 PID: 647 Comm: kworker/8:1 Tainted: GW OE
3.10.0-693.el7.x86_64 #1
Hardware name: HPE ProLiant DL380 Gen10/ProLiant DL380 Gen10, BIOS U30
06/20/2018
Workqueue: slowpath-13:00. qed_slowpath_task [qed]
task: 880429eb8fd0 ti: 88042919 task.ti: 88042919
RIP: 0010:[] [] strcmp+0x20/0x40
RSP: 0018:880429193c68 EFLAGS: 00010202
RAX: 000a RBX: 0002 RCX:
RDX: 0001 RSI: 0001 RDI: 88042bda7a41
RBP: 880429193c68 R08: R09:
R10: 0007 R11: 88042b3af338 R12: 880420b007a0
R13: 88081aa56af8 R14: 0001 R15: 88081aa50410
FS: () GS:88042fe0() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: CR3: 019f2000 CR4: 003407e0
DR0: DR1: DR2:
DR3: DR6: fffe0ff0 DR7: 0400
Stack:
880429193d20 c02a0c90 c90004b32000 8803fd3ec600
88042bda7800 88042bda7a00 88042bda7840 88042bda7a40
000129193d10 2e3836312e323931 ff000a342e363232 c01ad99d
Call Trace:
[] qedi_get_protocol_tlv_data+0x270/0x470 [qedi]
[] ? qed_mfw_process_tlv_req+0x24d/0xbf0 [qed]
[] qed_mfw_fill_tlv_data+0x5e/0xd0 [qed]
[] qed_mfw_process_tlv_req+0x269/0xbf0 [qed]
Fix kernel NULL pointer deref by checking for session is online
before getting iSCSI TLV data.
Signed-off-by: Manish Rangankar
---
drivers/scsi/qedi/qedi_main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
index 5308e6b..713db9c 100644
--- a/drivers/scsi/qedi/qedi_main.c
+++ b/drivers/scsi/qedi/qedi_main.c
@@ -952,6 +952,9 @@ static int qedi_find_boot_info(struct qedi_ctx *qedi,
cls_sess = iscsi_conn_to_session(cls_conn);
sess = cls_sess->dd_data;
+ if (!iscsi_is_session_online(cls_sess))
+ continue;
+
if (pri_ctrl_flags) {
if (!strcmp(pri_tgt->iscsi_name, sess->targetname) &&
!strcmp(pri_tgt->ip_addr, ep_ip_addr)) {
--
1.8.3.1
DISABLE_CLUSTERING in scsi drivers
Hi all, you in the To list maintain or wrote SCSI drivers that set the DISABLE_CLUSTERING flag, which basically disable merges of any bio segments. We already have the actual max_segment size limit to say which length a segment should have, independent of merged or originally created, so this limit generally should rarely if ever be required, and mostly is an old cut an paste error. Can you go over your drivers and check if it could be removed?
Re: [PATCH v3] scsi: tcmu: avoid cmd/qfull timers updated whenever a new cmd comes
On 11/20/2018 11:37 PM, Xiubo Li wrote:
> [...]
>>> -is_running = list_empty(&cmd->cmdr_queue_entry);
>>> +is_running = test_bit(TCMU_CMD_BIT_INFLIGHT, &cmd->flags);
>>> se_cmd = cmd->se_cmd;
>>> if (is_running) {
>>> @@ -1289,7 +1319,6 @@ static int tcmu_check_expired_cmd(int id, void
>>> *p, void *data)
>>> scsi_status = SAM_STAT_CHECK_CONDITION;
>>> } else {
>>> list_del_init(&cmd->cmdr_queue_entry);
>> Move this list_del_init call to outside the if/else.
>>
>> You need do delete it from the cmdr_inflight_queue if that is how it
>> timed out, or if you later call tcmu_get_next_deadline it will still
>> show up and possibly be used to set the next time out which already
>> happened.
>
> Firstly, this is in the timeout routine, if this cmd was already timed
> out and it must be time_after(jiffies, cmd->deadline), so it won't be
> used again.
It could be stuck a long time. What about jiffies rollover?
Re: [PATCH 01/23] zfcp: make DIX experimental, disabled, and independent of DIF
Hi Steffen, Sorry about the delay. Travel got in the way. >> BDI_CAP_STABLE_WRITES should take care of this. What's the configuration >> that fails? > > Apologies, if the commit description sounds unfair. I did not mean to > blame anyone. It's just the collection of issues we saw in distros over > the years. Some of the old issues might be fixed with above zfcp patch > or common code changes. Unfortunately, I could not handle the DIX things > we saw. I think, DIF by itself provides a lot of the protection benefit > and was not affected by the encountered issues. We would like to give > users an easy way to operate in such setup. I don't have a problem with zfcp having a parameter that affects the host protection mask, the other drivers do that too. However, these knobs exist exclusively for debugging and testing purposes. They are not something regular users should twiddle to switch features on or off. So DIF and DIX should always be enabled in the driver. And there is no point in ever operating without DIF enabled if the hardware is capable. If there is a desire to disable DIX protection for whatever reason (legacy code doing bad things), do so using the block layer sysfs knobs. That's where the policy of whether to generate and verify protection information resides, not in the HBA driver. And if there are unaddressed issues in the I/O stack that prevents you from having integrity enabled, I'd prefer to know about them so they can be fixed rather than circumventing them through driver module parameter. Hope that makes sense. -- Martin K. Petersen Oracle Linux Engineering
Re: DISABLE_CLUSTERING in scsi drivers
Christoph, for Atari SCSI, commands can only be merged if the physical addresses of all buffers are contiguous (limitation of the Falcon DMA engine). Documentation/scsi/scsi_mid_low_api.tx does not spell out whether that is the case. Atari SCSI disables scatter/gather, so if that's sufficient to cue midlevel or bio to not undertake any merging, the flag is no longer needed. Cheers, Michael On Wed, Nov 21, 2018 at 10:41 PM Christoph Hellwig wrote: > > Hi all, > > you in the To list maintain or wrote SCSI drivers that set the > DISABLE_CLUSTERING flag, which basically disable merges of any > bio segments. We already have the actual max_segment size limit > to say which length a segment should have, independent of merged > or originally created, so this limit generally should rarely if > ever be required, and mostly is an old cut an paste error. > > Can you go over your drivers and check if it could be removed?
Re: DISABLE_CLUSTERING in scsi drivers
On Wed, 21 Nov 2018, Christoph Hellwig wrote: > Hi all, > > you in the To list maintain or wrote SCSI drivers that set the > DISABLE_CLUSTERING flag, which basically disable merges of any > bio segments. We already have the actual max_segment size limit > to say which length a segment should have, independent of merged > or originally created, so this limit generally should rarely if > ever be required, and mostly is an old cut an paste error. > Are you referring to blk_queue_max_segment_size(q, dma_get_max_seg_size(dev)); in drivers/scsi/scsi_lib.c? Is the segment size limitation of the DMA controller the only reason to want DISABLE_CLUSTERING? > Can you go over your drivers and check if it could be removed? > Adding Sun 3 maintainer to Cc. Besides sun3_scsi.c and atari_scsi.c, the SCSI drivers I take an interest in don't actually use a DMA controller. If my question about DMA controllers gets a "yes" then I guess I can send a patch for those drivers. --
Re: [PATCH] iser: set sector for ambiguous mr status errors
On Wed, Nov 14, 2018 at 10:17:01AM -0800, Sagi Grimberg wrote:
> If for some reason we failed to query the mr status, we need to make sure
> to provide sufficient information for an ambiguous error (guard error on
> sector 0).
>
> Fixes: 0a7a08ad6f5f ("IB/iser: Implement check_protection")
> Cc:
> Reported-by: Dan Carpenter
> Signed-off-by: Sagi Grimberg
> ---
> drivers/infiniband/ulp/iser/iser_verbs.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
Applied to for-rc
Thanks,
Jason
Re: [PATCH v3] scsi: tcmu: avoid cmd/qfull timers updated whenever a new cmd comes
On 2018/11/22 0:37, Mike Christie wrote:
On 11/20/2018 11:37 PM, Xiubo Li wrote:
[...]
-is_running = list_empty(&cmd->cmdr_queue_entry);
+is_running = test_bit(TCMU_CMD_BIT_INFLIGHT, &cmd->flags);
se_cmd = cmd->se_cmd;
if (is_running) {
@@ -1289,7 +1319,6 @@ static int tcmu_check_expired_cmd(int id, void
*p, void *data)
scsi_status = SAM_STAT_CHECK_CONDITION;
} else {
list_del_init(&cmd->cmdr_queue_entry);
Move this list_del_init call to outside the if/else.
You need do delete it from the cmdr_inflight_queue if that is how it
timed out, or if you later call tcmu_get_next_deadline it will still
show up and possibly be used to set the next time out which already
happened.
Firstly, this is in the timeout routine, if this cmd was already timed
out and it must be time_after(jiffies, cmd->deadline), so it won't be
used again.
It could be stuck a long time. What about jiffies rollover?
Okay, will update this.
Thanks.
Re: [PATCH v5] target: add emulate_pr backstore attr to toggle PR support
David, > The new emulate_pr backstore attribute allows for Persistent Reservation > and SCSI2 RESERVE/RELEASE support to be completely disabled. This can be > useful for scenarios such as: > - Ensuring ATS (Compare & Write) usage on recent VMware ESXi initiators. > - Allowing clustered (e.g. tcm-user) backends to block such requests, > avoiding the multi-node reservation state propagation. > > When explicitly disabled, PR and RESERVE/RELEASE requests receive > Invalid Command Operation Code response sense data. Applied to 4.21/scsi-queue. Thank you! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH -next] scsi: libfc: Remove set but not used variable 'disc'
YueHaibing, > From: Yue Haibing > > Fixes gcc '-Wunused-but-set-variable' warning: > > drivers/scsi/libfc/fc_rport.c: In function 'fc_rport_recv_flogi_req': > drivers/scsi/libfc/fc_rport.c:866:18: warning: > variable 'disc' set but not used [-Wunused-but-set-variable] > > It no used any more after > commit baa6719f902a （"libfc: Update rport reference counting") ^^^ Please run checkpatch. Fixed it up and applied to 4.21/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v2] mpt3sas: Display message on Configurable secure HBA
Sreekanth, > Display below warning message only up on detection of > Configurable secure type controllers. > > "HBA is in Configurable Secure mode" > Please put a "---" separator before patch change log or any commentary that shouldn't end up in the commit description. Fixed it up. > v2 change set: > Replaced dev_warn() with dev_info() function while > displaying above message. Applied to 4.21/scsi-queue. Thanks. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v2] megaraid_sas: Add support for MegaRAID Aero controllers
Shivasharan, > This patch adds support for MegaRAID Aero controller PCI IDs. > Throw a message when a Configurable secure type controller is > encountered. Applied to 4.21/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v3] aha1542: convert to DMA mapping API
Christoph, > aha1542 is one of the last users of the legacy isa_*_to_bus APIs, which > also isn't portable enough. Convert it to the proper DMA mapping API. Applied to 4.21/scsi-queue, thank you! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH -next] scsi: libfc: Remove set but not used variable 'disc'
On 2018/11/22 11:15, Martin K. Petersen wrote: > > YueHaibing, > >> From: Yue Haibing >> >> Fixes gcc '-Wunused-but-set-variable' warning: >> >> drivers/scsi/libfc/fc_rport.c: In function 'fc_rport_recv_flogi_req': >> drivers/scsi/libfc/fc_rport.c:866:18: warning: >> variable 'disc' set but not used [-Wunused-but-set-variable] >> >> It no used any more after >> commit baa6719f902a （"libfc: Update rport reference counting") > ^^^ > > Please run checkpatch. Fixed it up and applied to 4.21/scsi-queue. Sorry for this， I will remember to do it next time. >
Re: [PATCH] scsi: csiostor: remove automatic irq affinity assignment
Christoph/Thomas: Please review. Varun has solicited feedback on this a
few times already.
Thanks!
> If number of interrupt vectors are more than num_online_cpus() then
> pci_alloc_irq_vectors_affinity() assigns cpumask based on
> num_possible_cpus() to the remaining vectors because of this
> interrupts are not generating for these vectors.
>
> This patch fixes this issue by using pci_alloc_irq_vectors() instead
> of pci_alloc_irq_vectors_affinity().
>
> Signed-off-by: Varun Prakash
> Cc: Christoph Hellwig
> Cc: Thomas Gleixner
> ---
> drivers/scsi/csiostor/csio_isr.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/csiostor/csio_isr.c
> b/drivers/scsi/csiostor/csio_isr.c
> index 7c88147..8b92c59 100644
> --- a/drivers/scsi/csiostor/csio_isr.c
> +++ b/drivers/scsi/csiostor/csio_isr.c
> @@ -480,7 +480,6 @@ csio_enable_msix(struct csio_hw *hw)
> int i, j, k, n, min, cnt;
> int extra = CSIO_EXTRA_VECS;
> struct csio_scsi_cpu_info *info;
> - struct irq_affinity desc = { .pre_vectors = 2 };
>
> min = hw->num_pports + extra;
> cnt = hw->num_sqsets + extra;
> @@ -491,8 +490,7 @@ csio_enable_msix(struct csio_hw *hw)
>
> csio_dbg(hw, "FW supp #niq:%d, trying %d msix's\n", hw->cfg_niq, cnt);
>
> - cnt = pci_alloc_irq_vectors_affinity(hw->pdev, min, cnt,
> - PCI_IRQ_MSIX | PCI_IRQ_AFFINITY, &desc);
> + cnt = pci_alloc_irq_vectors(hw->pdev, min, cnt, PCI_IRQ_MSIX);
> if (cnt < 0)
> return cnt;
--
Martin K. Petersen Oracle Linux Engineering
Re: [PATCH -next] scsi: bnx2i: remove set but not used variable 'cid_num'
YueHaibing,
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/scsi/bnx2i/bnx2i_hwi.c: In function 'bnx2i_process_ofld_cmpl':
> drivers/scsi/bnx2i/bnx2i_hwi.c:2430:6: warning:
> variable 'cid_num' set but not used [-Wunused-but-set-variable]
>
> It never used since commit
> cf4e6363859d ("[SCSI] bnx2i: Add bnx2i iSCSI driver.")
Applied to 4.21/scsi-queue, thanks!
--
Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] scsi: lpfc: fix block guard enablement on SLI3 adapters
> Since f44ac12f1dcc, BG enablement is tracked with the > LPFC_SLI3_BG_ENABLED bit, which is set in lpfc_get_cfgparam before > lpfc_sli_config_sli_port() is called. The bit shouldn't be cleared > before checking the feature. James, please review. Thanks! -- Martin K. Petersen Oracle Linux Engineering
[PATCH net-next] cxgb4: use new fw interface to get the VIN and smt index
From: Santosh Rastapur
If the fw supports returning VIN/VIVLD in FW_VI_CMD save it
in port_info structure else retrieve these from viid and save
them in port_info structure. Do the same for smt_idx from
FW_VI_MAC_CMD
Signed-off-by: Santosh Rastapur
Signed-off-by: Ganesh Goudar
---
drivers/crypto/chelsio/chtls/chtls_cm.c | 3 +-
drivers/infiniband/hw/cxgb4/cm.c| 6 +--
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 12 -
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 58 -
drivers/net/ethernet/chelsio/cxgb4/l2t.c| 13 +++---
drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 46 ++--
drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h | 20 +
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 3 +-
drivers/target/iscsi/cxgbit/cxgbit_cm.c | 8 ++--
9 files changed, 114 insertions(+), 55 deletions(-)
diff --git a/drivers/crypto/chelsio/chtls/chtls_cm.c
b/drivers/crypto/chelsio/chtls/chtls_cm.c
index 20209e2..228b91b 100644
--- a/drivers/crypto/chelsio/chtls/chtls_cm.c
+++ b/drivers/crypto/chelsio/chtls/chtls_cm.c
@@ -1074,8 +1074,7 @@ static struct sock *chtls_recv_sock(struct sock *lsk,
csk->txq_idx = (rxq_idx < cdev->lldi->ntxq) ? rxq_idx :
port_id * step;
csk->sndbuf = newsk->sk_sndbuf;
- csk->smac_idx = cxgb4_tp_smt_idx(cdev->lldi->adapter_type,
-cxgb4_port_viid(ndev));
+ csk->smac_idx = ((struct port_info *)netdev_priv(ndev))->smt_idx;
tp->rcv_wnd = select_rcv_wnd(csk);
RCV_WSCALE(tp) = select_rcv_wscale(tcp_full_space(newsk),
WSCALE_OK(tp),
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 8ed01e0..97ecc8c 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -2058,8 +2058,7 @@ static int import_ep(struct c4iw_ep *ep, int iptype, __u8
*peer_ip,
}
ep->mtu = pdev->mtu;
ep->tx_chan = cxgb4_port_chan(pdev);
- ep->smac_idx = cxgb4_tp_smt_idx(adapter_type,
- cxgb4_port_viid(pdev));
+ ep->smac_idx = ((struct port_info *)netdev_priv(pdev))->smt_idx;
step = cdev->rdev.lldi.ntxq /
cdev->rdev.lldi.nchan;
ep->txq_idx = cxgb4_port_idx(pdev) * step;
@@ -2078,8 +2077,7 @@ static int import_ep(struct c4iw_ep *ep, int iptype, __u8
*peer_ip,
goto out;
ep->mtu = dst_mtu(dst);
ep->tx_chan = cxgb4_port_chan(pdev);
- ep->smac_idx = cxgb4_tp_smt_idx(adapter_type,
- cxgb4_port_viid(pdev));
+ ep->smac_idx = ((struct port_info *)netdev_priv(pdev))->smt_idx;
step = cdev->rdev.lldi.ntxq /
cdev->rdev.lldi.nchan;
ep->txq_idx = cxgb4_port_idx(pdev) * step;
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index b16f4b3e..2d1ca92 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -404,6 +404,7 @@ struct adapter_params {
bool fr_nsmr_tpte_wr_support; /* FW support for FR_NSMR_TPTE_WR */
u8 fw_caps_support; /* 32-bit Port Capabilities */
bool filter2_wr_support;/* FW support for FILTER2_WR */
+ unsigned int viid_smt_extn_support:1; /* FW returns vin and smt index */
/* MPS Buffer Group Map[per Port]. Bit i is set if buffer group i is
* used by the Port
@@ -592,6 +593,13 @@ struct port_info {
bool ptp_enable;
struct sched_table *sched_tbl;
u32 eth_flags;
+
+ /* viid and smt fields either returned by fw
+* or decoded by parsing viid by driver.
+*/
+ u8 vin;
+ u8 vivld;
+ u8 smt_idx;
};
struct dentry;
@@ -1757,7 +1765,7 @@ int t4_cfg_pfvf(struct adapter *adap, unsigned int mbox,
unsigned int pf,
unsigned int nexact, unsigned int rcaps, unsigned int wxcaps);
int t4_alloc_vi(struct adapter *adap, unsigned int mbox, unsigned int port,
unsigned int pf, unsigned int vf, unsigned int nmac, u8 *mac,
- unsigned int *rss_size);
+ unsigned int *rss_size, u8 *vivld, u8 *vin);
int t4_free_vi(struct adapter *adap, unsigned int mbox,
unsigned int pf, unsigned int vf,
unsigned int viid);
@@ -1783,7 +1791,7 @@ int t4_free_mac_filt(struct adapter *adap, unsigned int
mbox,
unsigned int viid, unsigned int naddr,
const u8 **addr, bool sleep_ok);
int t4_change_mac(struct adapter *adap, unsigned int mbox, unsigned int viid,
- int idx, const u8 *addr, bool persist, bool add_smt);
+
Re: [PATCH 3/4] scsi: hisi_sas: Make sg_tablesize consistent value
On 11/20/2018 03:59 PM, John Garry wrote:
From: Xiang Chen
Sht->sg_tablesize is set in the driver, and it will be assigned to
shost->sg_tablesize in SCSI mid-layer. So it is not necessary to
assign shost->sg_table one more time in the driver.
In addition to the change, change each scsi_host_template.sg_tablesize
to HISI_SAS_SGE_PAGE_CNT instead of SG_ALL.
Might be completely irrelevant, so just as information: I once had
problems due to changing (reducing) SHT.sg_tablesize because block queue
limits of BSG devices of Scsi_Host and fc_host (not sure if you have an
equivalent bsg device for your transport(s)) inherit from SHT, but don't
update (automatically) on later updates of shost->sg_tablesize, which in
turn affect scsi_devices allocated after the shost update.
Cf.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/s390/scsi?id=5fea4291deacd80188b996d2f555fc6a1940e5d4
("[SCSI] zfcp: block queue limits with data router")
if you need more details.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_main.c | 1 -
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 2 +-
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 2 +-
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +--
4 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index d13a662..cbda48e 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -2410,7 +2410,6 @@ int hisi_sas_probe(struct platform_device *pdev,
shost->max_lun = ~0;
shost->max_channel = 1;
shost->max_cmd_len = 16;
- shost->sg_tablesize = min_t(u16, SG_ALL, HISI_SAS_SGE_PAGE_CNT);
if (hisi_hba->hw->slot_index_alloc) {
shost->can_queue = hisi_hba->hw->max_command_entries;
shost->cmd_per_lun = hisi_hba->hw->max_command_entries;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index d24342b..2d035cc 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -1815,7 +1815,7 @@ static int hisi_sas_v1_init(struct hisi_hba *hisi_hba)
.change_queue_depth = sas_change_queue_depth,
.bios_param = sas_bios_param,
.this_id= -1,
- .sg_tablesize = SG_ALL,
+ .sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
.max_sectors= SCSI_DEFAULT_MAX_SECTORS,
.use_clustering = ENABLE_CLUSTERING,
.eh_device_reset_handler = sas_eh_device_reset_handler,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index e78a97e..79e58a7 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -3570,7 +3570,7 @@ struct device_attribute *host_attrs_v2_hw[] = {
.change_queue_depth = sas_change_queue_depth,
.bios_param = sas_bios_param,
.this_id= -1,
- .sg_tablesize = SG_ALL,
+ .sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
.max_sectors= SCSI_DEFAULT_MAX_SECTORS,
.use_clustering = ENABLE_CLUSTERING,
.eh_device_reset_handler = sas_eh_device_reset_handler,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 7e2b020..8a08078 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -2224,7 +2224,7 @@ struct device_attribute *host_attrs_v3_hw[] = {
.change_queue_depth = sas_change_queue_depth,
.bios_param = sas_bios_param,
.this_id= -1,
- .sg_tablesize = SG_ALL,
+ .sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
.max_sectors= SCSI_DEFAULT_MAX_SECTORS,
.use_clustering = ENABLE_CLUSTERING,
.eh_device_reset_handler = sas_eh_device_reset_handler,
@@ -2366,7 +2366,6 @@ struct device_attribute *host_attrs_v3_hw[] = {
shost->max_lun = ~0;
shost->max_channel = 1;
shost->max_cmd_len = 16;
- shost->sg_tablesize = min_t(u16, SG_ALL, HISI_SAS_SGE_PAGE_CNT);
shost->can_queue = hisi_hba->hw->max_command_entries -
HISI_SAS_RESERVED_IPTT_CNT;
shost->cmd_per_lun = hisi_hba->hw->max_command_entries -
--
Mit freundlichen Gruessen / Kind regards
Steffen Maier
Linux on IBM Z Development
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [PATCH 3/4] scsi: hisi_sas: Make sg_tablesize consistent value
On 11/21/2018 12:02 PM, Steffen Maier wrote:
On 11/20/2018 03:59 PM, John Garry wrote:
From: Xiang Chen
Sht->sg_tablesize is set in the driver, and it will be assigned to
shost->sg_tablesize in SCSI mid-layer. So it is not necessary to
assign shost->sg_table one more time in the driver.
In addition to the change, change each scsi_host_template.sg_tablesize
to HISI_SAS_SGE_PAGE_CNT instead of SG_ALL.
Might be completely irrelevant, so just as information: I once had
problems due to changing (reducing) SHT.sg_tablesize because block queue
limits of BSG devices of Scsi_Host and fc_host (not sure if you have an
equivalent bsg device for your transport(s)) inherit from SHT, but don't
update (automatically) on later updates of shost->sg_tablesize, which in
turn affect scsi_devices allocated after the shost update.
Cf.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/s390/scsi?id=5fea4291deacd80188b996d2f555fc6a1940e5d4
Figured, your new constant seems to have the same value so no problem.
#define SG_CHUNK_SIZE 128
#define SG_ALL SG_CHUNK_SIZE
#define HISI_SAS_SGE_PAGE_CNT SG_CHUNK_SIZE
("[SCSI] zfcp: block queue limits with data router")
if you need more details.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_main.c | 1 -
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 2 +-
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 2 +-
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +--
4 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index d13a662..cbda48e 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -2410,7 +2410,6 @@ int hisi_sas_probe(struct platform_device *pdev,
shost->max_lun = ~0;
shost->max_channel = 1;
shost->max_cmd_len = 16;
- shost->sg_tablesize = min_t(u16, SG_ALL, HISI_SAS_SGE_PAGE_CNT);
if (hisi_hba->hw->slot_index_alloc) {
shost->can_queue = hisi_hba->hw->max_command_entries;
shost->cmd_per_lun = hisi_hba->hw->max_command_entries;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index d24342b..2d035cc 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -1815,7 +1815,7 @@ static int hisi_sas_v1_init(struct hisi_hba
*hisi_hba)
.change_queue_depth = sas_change_queue_depth,
.bios_param = sas_bios_param,
.this_id = -1,
- .sg_tablesize = SG_ALL,
+ .sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
.max_sectors = SCSI_DEFAULT_MAX_SECTORS,
.use_clustering = ENABLE_CLUSTERING,
.eh_device_reset_handler = sas_eh_device_reset_handler,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index e78a97e..79e58a7 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -3570,7 +3570,7 @@ struct device_attribute *host_attrs_v2_hw[] = {
.change_queue_depth = sas_change_queue_depth,
.bios_param = sas_bios_param,
.this_id = -1,
- .sg_tablesize = SG_ALL,
+ .sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
.max_sectors = SCSI_DEFAULT_MAX_SECTORS,
.use_clustering = ENABLE_CLUSTERING,
.eh_device_reset_handler = sas_eh_device_reset_handler,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 7e2b020..8a08078 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -2224,7 +2224,7 @@ struct device_attribute *host_attrs_v3_hw[] = {
.change_queue_depth = sas_change_queue_depth,
.bios_param = sas_bios_param,
.this_id = -1,
- .sg_tablesize = SG_ALL,
+ .sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
.max_sectors = SCSI_DEFAULT_MAX_SECTORS,
.use_clustering = ENABLE_CLUSTERING,
.eh_device_reset_handler = sas_eh_device_reset_handler,
@@ -2366,7 +2366,6 @@ struct device_attribute *host_attrs_v3_hw[] = {
shost->max_lun = ~0;
shost->max_channel = 1;
shost->max_cmd_len = 16;
- shost->sg_tablesize = min_t(u16, SG_ALL, HISI_SAS_SGE_PAGE_CNT);
shost->can_queue = hisi_hba->hw->max_command_entries -
HISI_SAS_RESERVED_IPTT_CNT;
shost->cmd_per_lun = hisi_hba->hw->max_command_entries -
--
Mit freundlichen Gruessen / Kind regards
Steffen Maier
Linux on IBM Z Development
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [PATCH 3/4] scsi: hisi_sas: Make sg_tablesize consistent value
On 21/11/2018 11:08, Steffen Maier wrote:
On 11/21/2018 12:02 PM, Steffen Maier wrote:
On 11/20/2018 03:59 PM, John Garry wrote:
From: Xiang Chen
Sht->sg_tablesize is set in the driver, and it will be assigned to
shost->sg_tablesize in SCSI mid-layer. So it is not necessary to
assign shost->sg_table one more time in the driver.
In addition to the change, change each scsi_host_template.sg_tablesize
to HISI_SAS_SGE_PAGE_CNT instead of SG_ALL.
Might be completely irrelevant, so just as information: I once had
problems due to changing (reducing) SHT.sg_tablesize because block
queue limits of BSG devices of Scsi_Host and fc_host (not sure if you
have an equivalent bsg device for your transport(s)) inherit from SHT,
but don't update (automatically) on later updates of
shost->sg_tablesize, which in turn affect scsi_devices allocated after
the shost update.
Cf.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/s390/scsi?id=5fea4291deacd80188b996d2f555fc6a1940e5d4
Figured, your new constant seems to have the same value so no problem.
Right, so previously we were doing similar to what you describe -
setting the value in the SHT and then setting shost->sg_tablesize after
the host is allocated.
However the values were the same, so in this patch we're just removing
setting shost->sg_tablesize (again).
Thanks,
John
#define SG_CHUNK_SIZE128
#define SG_ALLSG_CHUNK_SIZE
#define HISI_SAS_SGE_PAGE_CNT SG_CHUNK_SIZE
("[SCSI] zfcp: block queue limits with data router")
if you need more details.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_main.c | 1 -
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 2 +-
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 2 +-
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +--
4 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index d13a662..cbda48e 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -2410,7 +2410,6 @@ int hisi_sas_probe(struct platform_device *pdev,
shost->max_lun = ~0;
shost->max_channel = 1;
shost->max_cmd_len = 16;
-shost->sg_tablesize = min_t(u16, SG_ALL, HISI_SAS_SGE_PAGE_CNT);
if (hisi_hba->hw->slot_index_alloc) {
shost->can_queue = hisi_hba->hw->max_command_entries;
shost->cmd_per_lun = hisi_hba->hw->max_command_entries;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index d24342b..2d035cc 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -1815,7 +1815,7 @@ static int hisi_sas_v1_init(struct hisi_hba
*hisi_hba)
.change_queue_depth= sas_change_queue_depth,
.bios_param= sas_bios_param,
.this_id= -1,
-.sg_tablesize= SG_ALL,
+.sg_tablesize= HISI_SAS_SGE_PAGE_CNT,
.max_sectors= SCSI_DEFAULT_MAX_SECTORS,
.use_clustering= ENABLE_CLUSTERING,
.eh_device_reset_handler = sas_eh_device_reset_handler,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index e78a97e..79e58a7 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -3570,7 +3570,7 @@ struct device_attribute *host_attrs_v2_hw[] = {
.change_queue_depth= sas_change_queue_depth,
.bios_param= sas_bios_param,
.this_id= -1,
-.sg_tablesize= SG_ALL,
+.sg_tablesize= HISI_SAS_SGE_PAGE_CNT,
.max_sectors= SCSI_DEFAULT_MAX_SECTORS,
.use_clustering= ENABLE_CLUSTERING,
.eh_device_reset_handler = sas_eh_device_reset_handler,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 7e2b020..8a08078 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -2224,7 +2224,7 @@ struct device_attribute *host_attrs_v3_hw[] = {
.change_queue_depth= sas_change_queue_depth,
.bios_param= sas_bios_param,
.this_id= -1,
-.sg_tablesize= SG_ALL,
+.sg_tablesize= HISI_SAS_SGE_PAGE_CNT,
.max_sectors= SCSI_DEFAULT_MAX_SECTORS,
.use_clustering= ENABLE_CLUSTERING,
.eh_device_reset_handler = sas_eh_device_reset_handler,
@@ -2366,7 +2366,6 @@ struct device_attribute *host_attrs_v3_hw[] = {
shost->max_lun = ~0;
shost->max_channel = 1;
shost->max_cmd_len = 16;
-shost->sg_tablesize = min_t(u16, SG_ALL, HISI_SAS_SGE_PAGE_CNT);
shost->can_queue = hisi_hba->hw->max_command_entries -
HISI_SAS_RESERVED_IPTT_CNT;
shost->cmd_per_lun = hisi_hba->hw->max_command_entries -
Re: [PATCHv3 2/3] scsi: Do not rely on blk-mq for double completions
On Mon, Nov 19, 2018 at 08:19:00AM -0700, Keith Busch wrote:
> On Mon, Nov 19, 2018 at 12:58:15AM -0800, Christoph Hellwig wrote:
> > > index 5d83a162d03b..c1d5e4e36125 100644
> > > --- a/drivers/scsi/scsi_lib.c
> > > +++ b/drivers/scsi/scsi_lib.c
> > > @@ -1635,8 +1635,11 @@ static blk_status_t scsi_mq_prep_fn(struct request
> > > *req)
> > >
> > > static void scsi_mq_done(struct scsi_cmnd *cmd)
> > > {
> > > + if (unlikely(test_and_set_bit(__SCMD_COMPLETE, &cmd->flags)))
> > > + return;
> > > trace_scsi_dispatch_cmd_done(cmd);
> > > - blk_mq_complete_request(cmd->request);
> > > + if (unlikely(!blk_mq_complete_request(cmd->request)))
> > > + clear_bit(__SCMD_COMPLETE, &cmd->flags);
> > > }
> >
> > This looks a little odd to me. If we didn't complete the command
> > someone else did. Why would we clear the bit in this case?
>
> It's only to go along with the fake timeout. If we don't clear the bit,
> then then scsi timeout handler will believe it has nothing to do because
> scsi did its required part. The block layer just pretends the LLD didn't
> do its part, so scsi has to play along too.
This just looks way to magic to me. In other word - it needs a big fat
comment explaining the situation.
> > > +#define __SCMD_COMPLETE 3
> > > +#define SCMD_COMPLETE(1 << __SCMD_COMPLETE)
> >
> > This mixing of atomic and non-atomic bitops looks rather dangerous
> > to me. Can you add a new atomic_flags just for the completed flag,
> > and always use the bitops on it for now? I think we can eventually
> > kill most of the existing flags except for SCMD_TAGGED over the
> > next merge window or two and then move that over as well.
>
> The only concurrent access is completion + timeout, otherwise access is
> single-threaded. I'm using the atomic operations only where it is
> needed.
>
> We implicitly clear the SCMD_COMPLETED flag along with SCMD_TAGGED in
> scsi_init_command() too, and I didn't want to add new overhead with
> new atomics.
In general mixing access types on a single field (nevermind bit)
is going to cause us problems further down the road sooner or later.
I'd be much happier with a separate field.
[PATCH] scsi/mvsas/mv_init.c: Use dma_zalloc_coherent
Replace dma_alloc_coherent + memset with dma_zalloc_coherent Signed-off-by: Sabyasachi Gupta --- drivers/scsi/mvsas/mv_init.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c index 3ac3437..495bddb 100644 --- a/drivers/scsi/mvsas/mv_init.c +++ b/drivers/scsi/mvsas/mv_init.c @@ -253,33 +253,29 @@ static int mvs_alloc(struct mvs_info *mvi, struct Scsi_Host *shost) /* * alloc and init our DMA areas */ - mvi->tx = dma_alloc_coherent(mvi->dev, + mvi->tx = dma_zalloc_coherent(mvi->dev, sizeof(*mvi->tx) * MVS_CHIP_SLOT_SZ, &mvi->tx_dma, GFP_KERNEL); if (!mvi->tx) goto err_out; - memset(mvi->tx, 0, sizeof(*mvi->tx) * MVS_CHIP_SLOT_SZ); - mvi->rx_fis = dma_alloc_coherent(mvi->dev, MVS_RX_FISL_SZ, + mvi->rx_fis = dma_zalloc_coherent(mvi->dev, MVS_RX_FISL_SZ, &mvi->rx_fis_dma, GFP_KERNEL); if (!mvi->rx_fis) goto err_out; - memset(mvi->rx_fis, 0, MVS_RX_FISL_SZ); - mvi->rx = dma_alloc_coherent(mvi->dev, + mvi->rx = dma_zalloc_coherent(mvi->dev, sizeof(*mvi->rx) * (MVS_RX_RING_SZ + 1), &mvi->rx_dma, GFP_KERNEL); if (!mvi->rx) goto err_out; - memset(mvi->rx, 0, sizeof(*mvi->rx) * (MVS_RX_RING_SZ + 1)); mvi->rx[0] = cpu_to_le32(0xfff); mvi->rx_cons = 0xfff; - mvi->slot = dma_alloc_coherent(mvi->dev, + mvi->slot = dma_zalloc_coherent(mvi->dev, sizeof(*mvi->slot) * slot_nr, &mvi->slot_dma, GFP_KERNEL); if (!mvi->slot) goto err_out; - memset(mvi->slot, 0, sizeof(*mvi->slot) * slot_nr); mvi->bulk_buffer = dma_alloc_coherent(mvi->dev, TRASH_BUCKET_SIZE, -- 2.7.4
[PATCH v2 3/3] scsi: hisi_sas: Add support for DIF/DIX feature for v3 hw
From: Xiang Chen
For v3 hw, we support DIF/DIX operation for SAS, but not SATA.
In addition, DIF CRC16 is supported.
This patchset adds the SW support for the described features. The main
components are as follows:
- Allocate memory for PI
- Fill PI fields
- Fill related to DIF/DIX in DQ and protection iu memories
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas.h | 18
drivers/scsi/hisi_sas/hisi_sas_main.c | 99 ++---
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 189 -
3 files changed, 289 insertions(+), 17 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 535c613..a73aad6 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -55,6 +55,11 @@
#define hisi_sas_sge_addr_mem(slot) hisi_sas_sge_addr(slot->buf)
#define hisi_sas_sge_addr_dma(slot) hisi_sas_sge_addr(slot->buf_dma)
+#define hisi_sas_sge_dif_addr(buf) \
+ (buf + offsetof(struct hisi_sas_slot_dif_buf_table, sge_dif_page))
+#define hisi_sas_sge_dif_addr_mem(slot) hisi_sas_sge_dif_addr(slot->buf)
+#define hisi_sas_sge_dif_addr_dma(slot) hisi_sas_sge_dif_addr(slot->buf_dma)
+
#define HISI_SAS_MAX_SSP_RESP_SZ (sizeof(struct ssp_frame_hdr) + 1024)
#define HISI_SAS_MAX_SMP_RESP_SZ 1028
#define HISI_SAS_MAX_STP_RESP_SZ 28
@@ -197,6 +202,7 @@ struct hisi_sas_slot {
struct sas_task *task;
struct hisi_sas_port*port;
u64 n_elem;
+ u64 n_elem_dif;
int dlvry_queue;
int dlvry_queue_slot;
int cmplt_queue;
@@ -268,6 +274,8 @@ struct hisi_hba {
struct pci_dev *pci_dev;
struct device *dev;
+ bool enable_dif_dix;
+
void __iomem *regs;
void __iomem *sgpio_regs;
struct regmap *ctrl;
@@ -422,6 +430,11 @@ struct hisi_sas_sge_page {
struct hisi_sas_sge sge[HISI_SAS_SGE_PAGE_CNT];
} __aligned(16);
+#define HISI_SAS_SGE_DIF_PAGE_CNT SG_CHUNK_SIZE
+struct hisi_sas_sge_dif_page {
+ struct hisi_sas_sge sge[HISI_SAS_SGE_DIF_PAGE_CNT];
+} __aligned(16);
+
struct hisi_sas_command_table_ssp {
struct ssp_frame_hdr hdr;
union {
@@ -452,6 +465,11 @@ struct hisi_sas_slot_buf_table {
struct hisi_sas_sge_page sge_page;
};
+struct hisi_sas_slot_dif_buf_table {
+ struct hisi_sas_slot_buf_table slot_buf;
+ struct hisi_sas_sge_dif_page sge_dif_page;
+};
+
extern struct scsi_transport_template *hisi_sas_stt;
extern void hisi_sas_stop_phys(struct hisi_hba *hisi_hba);
extern int hisi_sas_alloc(struct hisi_hba *hisi_hba, struct Scsi_Host *shost);
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index cbda48e..d0b693b 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -252,14 +252,21 @@ void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba,
struct sas_task *task,
task->lldd_task = NULL;
- if (!sas_protocol_ata(task->task_proto))
+ if (!sas_protocol_ata(task->task_proto)) {
+ struct sas_ssp_task *ssp_task = &task->ssp_task;
+ struct scsi_cmnd *scsi_cmnd = ssp_task->cmd;
+
if (slot->n_elem)
dma_unmap_sg(dev, task->scatter,
task->num_scatter,
task->data_dir);
+ if (slot->n_elem_dif)
+ dma_unmap_sg(dev, scsi_prot_sglist(scsi_cmnd),
+scsi_prot_sg_count(scsi_cmnd),
+task->data_dir);
+ }
}
-
spin_lock_irqsave(&dq->lock, flags);
list_del_init(&slot->entry);
spin_unlock_irqrestore(&dq->lock, flags);
@@ -380,6 +387,59 @@ static int hisi_sas_dma_map(struct hisi_hba *hisi_hba,
return rc;
}
+static void hisi_sas_dif_dma_unmap(struct hisi_hba *hisi_hba,
+ struct sas_task *task, int n_elem_dif)
+{
+ struct device *dev = hisi_hba->dev;
+
+ if (n_elem_dif) {
+ struct sas_ssp_task *ssp_task = &task->ssp_task;
+ struct scsi_cmnd *scsi_cmnd = ssp_task->cmd;
+
+ dma_unmap_sg(dev, scsi_prot_sglist(scsi_cmnd),
+scsi_prot_sg_count(scsi_cmnd),
+task->data_dir);
+ }
+}
+
+static int hisi_sas_dif_dma_map(struct hisi_hba *hisi_hba,
+ int *n_elem_dif, struct sas_task *task)
+{
+ struct device *dev = hisi_hba->dev;
+ struct sas_ssp_task *ssp_task;
+ struct scsi_cmnd *scsi_cmnd;
+ int rc;
+
+ if (task->num_scatter) {
+ ssp_task = &task->ssp_task;
+ scsi_cmnd = ssp_task->cmd;
+
+ if
[PATCH v2 2/3] scsi: hisi_sas: Make sg_tablesize consistent value
From: Xiang Chen
Sht->sg_tablesize is set in the driver, and it will be assigned to
shost->sg_tablesize in SCSI mid-layer. So it is not necessary to
assign shost->sg_table one more time in the driver.
In addition to the change, change each scsi_host_template.sg_tablesize
to HISI_SAS_SGE_PAGE_CNT instead of SG_ALL.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_main.c | 1 -
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 2 +-
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 2 +-
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +--
4 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index d13a662..cbda48e 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -2410,7 +2410,6 @@ int hisi_sas_probe(struct platform_device *pdev,
shost->max_lun = ~0;
shost->max_channel = 1;
shost->max_cmd_len = 16;
- shost->sg_tablesize = min_t(u16, SG_ALL, HISI_SAS_SGE_PAGE_CNT);
if (hisi_hba->hw->slot_index_alloc) {
shost->can_queue = hisi_hba->hw->max_command_entries;
shost->cmd_per_lun = hisi_hba->hw->max_command_entries;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index d24342b..2d035cc 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -1815,7 +1815,7 @@ static int hisi_sas_v1_init(struct hisi_hba *hisi_hba)
.change_queue_depth = sas_change_queue_depth,
.bios_param = sas_bios_param,
.this_id= -1,
- .sg_tablesize = SG_ALL,
+ .sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
.max_sectors= SCSI_DEFAULT_MAX_SECTORS,
.use_clustering = ENABLE_CLUSTERING,
.eh_device_reset_handler = sas_eh_device_reset_handler,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index e78a97e..79e58a7 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -3570,7 +3570,7 @@ struct device_attribute *host_attrs_v2_hw[] = {
.change_queue_depth = sas_change_queue_depth,
.bios_param = sas_bios_param,
.this_id= -1,
- .sg_tablesize = SG_ALL,
+ .sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
.max_sectors= SCSI_DEFAULT_MAX_SECTORS,
.use_clustering = ENABLE_CLUSTERING,
.eh_device_reset_handler = sas_eh_device_reset_handler,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 7e2b020..8a08078 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -2224,7 +2224,7 @@ struct device_attribute *host_attrs_v3_hw[] = {
.change_queue_depth = sas_change_queue_depth,
.bios_param = sas_bios_param,
.this_id= -1,
- .sg_tablesize = SG_ALL,
+ .sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
.max_sectors= SCSI_DEFAULT_MAX_SECTORS,
.use_clustering = ENABLE_CLUSTERING,
.eh_device_reset_handler = sas_eh_device_reset_handler,
@@ -2366,7 +2366,6 @@ struct device_attribute *host_attrs_v3_hw[] = {
shost->max_lun = ~0;
shost->max_channel = 1;
shost->max_cmd_len = 16;
- shost->sg_tablesize = min_t(u16, SG_ALL, HISI_SAS_SGE_PAGE_CNT);
shost->can_queue = hisi_hba->hw->max_command_entries -
HISI_SAS_RESERVED_IPTT_CNT;
shost->cmd_per_lun = hisi_hba->hw->max_command_entries -
--
1.9.1
[PATCH v2 1/3] scsi: hisi_sas: Relocate some code to reduce complexity
From: Xiang Chen
Relocate the codes related to dma_map/unmap in hisi_sas_task_prep()
to reduce complexity, with a view to add DIF/DIX support.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_main.c | 146 +-
1 file changed, 90 insertions(+), 56 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 65dc749..d13a662 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -296,6 +296,90 @@ static void hisi_sas_task_prep_abort(struct hisi_hba
*hisi_hba,
device_id, abort_flag, tag_to_abort);
}
+static void hisi_sas_dma_unmap(struct hisi_hba *hisi_hba,
+ struct sas_task *task, int n_elem,
+ int n_elem_req, int n_elem_resp)
+{
+ struct device *dev = hisi_hba->dev;
+
+ if (!sas_protocol_ata(task->task_proto)) {
+ if (task->num_scatter) {
+ if (n_elem)
+ dma_unmap_sg(dev, task->scatter,
+task->num_scatter,
+task->data_dir);
+ } else if (task->task_proto & SAS_PROTOCOL_SMP) {
+ if (n_elem_req)
+ dma_unmap_sg(dev, &task->smp_task.smp_req,
+1, DMA_TO_DEVICE);
+ if (n_elem_resp)
+ dma_unmap_sg(dev, &task->smp_task.smp_resp,
+1, DMA_FROM_DEVICE);
+ }
+ }
+}
+
+static int hisi_sas_dma_map(struct hisi_hba *hisi_hba,
+ struct sas_task *task, int *n_elem,
+ int *n_elem_req, int *n_elem_resp)
+{
+ struct device *dev = hisi_hba->dev;
+ int rc;
+
+ if (sas_protocol_ata(task->task_proto)) {
+ *n_elem = task->num_scatter;
+ } else {
+ unsigned int req_len, resp_len;
+
+ if (task->num_scatter) {
+ *n_elem = dma_map_sg(dev, task->scatter,
+task->num_scatter, task->data_dir);
+ if (!*n_elem) {
+ rc = -ENOMEM;
+ goto prep_out;
+ }
+ } else if (task->task_proto & SAS_PROTOCOL_SMP) {
+ *n_elem_req = dma_map_sg(dev, &task->smp_task.smp_req,
+1, DMA_TO_DEVICE);
+ if (!*n_elem_req) {
+ rc = -ENOMEM;
+ goto prep_out;
+ }
+ req_len = sg_dma_len(&task->smp_task.smp_req);
+ if (req_len & 0x3) {
+ rc = -EINVAL;
+ goto err_out_dma_unmap;
+ }
+ *n_elem_resp = dma_map_sg(dev, &task->smp_task.smp_resp,
+ 1, DMA_FROM_DEVICE);
+ if (!*n_elem_resp) {
+ rc = -ENOMEM;
+ goto err_out_dma_unmap;
+ }
+ resp_len = sg_dma_len(&task->smp_task.smp_resp);
+ if (resp_len & 0x3) {
+ rc = -EINVAL;
+ goto err_out_dma_unmap;
+ }
+ }
+ }
+
+ if (*n_elem > HISI_SAS_SGE_PAGE_CNT) {
+ dev_err(dev, "task prep: n_elem(%d) > HISI_SAS_SGE_PAGE_CNT",
+ *n_elem);
+ rc = -EINVAL;
+ goto err_out_dma_unmap;
+ }
+ return 0;
+
+err_out_dma_unmap:
+ /* It would be better to call dma_unmap_sg() here, but it's messy */
+ hisi_sas_dma_unmap(hisi_hba, task, *n_elem,
+ *n_elem_req, *n_elem_resp);
+prep_out:
+ return rc;
+}
+
static int hisi_sas_task_prep(struct sas_task *task,
struct hisi_sas_dq **dq_pointer,
bool is_tmf, struct hisi_sas_tmf_task *tmf,
@@ -338,49 +422,10 @@ static int hisi_sas_task_prep(struct sas_task *task,
return -ECOMM;
}
- if (!sas_protocol_ata(task->task_proto)) {
- unsigned int req_len, resp_len;
-
- if (task->num_scatter) {
- n_elem = dma_map_sg(dev, task->scatter,
- task->num_scatter, task->data_dir);
- if (!n_elem) {
- rc = -ENOMEM;
- goto prep_out;
- }
- } else if (task->task_proto & SAS_PRO
[PATCH v2 0/3] hisi_sas: DIF/DIX support
This patchset introduces support to the driver for DIF/DIX (or PI - protection information). We will only support PI in v3 hw at the moment, even though previous hw versions also support it. The series is broken down as follows: - Tidy sg table size config - Some tidy-up to accept PI support - Add components for PI support for main and v3 driver Difference v1->v2: - drop scsi_prot_op_normal() Xiang Chen (3): scsi: hisi_sas: Relocate some code to reduce complexity scsi: hisi_sas: Make sg_tablesize consistent value scsi: hisi_sas: Add support for DIF/DIX feature for v3 hw drivers/scsi/hisi_sas/hisi_sas.h | 18 +++ drivers/scsi/hisi_sas/hisi_sas_main.c | 242 - drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 2 +- drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 2 +- drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 192 +- 5 files changed, 380 insertions(+), 76 deletions(-) -- 1.9.1
Re: [PATCH v2] codafs: Fix build using bare-metal toolchain
On Tue, Oct 30, 2018 at 10:27 PM Sam Protsenko wrote: > > The kernel is self-contained project and can be built with bare-metal > toolchain. But bare-metal toolchain doesn't define __linux__. Because of > this u_quad_t type is not defined when using bare-metal toolchain and > codafs build fails. This patch fixes it by defining u_quad_t type > unconditionally. > > Cc: Jan Harkes > Cc: Christoph Hellwig > Cc: Andy Shevchenko > Signed-off-by: Sam Protsenko > --- > include/linux/coda.h | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/include/linux/coda.h b/include/linux/coda.h > index d30209b9cef8..0ca0c83fdb1c 100644 > --- a/include/linux/coda.h > +++ b/include/linux/coda.h > @@ -58,8 +58,7 @@ Mellon the rights to redistribute these changes without > encumbrance. > #ifndef _CODA_HEADER_ > #define _CODA_HEADER_ > > -#if defined(__linux__) > typedef unsigned long long u_quad_t; > -#endif > + > #include > #endif > -- > 2.19.1 > + Jan Harkes, + Ruslan Bilovol Hi Jan, Can you please apply this? Nobody seems to be interested in taking this patch, so I'm not sure how to proceed further. Please advice. Thanks!
Re: [PATCH v2] codafs: Fix build using bare-metal toolchain
On Wed, Nov 21, 2018 at 6:31 PM Sam Protsenko wrote: > > On Tue, Oct 30, 2018 at 10:27 PM Sam Protsenko > wrote: > > > > The kernel is self-contained project and can be built with bare-metal > > toolchain. But bare-metal toolchain doesn't define __linux__. Because of > > this u_quad_t type is not defined when using bare-metal toolchain and > > codafs build fails. This patch fixes it by defining u_quad_t type > > unconditionally. > > > > Cc: Jan Harkes > > Cc: Christoph Hellwig > > Cc: Andy Shevchenko I'm not sure how you managed to miss people in this list (perhaps by default you have suppress all Cc in your Git configuration), but I guess we may gently ask Christoph to apply this in case Jan will not appear. > > Signed-off-by: Sam Protsenko > > --- > > include/linux/coda.h | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/include/linux/coda.h b/include/linux/coda.h > > index d30209b9cef8..0ca0c83fdb1c 100644 > > --- a/include/linux/coda.h > > +++ b/include/linux/coda.h > > @@ -58,8 +58,7 @@ Mellon the rights to redistribute these changes without > > encumbrance. > > #ifndef _CODA_HEADER_ > > #define _CODA_HEADER_ > > > > -#if defined(__linux__) > > typedef unsigned long long u_quad_t; > > -#endif > > + > > #include > > #endif > > -- > > 2.19.1 > > > > + Jan Harkes, + Ruslan Bilovol > > Hi Jan, > > Can you please apply this? Nobody seems to be interested in taking > this patch, so I'm not sure how to proceed further. Please advice. > > Thanks! -- With Best Regards, Andy Shevchenko
Re: [PATCH v2] codafs: Fix build using bare-metal toolchain
On Wed, Nov 21, 2018 at 06:41:13PM +0200, Andy Shevchenko wrote: > I'm not sure how you managed to miss people in this list (perhaps by > default you have suppress all Cc in your Git configuration), but I > guess we may gently ask Christoph to apply this in case Jan will not > appear. I don't really have a relevant tree. You probably want to send it to Al Viro.
[PATCH] iscsi: Capture iscsi debug messages using tracepoints
From: Fred Herard This commit enhances iscsi initiator modules to capture iscsi debug messages using linux kernel tracepoint facility: https://www.kernel.org/doc/Documentation/trace/tracepoints.txt The following tracepoint events have been created under the iscsi tracepoint event group: iscsi_dbg_conn - to capture connection debug messages (libiscsi module) iscsi_dbg_session - to capture session debug messages (libiscsi module) iscsi_dbg_eh - to capture error handling debug messages (libiscsi module) iscsi_dbg_tcp - to capture iscsi tcp debug messages (libiscsi_tcp module) iscsi_dbg_sw_tcp - to capture iscsi sw tcp debug messages (iscsi_tcp module) iscsi_dbg_trans_session - to cpature iscsi trasnsport sess debug messages (scsi_transport_iscsi module) iscsi_dbg_trans_conn - to capture iscsi tansport conn debug messages (scsi_transport_iscsi module) Signed-off-by: Fred Herard Reviewed-by: Rajan Shanmugavelu --- drivers/scsi/iscsi_tcp.c| 4 ++ drivers/scsi/libiscsi.c | 10 +++ drivers/scsi/libiscsi_tcp.c | 4 ++ drivers/scsi/scsi_transport_iscsi.c | 34 - include/trace/events/iscsi.h| 107 5 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 include/trace/events/iscsi.h diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c index e11eff6b0e97..cf6475af72b1 100644 --- a/drivers/scsi/iscsi_tcp.c +++ b/drivers/scsi/iscsi_tcp.c @@ -44,6 +44,7 @@ #include #include #include +#include #include "iscsi_tcp.h" @@ -72,6 +73,9 @@ MODULE_PARM_DESC(debug_iscsi_tcp, "Turn on debugging for iscsi_tcp module " iscsi_conn_printk(KERN_INFO, _conn, \ "%s " dbg_fmt, \ __func__, ##arg); \ + iscsi_dbg_trace(trace_iscsi_dbg_sw_tcp, \ + &(_conn)->cls_conn->dev,\ + "%s " dbg_fmt, __func__, ##arg);\ } while (0); diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c index cf8a15e54d83..088e90308c1f 100644 --- a/drivers/scsi/libiscsi.c +++ b/drivers/scsi/libiscsi.c @@ -40,6 +40,7 @@ #include #include #include +#include static int iscsi_dbg_lib_conn; module_param_named(debug_libiscsi_conn, iscsi_dbg_lib_conn, int, @@ -68,6 +69,9 @@ MODULE_PARM_DESC(debug_libiscsi_eh, iscsi_conn_printk(KERN_INFO, _conn, \ "%s " dbg_fmt, \ __func__, ##arg); \ + iscsi_dbg_trace(trace_iscsi_dbg_conn, \ + &(_conn)->cls_conn->dev,\ + "%s " dbg_fmt, __func__, ##arg);\ } while (0); #define ISCSI_DBG_SESSION(_session, dbg_fmt, arg...) \ @@ -76,6 +80,9 @@ MODULE_PARM_DESC(debug_libiscsi_eh, iscsi_session_printk(KERN_INFO, _session, \ "%s " dbg_fmt, \ __func__, ##arg); \ + iscsi_dbg_trace(trace_iscsi_dbg_session,\ + &(_session)->cls_session->dev, \ + "%s " dbg_fmt, __func__, ##arg);\ } while (0); #define ISCSI_DBG_EH(_session, dbg_fmt, arg...) \ @@ -84,6 +91,9 @@ MODULE_PARM_DESC(debug_libiscsi_eh, iscsi_session_printk(KERN_INFO, _session, \ "%s " dbg_fmt, \ __func__, ##arg); \ + iscsi_dbg_trace(trace_iscsi_dbg_eh, \ + &(_session)->cls_session->dev, \ + "%s " dbg_fmt, __func__, ##arg);\ } while (0); inline void iscsi_conn_queue_work(struct iscsi_conn *conn) diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c index 63a1d69ff515..d641a72ae033 100644 --- a/drivers/scsi/libiscsi_tcp.c +++ b/drivers/scsi/libiscsi_tcp.c @@ -43,6 +43,7 @@ #include #include #include +#include #include "iscsi_tcp.h" @@ -65,6 +66,9 @@ MODULE_PARM_DESC(debug_libiscsi_tcp, "Turn on debugging for libiscsi_tcp " iscsi_conn_printk(KERN_INFO, _conn, \ "%s " dbg_fmt, \ __func__, ##arg); \ + iscsi_dbg_trace(trace_iscsi_dbg_tcp,\ + &(_conn)->cls_conn->dev,\ + "%s " dbg_fmt, __func__, ##arg);\ } while (0); static int iscsi_tcp_hdr_recv_done(struct iscsi_tcp_conn *tcp_conn, diff -
Re: [PATCH v7] scsi: Add hwmon support for SMART temperature sensors
Hi Linus! > This driver does not block any simultaneous use of other SMART > userspace tools, it's a both/and approach, not either/or. The problem with all this is that the storage topology is largely undiscoverable for monitoring purposes. We can use heuristics, but in many cases there is no reliable way to find out that there is an ATA device behind member #3 of a USB-attached RAID controller's virtual disk #5. So while I am sympathetic to providing this type of information inside the kernel, the complexity of getting it right is mindboggling. Which is why it currently lives in smartmontools in userland. And why even the latter defers several of the topology decisions to the administrator. You could then argue that the kernel should only provide sensors for a trivial subset of configurations such as direct-attached ATA/SAS/USB devices that provide sufficient heuristics to ensure we don't accidentally send commands down that may wedge the device. I.e. repicate smartmontools' heuristics inside the kernel. That's a valid position but I remain unconvinced that it's worth it. Do you have specific user cases other than this particular RAID box without enclosure sensors? (It's also worth noting that HDD temperature sensors are notoriously unreliable). And finally, from an implementation perspective, both James and Doug pointed you to SAT and the SCSI Temperature Log Page. libata is our SAT. And thus the S.M.A.R.T. bits should be located in a libsmart library that libata and USB can use to fill out the SCSI Temperature Log Page. The hwmon-facing code would then use that log page instead of dissecting S.M.A.R.T. information directly. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v2] codafs: Fix build using bare-metal toolchain
On Wed, Nov 21, 2018 at 06:41:13PM +0200, Andy Shevchenko wrote: > I'm not sure how you managed to miss people in this list (perhaps by > default you have suppress all Cc in your Git configuration), but I > guess we may gently ask Christoph to apply this in case Jan will not > appear. You have got to give me a little more than 10 minutes to respond before assuming that I would not appear... I don't think I've ignored any previous emails on this subject and the only issues has been some people not receiving my responses for unknown reasons (agressive spam filter?). I have no problem with this patch, have it sitting with some other non-urgent patches and in case it doesn't appear upstream it should piggyback with whatever I have to send. I still don't know why the bare-metal toolchain couldn't just add a -D__linux__. I understand that this define is expected to be always present while compiling kernel headers so that there is no good reason to even bother testing for it, which is why I have no issue with the patch. But it seems it would make your life a lot easier if you had it defined. Jan
Re: [PATCH v2] codafs: Fix build using bare-metal toolchain
On Wed, Nov 21, 2018 at 8:10 PM Jan Harkes wrote: > > On Wed, Nov 21, 2018 at 06:41:13PM +0200, Andy Shevchenko wrote: > > I'm not sure how you managed to miss people in this list (perhaps by > > default you have suppress all Cc in your Git configuration), but I > > guess we may gently ask Christoph to apply this in case Jan will not > > appear. > > You have got to give me a little more than 10 minutes to respond before > assuming that I would not appear... I don't think I've ignored any > previous emails on this subject and the only issues has been some people > not receiving my responses for unknown reasons (agressive spam filter?). > > I have no problem with this patch, have it sitting with some other > non-urgent patches and in case it doesn't appear upstream it should > piggyback with whatever I have to send. > Thanks, Jan, really appreciate it. We need this patch to fix our tests with allmodconfig configuration (in Linaro CI loops). > I still don't know why the bare-metal toolchain couldn't just add a > -D__linux__. I understand that this define is expected to be always > present while compiling kernel headers so that there is no good reason > to even bother testing for it, which is why I have no issue with the > patch. But it seems it would make your life a lot easier if you had it > defined. > As I understand it, from toolchain's point of view, if __linux__ is defined then it means that the program is being built *for* Linux (i.e. we can use Linux specific features, ABI, like syscalls). Checking this definition can make sense in uapi headers, but in kernel code we shouldn't use it (as kernel is baremetal program and not compiled for some OS). I presume that's why __linux__ is not defined in bare-metal toolchains (as those don't provide Linux specific features, libc, etc). > Jan >
Re: [PATCH v2] codafs: Fix build using bare-metal toolchain
+ Jan Harkes back to "To:" list, slipped away somehow... On Wed, Nov 21, 2018 at 9:36 PM Sam Protsenko wrote: > > On Wed, Nov 21, 2018 at 8:10 PM Jan Harkes wrote: > > > > On Wed, Nov 21, 2018 at 06:41:13PM +0200, Andy Shevchenko wrote: > > > I'm not sure how you managed to miss people in this list (perhaps by > > > default you have suppress all Cc in your Git configuration), but I > > > guess we may gently ask Christoph to apply this in case Jan will not > > > appear. > > > > You have got to give me a little more than 10 minutes to respond before > > assuming that I would not appear... I don't think I've ignored any > > previous emails on this subject and the only issues has been some people > > not receiving my responses for unknown reasons (agressive spam filter?). > > > > I have no problem with this patch, have it sitting with some other > > non-urgent patches and in case it doesn't appear upstream it should > > piggyback with whatever I have to send. > > > > Thanks, Jan, really appreciate it. We need this patch to fix our tests > with allmodconfig configuration (in Linaro CI loops). > > > I still don't know why the bare-metal toolchain couldn't just add a > > -D__linux__. I understand that this define is expected to be always > > present while compiling kernel headers so that there is no good reason > > to even bother testing for it, which is why I have no issue with the > > patch. But it seems it would make your life a lot easier if you had it > > defined. > > > > As I understand it, from toolchain's point of view, if __linux__ is > defined then it means that the program is being built *for* Linux > (i.e. we can use Linux specific features, ABI, like syscalls). > Checking this definition can make sense in uapi headers, but in kernel > code we shouldn't use it (as kernel is baremetal program and not > compiled for some OS). I presume that's why __linux__ is not defined > in bare-metal toolchains (as those don't provide Linux specific > features, libc, etc). > > > Jan > >
Re: [PATCH V2] SCSI: fix queue cleanup race before queue initialization is done
On 11/14/18 8:20 AM, Jens Axboe wrote:
> On 11/14/18 1:25 AM, Ming Lei wrote:
>> c2856ae2f315d ("blk-mq: quiesce queue before freeing queue") has
>> already fixed this race, however the implied synchronize_rcu()
>> in blk_mq_quiesce_queue() can slow down LUN probe a lot, so caused
>> performance regression.
>>
>> Then 1311326cf4755c7 ("blk-mq: avoid to synchronize rcu inside
>> blk_cleanup_queue()")
>> tried to quiesce queue for avoiding unnecessary synchronize_rcu()
>> only when queue initialization is done, because it is usual to see
>> lots of inexistent LUNs which need to be probed.
>>
>> However, turns out it isn't safe to quiesce queue only when queue
>> initialization is done. Because when one SCSI command is completed,
>> the user of sending command can be waken up immediately, then the
>> scsi device may be removed, meantime the run queue in scsi_end_request()
>> is still in-progress, so kernel panic can be caused.
>>
>> In Red Hat QE lab, there are several reports about this kind of kernel
>> panic triggered during kernel booting.
>>
>> This patch tries to address the issue by grabing one queue usage
>> counter during freeing one request and the following run queue.
>
> Thanks applied, this bug was elusive but ever present in recent
> testing that we did internally, it's been a huge pain in the butt.
> The symptoms were usually a crash in blk_mq_get_driver_tag() with
> hctx->tags == NULL, or a crash inside deadline request insert off
> requeue.
I'm still hitting some weird crashes even with this applied, like
this one:
BUG: unable to handle kernel NULL pointer dereference at 0148
PGD 0 P4D 0.
Oops: [#1] SMP PTI
CPU: 37 PID: 763 Comm: kworker/37:1H Not tainted
4.20.0-rc3-00649-ge64d9a554a91-dirty #14
Hardware name: Wiwynn Leopard-Orv2/Leopard-DDR BW, BIOS LBM08 03/03/2017
Workqueue: kblockd blk_mq_run_work_fn
RIP: 0010:blk_mq_get_driver_tag+0x81/0x120
Code: 24 10 48 89 7c 24 20 74 21 83 fa ff 0f 95 c0 48 8b 4c 24 28 65 48 33 0c
25 28 00 00 00 0f 85 96 00 00 00 48 83 c4 30 5b 5d c3 <48> 8b 87 48 01 00 00 8b
40 04 39 43 20 72 37 f6 87 b0 00 00 00 02
RSP: 0018:c90004aabd30 EFLAGS: 00010246
RAX: 0003 RBX: 888465ea1300 RCX: c90004aabde8
RDX: RSI: c90004aabde8 RDI:
RBP: R08: 888465ea1348 R09:
R10: 1000 R11: R12: 888465ea1300
R13: R14: 888465ea1348 R15: 888465d1
FS: () GS:88846f9c() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 0148 CR3: 0220a003 CR4: 003606e0
DR0: DR1: DR2:
DR3: DR6: fffe0ff0 DR7: 0400
Call Trace:
blk_mq_dispatch_rq_list+0xec/0x480
? elv_rb_del+0x11/0x30
blk_mq_do_dispatch_sched+0x6e/0xf0
blk_mq_sched_dispatch_requests+0xfa/0x170
__blk_mq_run_hw_queue+0x5f/0xe0
process_one_work+0x154/0x350
worker_thread+0x46/0x3c0
kthread+0xf5/0x130
? process_one_work+0x350/0x350
? kthread_destroy_worker+0x50/0x50
ret_from_fork+0x1f/0x30
Modules linked in: sb_edac x86_pkg_temp_thermal intel_powerclamp coretemp
kvm_intel kvm switchtec irqbypass iTCO_wdt iTCO_vendor_support efivars
cdc_ether usbnet mii cdc_acm i2c_i801 lpc_ich mfd_core ipmi_si ipmi_devintf
ipmi_msghandler acpi_cpufreq button sch_fq_codel nfsd nfs_acl lockd grace
auth_rpcgss oid_registry sunrpc nvme nvme_core fuse sg loop efivarfs autofs4
CR2: 0148
---[ end trace 340a1fb996df1b9b ]---
RIP: 0010:blk_mq_get_driver_tag+0x81/0x120
Code: 24 10 48 89 7c 24 20 74 21 83 fa ff 0f 95 c0 48 8b 4c 24 28 65 48 33 0c
25 28 00 00 00 0f 85 96 00 00 00 48 83 c4 30 5b 5d c3 <48> 8b 87 48 01 00 00 8b
40 04 39 43 20
Re: [PATCH V2] SCSI: fix queue cleanup race before queue initialization is done
On Wed, Nov 21, 2018 at 02:47:35PM -0700, Jens Axboe wrote: > > Thanks applied, this bug was elusive but ever present in recent > > testing that we did internally, it's been a huge pain in the butt. > > The symptoms were usually a crash in blk_mq_get_driver_tag() with > > hctx->tags == NULL, or a crash inside deadline request insert off > > requeue. > > I'm still hitting some weird crashes even with this applied, like > this one: FYI, there are a number of Ubuntu users running 4.19, 4.19.1, and 4.19.2 which have been reporting file system corruption problems. They have a fix of configurations, but one of the things which is seem to be a common factor is they all have CONFIG_SCSI_MQ_DEFAULT disabled. (Which also happens to be how I happen to be running my laptop, and I've noticed no problems.) https://bugzilla.kernel.org/show_bug.cgi?id=201685 One user in particular reported that 4.19 worked fine, and 4.19.1 had fs corruptions (and there are no storage-related changes between 4.19 and 4.19.1) --- but the one thing those two kernels had in common was his 4.19 build had SCSI_MQ_DEFAULT disabled, and his 4.19.1 build did *not* have SCSI_MQ_DEFAULT enabled. This same user tried 4.19.3, and after two hours of heavy I/O, he's not seen a repeat, and interestingly, 4.19.3 has the backport mentioned on this thread. The weird thing is that it looked like the problem that was fixed by this commit would only show up at queue setup and teardown time. Is that correct? Is it possible that the bug fixed here would manifest as data corruptions on disk? Or would only manifest as kernel BUG_ON's and/or crashes? One more thing. I tried building a 4.20-rc2 based kernel with CONFIG_SCSI_MQ_DEFAULT=y, and I tried running gce-xfstests (which uses virtio-scsi) and I saw no failures. So I don't have a clean repro of Kernel Bugzilla #201685, and at the moment I'm too chicken to enable CONFIG_SCSI_MQ_DEFAULT on my primary development laptop... Any thoughts/suggestions appreciated. - Ted
Re: [PATCH v2] codafs: Fix build using bare-metal toolchain
That actually makes a lot of sense. Jan On November 21, 2018 2:39:03 PM EST, Sam Protsenko wrote: >+ Jan Harkes back to "To:" list, slipped away somehow... > >On Wed, Nov 21, 2018 at 9:36 PM Sam Protsenko > wrote: >> >> On Wed, Nov 21, 2018 at 8:10 PM Jan Harkes >wrote: >> > >> > On Wed, Nov 21, 2018 at 06:41:13PM +0200, Andy Shevchenko wrote: >> > > I'm not sure how you managed to miss people in this list (perhaps >by >> > > default you have suppress all Cc in your Git configuration), but >I >> > > guess we may gently ask Christoph to apply this in case Jan will >not >> > > appear. >> > >> > You have got to give me a little more than 10 minutes to respond >before >> > assuming that I would not appear... I don't think I've ignored any >> > previous emails on this subject and the only issues has been some >people >> > not receiving my responses for unknown reasons (agressive spam >filter?). >> > >> > I have no problem with this patch, have it sitting with some other >> > non-urgent patches and in case it doesn't appear upstream it should >> > piggyback with whatever I have to send. >> > >> >> Thanks, Jan, really appreciate it. We need this patch to fix our >tests >> with allmodconfig configuration (in Linaro CI loops). >> >> > I still don't know why the bare-metal toolchain couldn't just add a >> > -D__linux__. I understand that this define is expected to be >always >> > present while compiling kernel headers so that there is no good >reason >> > to even bother testing for it, which is why I have no issue with >the >> > patch. But it seems it would make your life a lot easier if you had >it >> > defined. >> > >> >> As I understand it, from toolchain's point of view, if __linux__ is >> defined then it means that the program is being built *for* Linux >> (i.e. we can use Linux specific features, ABI, like syscalls). >> Checking this definition can make sense in uapi headers, but in >kernel >> code we shouldn't use it (as kernel is baremetal program and not >> compiled for some OS). I presume that's why __linux__ is not defined >> in bare-metal toolchains (as those don't provide Linux specific >> features, libc, etc). >> >> > Jan >> >
Re: [PATCH V2] SCSI: fix queue cleanup race before queue initialization is done
On Wed, Nov 21, 2018 at 02:47:35PM -0700, Jens Axboe wrote:
> On 11/14/18 8:20 AM, Jens Axboe wrote:
> > On 11/14/18 1:25 AM, Ming Lei wrote:
> >> c2856ae2f315d ("blk-mq: quiesce queue before freeing queue") has
> >> already fixed this race, however the implied synchronize_rcu()
> >> in blk_mq_quiesce_queue() can slow down LUN probe a lot, so caused
> >> performance regression.
> >>
> >> Then 1311326cf4755c7 ("blk-mq: avoid to synchronize rcu inside
> >> blk_cleanup_queue()")
> >> tried to quiesce queue for avoiding unnecessary synchronize_rcu()
> >> only when queue initialization is done, because it is usual to see
> >> lots of inexistent LUNs which need to be probed.
> >>
> >> However, turns out it isn't safe to quiesce queue only when queue
> >> initialization is done. Because when one SCSI command is completed,
> >> the user of sending command can be waken up immediately, then the
> >> scsi device may be removed, meantime the run queue in scsi_end_request()
> >> is still in-progress, so kernel panic can be caused.
> >>
> >> In Red Hat QE lab, there are several reports about this kind of kernel
> >> panic triggered during kernel booting.
> >>
> >> This patch tries to address the issue by grabing one queue usage
> >> counter during freeing one request and the following run queue.
> >
> > Thanks applied, this bug was elusive but ever present in recent
> > testing that we did internally, it's been a huge pain in the butt.
> > The symptoms were usually a crash in blk_mq_get_driver_tag() with
> > hctx->tags == NULL, or a crash inside deadline request insert off
> > requeue.
>
> I'm still hitting some weird crashes even with this applied, like
> this one:
>
> BUG: unable to handle kernel NULL pointer dereference at 0148
>
> PGD 0 P4D 0.
>
> Oops: [#1] SMP PTI
>
> CPU: 37 PID: 763 Comm: kworker/37:1H Not tainted
> 4.20.0-rc3-00649-ge64d9a554a91-dirty #14
> Hardware name: Wiwynn Leopard-Orv2/Leopard-DDR BW, BIOS LBM08 03/03/2017
>
> Workqueue: kblockd blk_mq_run_work_fn
>
> RIP: 0010:blk_mq_get_driver_tag+0x81/0x120
>
> Code: 24 10 48 89 7c 24 20 74 21 83 fa ff 0f 95 c0 48 8b 4c 24 28 65 48 33
> 0c 25 28 00 00 00 0f 85 96 00 00 00 48 83 c4 30 5b 5d c3 <48> 8b 87 48 01 00
> 00 8b 40 04 39 43 20 72 37 f6 87 b0 00 00 00 02
> RSP: 0018:c90004aabd30 EFLAGS: 00010246
>
> RAX: 0003 RBX: 888465ea1300 RCX: c90004aabde8
>
> RDX: RSI: c90004aabde8 RDI:
>
> RBP: R08: 888465ea1348 R09:
>
> R10: 1000 R11: R12: 888465ea1300
>
> R13: R14: 888465ea1348 R15: 888465d1
>
> FS: () GS:88846f9c() knlGS:
>
> CS: 0010 DS: ES: CR0: 80050033
>
> CR2: 0148 CR3: 0220a003 CR4: 003606e0
>
> DR0: DR1: DR2:
>
> DR3: DR6: fffe0ff0 DR7: 0400
>
> Call Trace:
>
> blk_mq_dispatch_rq_list+0xec/0x480
>
> ? elv_rb_del+0x11/0x30
>
> blk_mq_do_dispatch_sched+0x6e/0xf0
>
> blk_mq_sched_dispatch_requests+0xfa/0x170
>
> __blk_mq_run_hw_queue+0x5f/0xe0
>
> process_one_work+0x154/0x350
>
> worker_thread+0x46/0x3c0
>
> kthread+0xf5/0x130
>
> ? process_one_work+0x350/0x350
>
> ? kthread_destroy_worker+0x50/0x50
>
> ret_from_fork+0x1f/0x30
>
> Modules linked in: sb_edac x86_pkg_temp_thermal intel_powerclamp coretemp
> kvm_intel kvm switchtec irqbypass iTCO_wdt iTCO_vendor_support efivars
> cdc_ether usbnet mii cdc_acm i2c_i801 lpc_ich mfd_core ipmi_si ipmi_devintf
> ipmi_msghandler acpi_cpufreq button sch_fq_codel nfsd nfs_acl lockd grace
> auth_rpcgss oid_registry sunrpc nvme nvme_core fuse sg loop efivarfs autofs4
> CR2: 0148
>
> ---[ end trace 340a1fb996df1b9b ]---
Re: [PATCH V2] SCSI: fix queue cleanup race before queue initialization is done
On 11/21/18 6:00 PM, Ming Lei wrote:
> On Wed, Nov 21, 2018 at 02:47:35PM -0700, Jens Axboe wrote:
>> On 11/14/18 8:20 AM, Jens Axboe wrote:
>>> On 11/14/18 1:25 AM, Ming Lei wrote:
c2856ae2f315d ("blk-mq: quiesce queue before freeing queue") has
already fixed this race, however the implied synchronize_rcu()
in blk_mq_quiesce_queue() can slow down LUN probe a lot, so caused
performance regression.
Then 1311326cf4755c7 ("blk-mq: avoid to synchronize rcu inside
blk_cleanup_queue()")
tried to quiesce queue for avoiding unnecessary synchronize_rcu()
only when queue initialization is done, because it is usual to see
lots of inexistent LUNs which need to be probed.
However, turns out it isn't safe to quiesce queue only when queue
initialization is done. Because when one SCSI command is completed,
the user of sending command can be waken up immediately, then the
scsi device may be removed, meantime the run queue in scsi_end_request()
is still in-progress, so kernel panic can be caused.
In Red Hat QE lab, there are several reports about this kind of kernel
panic triggered during kernel booting.
This patch tries to address the issue by grabing one queue usage
counter during freeing one request and the following run queue.
>>>
>>> Thanks applied, this bug was elusive but ever present in recent
>>> testing that we did internally, it's been a huge pain in the butt.
>>> The symptoms were usually a crash in blk_mq_get_driver_tag() with
>>> hctx->tags == NULL, or a crash inside deadline request insert off
>>> requeue.
>>
>> I'm still hitting some weird crashes even with this applied, like
>> this one:
>>
>> BUG: unable to handle kernel NULL pointer dereference at 0148
>>
>> PGD 0 P4D 0.
>>
>> Oops: [#1] SMP PTI
>>
>> CPU: 37 PID: 763 Comm: kworker/37:1H Not tainted
>> 4.20.0-rc3-00649-ge64d9a554a91-dirty #14
>> Hardware name: Wiwynn Leopard-Orv2/Leopard-DDR BW, BIOS LBM08 03/03/2017
>>
>> Workqueue: kblockd blk_mq_run_work_fn
>>
>> RIP: 0010:blk_mq_get_driver_tag+0x81/0x120
>>
>> Code: 24 10 48 89 7c 24 20 74 21 83 fa ff 0f 95 c0 48 8b 4c 24 28 65 48 33
>> 0c 25 28 00 00 00 0f 85 96 00 00 00 48 83 c4 30 5b 5d c3 <48> 8b 87 48 01 00
>> 00 8b 40 04 39 43 20 72 37 f6 87 b0 00 00 00 02
>> RSP: 0018:c90004aabd30 EFLAGS: 00010246
>>
>> RAX: 0003 RBX: 888465ea1300 RCX: c90004aabde8
>>
>> RDX: RSI: c90004aabde8 RDI:
>>
>> RBP: R08: 888465ea1348 R09:
>>
>> R10: 1000 R11: R12: 888465ea1300
>>
>> R13: R14: 888465ea1348 R15: 888465d1
>>
>> FS: () GS:88846f9c()
>> knlGS:
>> CS: 0010 DS: ES: CR0: 80050033
>>
>> CR2: 0148 CR3: 0220a003 CR4: 003606e0
>>
>> DR0: DR1: DR2:
>>
>> DR3: DR6: fffe0ff0 DR7: 0400
>>
>> Call Trace:
>>
>> blk_mq_dispatch_rq_list+0xec/0x480
>>
>> ? elv_rb_del+0x11/0x30
>>
>> blk_mq_do_dispatch_sched+0x6e/0xf0
>>
>> blk_mq_sched_dispatch_requests+0xfa/0x170
>>
>> __blk_mq_run_hw_queue+0x5f/0xe0
>>
>> process_one_work+0x154/0x350
>>
>> worker_thread+0x46/0x3c0
>>
>> kthread+0xf5/0x130
>>
>> ? process_one_work+0x350/0x350
>>
>> ? kthread_destroy_worker+0x50/0x50
>>
>> ret_from_fork+0x1f/0x30
>>
>> Modules linked in: sb_edac x86_pkg_temp_thermal intel_powerclamp coretemp
>> kvm_intel kvm switchtec irqbypass iTCO_wdt iTCO_vendor_support efivars
>> cdc_ether usbnet mii cdc_acm i2c_i801 lpc_ich mfd_core ipmi_si ipmi_devintf
>> ipmi_msghandler acpi_cpufreq button sch_fq_codel nfsd nfs_acl lockd grace
>> auth_rpcgss oid_registry sunrpc nvme nvme_core fuse sg loop efivarfs autofs4
>> CR2:
Re: [PATCH V2] SCSI: fix queue cleanup race before queue initialization is done
On Wed, Nov 21, 2018 at 06:42:51PM -0700, Jens Axboe wrote:
> On 11/21/18 6:00 PM, Ming Lei wrote:
> > On Wed, Nov 21, 2018 at 02:47:35PM -0700, Jens Axboe wrote:
> >> On 11/14/18 8:20 AM, Jens Axboe wrote:
> >>> On 11/14/18 1:25 AM, Ming Lei wrote:
> c2856ae2f315d ("blk-mq: quiesce queue before freeing queue") has
> already fixed this race, however the implied synchronize_rcu()
> in blk_mq_quiesce_queue() can slow down LUN probe a lot, so caused
> performance regression.
>
> Then 1311326cf4755c7 ("blk-mq: avoid to synchronize rcu inside
> blk_cleanup_queue()")
> tried to quiesce queue for avoiding unnecessary synchronize_rcu()
> only when queue initialization is done, because it is usual to see
> lots of inexistent LUNs which need to be probed.
>
> However, turns out it isn't safe to quiesce queue only when queue
> initialization is done. Because when one SCSI command is completed,
> the user of sending command can be waken up immediately, then the
> scsi device may be removed, meantime the run queue in scsi_end_request()
> is still in-progress, so kernel panic can be caused.
>
> In Red Hat QE lab, there are several reports about this kind of kernel
> panic triggered during kernel booting.
>
> This patch tries to address the issue by grabing one queue usage
> counter during freeing one request and the following run queue.
> >>>
> >>> Thanks applied, this bug was elusive but ever present in recent
> >>> testing that we did internally, it's been a huge pain in the butt.
> >>> The symptoms were usually a crash in blk_mq_get_driver_tag() with
> >>> hctx->tags == NULL, or a crash inside deadline request insert off
> >>> requeue.
> >>
> >> I'm still hitting some weird crashes even with this applied, like
> >> this one:
> >>
> >> BUG: unable to handle kernel NULL pointer dereference at 0148
> >>
> >> PGD 0 P4D 0.
> >>
> >> Oops: [#1] SMP PTI
> >>
> >> CPU: 37 PID: 763 Comm: kworker/37:1H Not tainted
> >> 4.20.0-rc3-00649-ge64d9a554a91-dirty #14
> >> Hardware name: Wiwynn Leopard-Orv2/Leopard-DDR BW, BIOS LBM08
> >> 03/03/2017
> >> Workqueue: kblockd blk_mq_run_work_fn
> >>
> >> RIP: 0010:blk_mq_get_driver_tag+0x81/0x120
> >>
> >> Code: 24 10 48 89 7c 24 20 74 21 83 fa ff 0f 95 c0 48 8b 4c 24 28 65 48
> >> 33 0c 25 28 00 00 00 0f 85 96 00 00 00 48 83 c4 30 5b 5d c3 <48> 8b 87 48
> >> 01 00 00 8b 40 04 39 43 20 72 37 f6 87 b0 00 00 00 02
> >> RSP: 0018:c90004aabd30 EFLAGS: 00010246
> >>
> >> RAX: 0003 RBX: 888465ea1300 RCX: c90004aabde8
> >>
> >> RDX: RSI: c90004aabde8 RDI:
> >>
> >> RBP: R08: 888465ea1348 R09:
> >>
> >> R10: 1000 R11: R12: 888465ea1300
> >>
> >> R13: R14: 888465ea1348 R15: 888465d1
> >>
> >> FS: () GS:88846f9c()
> >> knlGS:
> >> CS: 0010 DS: ES: CR0: 80050033
> >>
> >> CR2: 0148 CR3: 0220a003 CR4: 003606e0
> >>
> >> DR0: DR1: DR2:
> >>
> >> DR3: DR6: fffe0ff0 DR7: 0400
> >>
> >> Call Trace:
> >>
> >> blk_mq_dispatch_rq_list+0xec/0x480
> >>
> >> ? elv_rb_del+0x11/0x30
> >>
> >> blk_mq_do_dispatch_sched+0x6e/0xf0
> >>
> >> blk_mq_sched_dispatch_requests+0xfa/0x170
> >>
> >> __blk_mq_run_hw_queue+0x5f/0xe0
> >>
> >> process_one_work+0x154/0x350
> >>
> >> worker_thread+0x46/0x3c0
> >>
> >> kthread+0xf5/0x130
> >>
> >> ? process_one_work+0x350/0x350
> >>
> >> ? kthread_destroy_worker+0x50/0x50
> >>
> >> ret_from_fork+0x1f/0x30
> >>
> >> Modules linked in: sb_edac x86_pkg_temp_thermal intel_powerclamp coretemp
> >> kvm_intel kvm switchtec irqbypass iTCO_wdt iTCO_vendor_support ef
Re: [PATCH V2] SCSI: fix queue cleanup race before queue initialization is done
On 11/21/18 7:00 PM, Ming Lei wrote:
> On Wed, Nov 21, 2018 at 06:42:51PM -0700, Jens Axboe wrote:
>> On 11/21/18 6:00 PM, Ming Lei wrote:
>>> On Wed, Nov 21, 2018 at 02:47:35PM -0700, Jens Axboe wrote:
On 11/14/18 8:20 AM, Jens Axboe wrote:
> On 11/14/18 1:25 AM, Ming Lei wrote:
>> c2856ae2f315d ("blk-mq: quiesce queue before freeing queue") has
>> already fixed this race, however the implied synchronize_rcu()
>> in blk_mq_quiesce_queue() can slow down LUN probe a lot, so caused
>> performance regression.
>>
>> Then 1311326cf4755c7 ("blk-mq: avoid to synchronize rcu inside
>> blk_cleanup_queue()")
>> tried to quiesce queue for avoiding unnecessary synchronize_rcu()
>> only when queue initialization is done, because it is usual to see
>> lots of inexistent LUNs which need to be probed.
>>
>> However, turns out it isn't safe to quiesce queue only when queue
>> initialization is done. Because when one SCSI command is completed,
>> the user of sending command can be waken up immediately, then the
>> scsi device may be removed, meantime the run queue in scsi_end_request()
>> is still in-progress, so kernel panic can be caused.
>>
>> In Red Hat QE lab, there are several reports about this kind of kernel
>> panic triggered during kernel booting.
>>
>> This patch tries to address the issue by grabing one queue usage
>> counter during freeing one request and the following run queue.
>
> Thanks applied, this bug was elusive but ever present in recent
> testing that we did internally, it's been a huge pain in the butt.
> The symptoms were usually a crash in blk_mq_get_driver_tag() with
> hctx->tags == NULL, or a crash inside deadline request insert off
> requeue.
I'm still hitting some weird crashes even with this applied, like
this one:
BUG: unable to handle kernel NULL pointer dereference at 0148
PGD 0 P4D 0.
Oops: [#1] SMP PTI
CPU: 37 PID: 763 Comm: kworker/37:1H Not tainted
4.20.0-rc3-00649-ge64d9a554a91-dirty #14
Hardware name: Wiwynn Leopard-Orv2/Leopard-DDR BW, BIOS LBM08
03/03/2017
Workqueue: kblockd blk_mq_run_work_fn
RIP: 0010:blk_mq_get_driver_tag+0x81/0x120
Code: 24 10 48 89 7c 24 20 74 21 83 fa ff 0f 95 c0 48 8b 4c 24 28 65 48
33 0c 25 28 00 00 00 0f 85 96 00 00 00 48 83 c4 30 5b 5d c3 <48> 8b 87 48
01 00 00 8b 40 04 39 43 20 72 37 f6 87 b0 00 00 00 02
RSP: 0018:c90004aabd30 EFLAGS: 00010246
RAX: 0003 RBX: 888465ea1300 RCX: c90004aabde8
RDX: RSI: c90004aabde8 RDI:
RBP: R08: 888465ea1348 R09:
R10: 1000 R11: R12: 888465ea1300
R13: R14: 888465ea1348 R15: 888465d1
FS: () GS:88846f9c()
knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 0148 CR3: 0220a003 CR4: 003606e0
DR0: DR1: DR2:
DR3: DR6: fffe0ff0 DR7: 0400
Call Trace:
blk_mq_dispatch_rq_list+0xec/0x480
? elv_rb_del+0x11/0x30
blk_mq_do_dispatch_sched+0x6e/0xf0
blk_mq_sched_dispatch_requests+0xfa/0x170
__blk_mq_run_hw_queue+0x5f/0xe0
process_one_work+0x154/0x350
worker_thread+0x46/0x3c0
kthread+0xf5/0x130
? process_one_work+0x350/0x350
? kthread_destroy_worker+0x50/0x50
ret_from_fork+0x1f/0x30
Modules linked in: sb_edac x86_pkg_temp_thermal intel_powerclamp coretemp
kvm_intel kvm switchtec i
Re: [PATCH net-next] cxgb4: use new fw interface to get the VIN and smt index
On Wed, Nov 21, 2018 at 01:40:24PM +0530, Ganesh Goudar wrote: > From: Santosh Rastapur > > If the fw supports returning VIN/VIVLD in FW_VI_CMD save it > in port_info structure else retrieve these from viid and save > them in port_info structure. Do the same for smt_idx from > FW_VI_MAC_CMD > > Signed-off-by: Santosh Rastapur > Signed-off-by: Ganesh Goudar > drivers/crypto/chelsio/chtls/chtls_cm.c | 3 +- > drivers/infiniband/hw/cxgb4/cm.c| 6 +-- > drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 12 - > drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 58 > - > drivers/net/ethernet/chelsio/cxgb4/l2t.c| 13 +++--- > drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 46 ++-- > drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h | 20 + > drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 3 +- > drivers/target/iscsi/cxgbit/cxgbit_cm.c | 8 ++-- > 9 files changed, 114 insertions(+), 55 deletions(-) Applied to for-next, but please try to write better commit messages in future, explain what benifit your change is bringing. Jason
Re: [PATCH V2] SCSI: fix queue cleanup race before queue initialization is done
On Wed, Nov 21, 2018 at 07:14:35PM -0700, Jens Axboe wrote:
> On 11/21/18 7:00 PM, Ming Lei wrote:
> > On Wed, Nov 21, 2018 at 06:42:51PM -0700, Jens Axboe wrote:
> >> On 11/21/18 6:00 PM, Ming Lei wrote:
> >>> On Wed, Nov 21, 2018 at 02:47:35PM -0700, Jens Axboe wrote:
> On 11/14/18 8:20 AM, Jens Axboe wrote:
> > On 11/14/18 1:25 AM, Ming Lei wrote:
> >> c2856ae2f315d ("blk-mq: quiesce queue before freeing queue") has
> >> already fixed this race, however the implied synchronize_rcu()
> >> in blk_mq_quiesce_queue() can slow down LUN probe a lot, so caused
> >> performance regression.
> >>
> >> Then 1311326cf4755c7 ("blk-mq: avoid to synchronize rcu inside
> >> blk_cleanup_queue()")
> >> tried to quiesce queue for avoiding unnecessary synchronize_rcu()
> >> only when queue initialization is done, because it is usual to see
> >> lots of inexistent LUNs which need to be probed.
> >>
> >> However, turns out it isn't safe to quiesce queue only when queue
> >> initialization is done. Because when one SCSI command is completed,
> >> the user of sending command can be waken up immediately, then the
> >> scsi device may be removed, meantime the run queue in
> >> scsi_end_request()
> >> is still in-progress, so kernel panic can be caused.
> >>
> >> In Red Hat QE lab, there are several reports about this kind of kernel
> >> panic triggered during kernel booting.
> >>
> >> This patch tries to address the issue by grabing one queue usage
> >> counter during freeing one request and the following run queue.
> >
> > Thanks applied, this bug was elusive but ever present in recent
> > testing that we did internally, it's been a huge pain in the butt.
> > The symptoms were usually a crash in blk_mq_get_driver_tag() with
> > hctx->tags == NULL, or a crash inside deadline request insert off
> > requeue.
>
> I'm still hitting some weird crashes even with this applied, like
> this one:
>
> BUG: unable to handle kernel NULL pointer dereference at
> 0148
> PGD 0 P4D 0.
>
> Oops: [#1] SMP PTI
>
> CPU: 37 PID: 763 Comm: kworker/37:1H Not tainted
> 4.20.0-rc3-00649-ge64d9a554a91-dirty #14
> Hardware name: Wiwynn Leopard-Orv2/Leopard-DDR BW, BIOS LBM08
> 03/03/2017
> Workqueue: kblockd blk_mq_run_work_fn
>
> RIP: 0010:blk_mq_get_driver_tag+0x81/0x120
>
> Code: 24 10 48 89 7c 24 20 74 21 83 fa ff 0f 95 c0 48 8b 4c 24 28 65 48
> 33 0c 25 28 00 00 00 0f 85 96 00 00 00 48 83 c4 30 5b 5d c3 <48> 8b 87
> 48 01 00 00 8b 40 04 39 43 20 72 37 f6 87 b0 00 00 00 02
> RSP: 0018:c90004aabd30 EFLAGS: 00010246
>
> RAX: 0003 RBX: 888465ea1300 RCX: c90004aabde8
>
> RDX: RSI: c90004aabde8 RDI:
>
> RBP: R08: 888465ea1348 R09:
>
> R10: 1000 R11: R12: 888465ea1300
>
> R13: R14: 888465ea1348 R15: 888465d1
>
> FS: () GS:88846f9c()
> knlGS:
> CS: 0010 DS: ES: CR0: 80050033
>
> CR2: 0148 CR3: 0220a003 CR4: 003606e0
>
> DR0: DR1: DR2:
>
> DR3: DR6: fffe0ff0 DR7: 0400
>
> Call Trace:
>
> blk_mq_dispatch_rq_list+0xec/0x480
>
> ? elv_rb_del+0x11/0x30
>
> blk_mq_do_dispatch_sched+0x6e/0xf0
>
> blk_mq_sched_dispatch_requests+0xfa/0x170
>
> __blk_mq_run_hw_queue+0x5f/0xe0
>
> process_one_work+0x154/0x350
>
> worker_thread+0x46/0x3c0
>
> kthread+0xf5/0x130
>
> ? process_one_work+0x350/0x350
>
> ? kthread_destroy_work
Re: [PATCH] scsi/aic94xx/aic94xx_hwi.c: Use dma_pool_zalloc
Souptick, > Replaced dma_pool_alloc + memset with dma_pool_zalloc Applied to 4.21/scsi-queue, thanks. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] cxgb4i: fix thermal configuration dependencies
Arnd, > I fixed a bug by adding a dependency in the network driver, but that > fix caused a related bug in the SCSI driver: Applied to 4.21/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] scsi/lpfc/lpfc_sli.c: Use dma_zalloc_coherent
Sabyasachi, > Replaced dma_alloc_coherent + memset with dma_zalloc_coherent Applied to 4.21/scsi-queue. Thanks. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH V2] SCSI: fix queue cleanup race before queue initialization is done
On Wed, Nov 21, 2018 at 05:02:13PM -0500, Theodore Y. Ts'o wrote: > On Wed, Nov 21, 2018 at 02:47:35PM -0700, Jens Axboe wrote: > > > Thanks applied, this bug was elusive but ever present in recent > > > testing that we did internally, it's been a huge pain in the butt. > > > The symptoms were usually a crash in blk_mq_get_driver_tag() with > > > hctx->tags == NULL, or a crash inside deadline request insert off > > > requeue. > > > > I'm still hitting some weird crashes even with this applied, like > > this one: > > FYI, there are a number of Ubuntu users running 4.19, 4.19.1, and > 4.19.2 which have been reporting file system corruption problems. > They have a fix of configurations, but one of the things which is seem > to be a common factor is they all have CONFIG_SCSI_MQ_DEFAULT > disabled. (Which also happens to be how I happen to be running my > laptop, and I've noticed no problems.) One correction to the above --- the people who are having the problem have CONFIG_SCSI_MQ_DEFAULT *enabled* (at least for those who reported the kernel configs --- not all of them did). I have CONFIG_SCSI_MQ_DEFAULT *disabled*, and things are running just fine on my laptop. Although that may be a red herring, since as you pointed out on the bug NVMe isn't affected by the SCSI_MQ_DEFAULT setting (sorry, I'm still used to a world where SCSI controls the whole world :-). And my laptop is an XPS 13 with an NVMe-attached 1T SSD. Fortunately I've not seen any corruption (or at least nothing visible yet). Anyway, all of this is in the bug, and I'll see if I can find a way of repro'ing corruption in a KVM or GCE crash-and-burn environment... - Ted
[GIT PULL] SCSI fixes for 4.20-rc3
Two small fixes. The qla2xxx is a regression from 4.18 and the ufs one
is a device enablement fix.
The patch is available here:
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes
The short changelog is:
Bill Kuzeja (1):
scsi: qla2xxx: Timeouts occur on surprise removal of QLogic adapter
Wei Li (1):
scsi: ufs: Fix hynix ufs bug with quirk on hi36xx SoC
And the diffstat:
drivers/scsi/qla2xxx/qla_os.c | 10 --
drivers/scsi/ufs/ufs-hisi.c | 9 +
drivers/scsi/ufs/ufs_quirks.h | 6 ++
drivers/scsi/ufs/ufshcd.c | 2 ++
4 files changed, 25 insertions(+), 2 deletions(-)
With full diff below.
James
---
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 20c85eed1a75..b658b9a5eb1e 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -1749,7 +1749,7 @@ qla2x00_loop_reset(scsi_qla_host_t *vha)
static void
__qla2x00_abort_all_cmds(struct qla_qpair *qp, int res)
{
- int cnt;
+ int cnt, status;
unsigned long flags;
srb_t *sp;
scsi_qla_host_t *vha = qp->vha;
@@ -1799,10 +1799,16 @@ __qla2x00_abort_all_cmds(struct qla_qpair *qp, int res)
if (!sp_get(sp)) {
spin_unlock_irqrestore
(qp->qp_lock_ptr,
flags);
- qla2xxx_eh_abort(
+ status = qla2xxx_eh_abort(
GET_CMD_SP(sp));
spin_lock_irqsave
(qp->qp_lock_ptr,
flags);
+ /*
+* Get rid of extra reference
caused
+* by early exit from
qla2xxx_eh_abort
+*/
+ if (status == FAST_IO_FAIL)
+
atomic_dec(&sp->ref_count);
}
}
sp->done(sp, res);
diff --git a/drivers/scsi/ufs/ufs-hisi.c b/drivers/scsi/ufs/ufs-hisi.c
index 46df707e6f2c..452e19f8fb47 100644
--- a/drivers/scsi/ufs/ufs-hisi.c
+++ b/drivers/scsi/ufs/ufs-hisi.c
@@ -20,6 +20,7 @@
#include "unipro.h"
#include "ufs-hisi.h"
#include "ufshci.h"
+#include "ufs_quirks.h"
static int ufs_hisi_check_hibern8(struct ufs_hba *hba)
{
@@ -390,6 +391,14 @@ static void ufs_hisi_set_dev_cap(struct
ufs_hisi_dev_params *hisi_param)
static void ufs_hisi_pwr_change_pre_change(struct ufs_hba *hba)
{
+ if (hba->dev_quirks & UFS_DEVICE_QUIRK_HOST_VS_DEBUGSAVECONFIGTIME) {
+ pr_info("ufs flash device must set VS_DebugSaveConfigTime
0x10\n");
+ /* VS_DebugSaveConfigTime */
+ ufshcd_dme_set(hba, UIC_ARG_MIB(0xD0A0), 0x10);
+ /* sync length */
+ ufshcd_dme_set(hba, UIC_ARG_MIB(0x1556), 0x48);
+ }
+
/* update */
ufshcd_dme_set(hba, UIC_ARG_MIB(0x15A8), 0x1);
/* PA_TxSkip */
diff --git a/drivers/scsi/ufs/ufs_quirks.h b/drivers/scsi/ufs/ufs_quirks.h
index 71f73d1d1ad1..5d2dfdb41a6f 100644
--- a/drivers/scsi/ufs/ufs_quirks.h
+++ b/drivers/scsi/ufs/ufs_quirks.h
@@ -131,4 +131,10 @@ struct ufs_dev_fix {
*/
#define UFS_DEVICE_QUIRK_HOST_PA_SAVECONFIGTIME(1 << 8)
+/*
+ * Some UFS devices require VS_DebugSaveConfigTime is 0x10,
+ * enabling this quirk ensure this.
+ */
+#define UFS_DEVICE_QUIRK_HOST_VS_DEBUGSAVECONFIGTIME (1 << 9)
+
#endif /* UFS_QUIRKS_H_ */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 27db55b0ca7f..f1c57cd33b5b 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -231,6 +231,8 @@ static struct ufs_dev_fix ufs_fixups[] = {
UFS_FIX(UFS_VENDOR_SKHYNIX, UFS_ANY_MODEL, UFS_DEVICE_NO_VCCQ),
UFS_FIX(UFS_VENDOR_SKHYNIX, UFS_ANY_MODEL,
UFS_DEVICE_QUIRK_HOST_PA_SAVECONFIGTIME),
+ UFS_FIX(UFS_VENDOR_SKHYNIX, "hB8aL1" /*H28U62301AMR*/,
+ UFS_DEVICE_QUIRK_HOST_VS_DEBUGSAVECONFIGTIME),
END_FIX
};
