Re: [PATCH V5 07/10] aacraid: Created new mutex for ioctl path
Hi Raghava,
[auto build test ERROR on scsi/for-next]
[also build test ERROR on v4.5-rc2 next-20160203]
[if your patch is applied to the wrong git tree, please drop us a note to help
improving the system]
url:
https://github.com/0day-ci/linux/commits/Raghava-Aditya-Renukunta/aacraid-Patchset-for-aacraid-driver-version-41052/20160203-085108
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: x86_64-lkp (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
Note: the
linux-review/Raghava-Aditya-Renukunta/aacraid-Patchset-for-aacraid-driver-version-41052/20160203-085108
HEAD 6b0bf60a881075778b6548999d5bc8d04268996b builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
drivers/scsi/aacraid/linit.c: In function 'aac_cfg_ioctl':
>> drivers/scsi/aacraid/linit.c:706:33: error: 'aac' undeclared (first use in
>> this function)
if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
^
drivers/scsi/aacraid/linit.c:706:33: note: each undeclared identifier is
reported only once for each function it appears in
vim +/aac +706 drivers/scsi/aacraid/linit.c
^1da177e Linus Torvalds 2005-04-16 700 * Bugs: Needs to handle
hot plugging
^1da177e Linus Torvalds 2005-04-16 701 */
^1da177e Linus Torvalds 2005-04-16 702
f4927c45 Arnd Bergmann2010-04-27 703 static long
aac_cfg_ioctl(struct file *file,
^1da177e Linus Torvalds 2005-04-16 704 unsigned int
cmd, unsigned long arg)
^1da177e Linus Torvalds 2005-04-16 705 {
f9c42596 Mahesh Rajashekhara 2015-03-26 @706 if
(!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
60395bb6 Alan Cox 2007-07-23 707 return -EPERM;
99da1769 Raghava Aditya Renukunta 2016-02-02 708 return
aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
^1da177e Linus Torvalds 2005-04-16 709 }
:: The code at line 706 was first introduced by commit
:: f9c4259678cbde854a4e94398d66ef379178fd7c aacraid: IOCTL fix
:: TO: Mahesh Rajashekhara
:: CC: James Bottomley
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: Binary data
RE: [patch] bfa: use strncpy() instead of memcpy()
Acked-by: Anil Gurumurthy
-Original Message-
From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
Sent: 30 January 2016 20:06
To: Anil Gurumurthy
Cc: Sudarsana Kalluru ; James E.J. Bottomley
; Martin K. Petersen ;
linux-scsi ; linux-kernel
; kernel-janit...@vger.kernel.org
Subject: [patch] bfa: use strncpy() instead of memcpy()
BFA_MFG_NAME is "QLogic" which is only 7 bytes, but we are copying 8 bytes.
It's harmless because the badding byte is likely zero but it makes static
checkers complain.
Signed-off-by: Dan Carpenter
---
Technically the memset() is not needed because strncpy() will pad the rest of
the buffer with zeros but I was worried that people would be paranoid.
diff --git a/drivers/scsi/bfa/bfa_ioc.c b/drivers/scsi/bfa/bfa_ioc.c index
251e2ff..a1ada4a 100644
--- a/drivers/scsi/bfa/bfa_ioc.c
+++ b/drivers/scsi/bfa/bfa_ioc.c
@@ -2803,7 +2803,7 @@ void
bfa_ioc_get_adapter_manufacturer(struct bfa_ioc_s *ioc, char *manufacturer) {
memset((void *)manufacturer, 0, BFA_ADAPTER_MFG_NAME_LEN);
- memcpy(manufacturer, BFA_MFG_NAME, BFA_ADAPTER_MFG_NAME_LEN);
+ strncpy(manufacturer, BFA_MFG_NAME, BFA_ADAPTER_MFG_NAME_LEN);
}
void
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] scsi: Fix endless loop of ATA hard resets due to VPD reads
On Tue, Feb 02, 2016 at 09:45:48PM -0500, Martin K. Petersen wrote: > > "Kirill" == Kirill A Shutemov writes: > > Kirill> I have the same problem. > > Kirill> Shouldn't we put quirk for that? > > I was hoping that Hannes' patch would do the trick so we could avoid > blacklisting: > > https://patchwork.kernel.org/patch/8079011/ It didn't help me. -- Kirill A. Shutemov -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v3 12/14] xen-scsiback: Convert to TARGET_SCF_ACK_KREF I/O krefs
Hi Juergen, On Tue, 2016-02-02 at 17:31 +0100, Juergen Gross wrote: > On 30/01/16 08:05, Nicholas A. Bellinger wrote: > > From: Nicholas Bellinger > > > > Cc: Juergen Gross > > Cc: Hannes Reinecke > > Cc: David Vrabel > > Signed-off-by: Nicholas Bellinger > > Sorry, with your patches applied xen-scsiback isn't working any more. > I've tried multiple times with and without your patches. Without the > patches everything is fine, while with the patches applied I get the > warnings shown in the attached log. I just passed through a DVD drive > and did "eject" in the domain. > Thanks for testing. :) So it looks like a left-over memset of pending_req->se_cmd memory in scsiback_cmd_exec() was clobbering the saved percpu_ida map_tag from scsiback_get_pend_req(), resulting in a use-after-free. Please verify with the following: diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c index eaf9e21..c3f55a2 100644 --- a/drivers/xen/xen-scsiback.c +++ b/drivers/xen/xen-scsiback.c @@ -400,10 +400,6 @@ static void scsiback_cmd_exec(struct vscsibk_pend *pending_req) struct se_session *sess = pending_req->v2p->tpg->tpg_nexus->tvn_se_sess; int rc; - memset(pending_req->sense_buffer, 0, VSCSIIF_SENSE_BUFFERSIZE); - - memset(se_cmd, 0, sizeof(*se_cmd)); - scsiback_get(pending_req->info); se_cmd->tag = pending_req->rqid; rc = target_submit_cmd_map_sgls(se_cmd, sess, pending_req->cmnd, -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] megaraid: fix null pointer check in megasas_detach_one().
> -Original Message- > From: Martin K. Petersen [mailto:martin.peter...@oracle.com] > Sent: Tuesday, February 02, 2016 7:24 AM > To: Maurizio Lombardi > Cc: sumit.sax...@avagotech.com; kashyap.de...@avagotech.com; > uday.ling...@avagotech.com; linux-scsi@vger.kernel.org; > jbottom...@parallels.com; the...@redhat.com > Subject: Re: [PATCH] megaraid: fix null pointer check in megasas_detach_one(). > > > "Maurizio" == Maurizio Lombardi writes: > > Maurizio> The pd_seq_sync pointer can't be NULL, we have to check its > Maurizio> entries instead. > > Sumit? Martin, Patch looks good but it needs to be rebased with latest repo as very recently you have checked in few megaraid_sas patches and there are two critical patches for megaraid_sas are yet pending. Once we get status of those two outstanding patches, I will ask submitter to rebase and send this patch again or I will be resending. Thanks, Sumit > > -- > Martin K. PetersenOracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What partition should the MTMKPART argument specify? Was: Re: st driver doesn't seem to grok LTO partitioning
Hello Finally got my firmware on my DAT updated. Using Kai's latest patch I validated the patch on my DAT driver as well Thanks to Shane for providing the correct mt code, as that was also one of my problems besides firmware. [root@srp-server mt-st-1.1-patched]# ./mt -f /dev/st0 stsetoption can-partitions [root@srp-server mt-st-1.1-patched]# ./mt -f /dev/st0 mkpartition 1000 Took almost 6 minutes to partition this old DDS Feb 02 22:25:10 srp-server kernel: st 6:0:1:0: [st0] Mode sense. Length 11, medium 0, WBS 10, BLL 8 Feb 02 22:25:10 srp-server kernel: st 6:0:1:0: [st0] Density 47, tape length: 0, drv buffer: 1 Feb 02 22:25:10 srp-server kernel: st 6:0:1:0: [st0] Block size: 0, buffer size: 4096 (1 blocks). Feb 02 22:25:10 srp-server kernel: st 6:0:1:0: [st0] Mode 0 options: buffer writes: 1, async writes: 1, read ahead: 1 Feb 02 22:25:10 srp-server kernel: st 6:0:1:0: [st0] can bsr: 1, two FMs: 0, fast mteom: 0, auto lock: 0, Feb 02 22:25:10 srp-server kernel: st 6:0:1:0: [st0] defs for wr: 0, no block limits: 0, partitions: 1, s2 log: 0 Feb 02 22:25:10 srp-server kernel: st 6:0:1:0: [st0] sysv: 0 nowait: 0 sili: 0 nowait_filemark: 0 Feb 02 22:25:10 srp-server kernel: st 6:0:1:0: [st0] debugging: 1 Feb 02 22:25:10 srp-server kernel: st 6:0:1:0: [st0] Rewinding tape. Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] Block limits 1 - 16777215 bytes. Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] Mode sense. Length 11, medium 0, WBS 10, BLL 8 Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] Density 47, tape length: 0, drv buffer: 1 Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] Block size: 0, buffer size: 4096 (1 blocks). Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] Updating partition number in status. Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] Got tape pos. blk 0 part 0. Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] Loading tape. Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] Error: 802, cmd: 0 0 0 0 0 0 Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] Sense Key : Unit Attention [current] Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] Add. Sense: Not ready to ready change, medium may have changed Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] Block limits 1 - 16777215 bytes. Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] Mode sense. Length 11, medium 0, WBS 10, BLL 8 Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] Density 47, tape length: 0, drv buffer: 1 Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] Block size: 0, buffer size: 4096 (1 blocks). Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] Partition page length is 10 bytes. Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] PP: max 1, add 0, xdp 0, psum 02, pofmetc 0, rec 03, units 00, sizes: 0 65535 Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] MP: 11 08 01 00 10 03 00 00 00 00 ff ff Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] psd_cnt 1, max.parts 1, nbr_parts 0 Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] Formatting tape with two partitions (1 = 1000 MB). Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] Sent partition page length is 10 bytes. needs_format: 0 Feb 02 22:25:24 srp-server kernel: st 6:0:1:0: [st0] PP: max 1, add 1, xdp 1, psum 02, pofmetc 0, rec 03, units 00, sizes: 1000 65535 Feb 02 22:31:45 srp-server kernel: st 6:0:1:0: [st0] Rewinding tape. I will retest with Shane's latest additions he just sent after first testing with Kai's latest patch on my LTO5. (here's hoping I dont have to update the f/w on that one) Laurence Oberman Principal Software Maintenance Engineer Red Hat Global Support Services - Original Message - From: "Kai Mäkisara (Kolumbus)" To: "Shane M Seymour" Cc: "Laurence Oberman" , "Emmanuel Florac" , "Laurence Oberman" , linux-scsi@vger.kernel.org Sent: Monday, February 1, 2016 1:43:26 PM Subject: Re: What partition should the MTMKPART argument specify? Was: Re: st driver doesn't seem to grok LTO partitioning > On 1.2.2016, at 8.31, Seymour, Shane M wrote: > > Hi Kai, > > Thanks for the changes the HPE DAT72 DDS5 drive now works as expected: > Good. Thanks for testing. ... > > I'm asking around again one final time to see if I can lay my hands on a LTO5 > or greater drive so I can test LTO partitioning as well. > > The only other thing I can think of (I'm not sure if this is an improvement > or not) is if bp[pgo + PP_OFF_MAX_ADD_PARTS] + bp[pgo + PP_OFF_NBR_ADD_PARTS] > (max.parts and nbr_parts in the debug message) is zero just return -EINVAL > unless you know of any take drives that report them both as 0 but can be > partitioned? That is after this: > >DEBC_printk(STp, "psd_cnt %d, max.parts %d, nbr_parts %d\n", >psd_cnt, bp[pgo + PP_OFF_MAX_ADD_PARTS], >bp[pgo + PP_OFF_NBR_ADD_PARTS]); > > add (and also turn off the can-partitions option): > > if ((bp[pgo + PP_OF
Re: [PATCH-v3 4/5] target: Fix remote-port TMR ABORT + se_cmd fabric stop
Btw, here's the updated patch:
>From 731b9981fc49d9b11e93b5c2f0a56e27862b4335 Mon Sep 17 00:00:00 2001
From: Nicholas Bellinger
Date: Tue, 19 Jan 2016 15:23:02 -0800
Subject: [PATCH] target: Fix remote-port TMR ABORT + se_cmd fabric stop
To address the bug where fabric driver level shutdown
of se_cmd occurs at the same time when TMR CMD_T_ABORTED
is happening resulting in a -1 ->cmd_kref, this patch
adds a target_tmr_put_cmd() wrapper + CMD_T_FABRIC_STOP
bit that is used to determine when TMR + driver I_T
nexus shutdown is happening concurrently.
It changes target_sess_cmd_list_set_waiting() to obtain
se_cmd->cmd_kref + set CMD_T_FABRIC_STOP, and drop local
reference in target_wait_for_sess_cmds() and invoke extra
target_put_sess_cmd() during Task Aborted Status (TAS)
when necessary.
Also, it adds a new target_wait_free_cmd() wrapper around
transport_wait_for_tasks() for the special case within
transport_generic_free_cmd() to set CMD_T_FABRIC_STOP,
and is now aware of CMD_T_ABORTED + CMD_T_TAS status
bits to know when an extra transport_put_cmd() during
TAS is required.
Note transport_generic_free_cmd() is expected to block on
cmd->cmd_wait_comp in order to follow what iscsi-target
expects during iscsi_conn context se_cmd shutdown.
Cc: Quinn Tran
Cc: Himanshu Madhani
Cc: Sagi Grimberg
Cc: Christoph Hellwig
Cc: Hannes Reinecke
Cc: Andy Grover
Cc: Mike Christie
Cc: sta...@vger.kernel.org # 3.10+
Signed-off-by: Nicholas Bellinger
---
drivers/target/target_core_tmr.c | 39 +-
drivers/target/target_core_transport.c | 132 -
include/target/target_core_base.h | 2 +
3 files changed, 136 insertions(+), 37 deletions(-)
diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index 9d67d16..2ac3228 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -110,6 +110,7 @@ static int target_check_cdb_and_preempt(struct list_head
*list,
static bool __target_check_io_state(struct se_cmd *se_cmd)
{
struct se_session *sess = se_cmd->se_sess;
+ bool ret;
assert_spin_locked(&sess->sess_cmd_lock);
WARN_ON_ONCE(!irqs_disabled());
@@ -129,10 +130,36 @@ static bool __target_check_io_state(struct se_cmd *se_cmd)
spin_unlock(&se_cmd->t_state_lock);
return false;
}
+ if (sess->sess_tearing_down || se_cmd->cmd_wait_set) {
+ pr_debug("Attempted to abort io tag: %llu already shutdown,"
+ " skipping\n", se_cmd->tag);
+ spin_unlock(&se_cmd->t_state_lock);
+ return false;
+ }
se_cmd->transport_state |= CMD_T_ABORTED;
spin_unlock(&se_cmd->t_state_lock);
- return kref_get_unless_zero(&se_cmd->cmd_kref);
+ ret = kref_get_unless_zero(&se_cmd->cmd_kref);
+ if (ret)
+ se_cmd->cmd_wait_set = 1;
+ return ret;
+}
+
+static void target_tmr_put_cmd(struct se_cmd *se_cmd)
+{
+ unsigned long flags;
+ bool fabric_stop;
+
+ spin_lock_irqsave(&se_cmd->t_state_lock, flags);
+ fabric_stop = (se_cmd->transport_state & CMD_T_FABRIC_STOP);
+ spin_unlock_irqrestore(&se_cmd->t_state_lock, flags);
+
+ target_put_sess_cmd(se_cmd);
+
+ if (!fabric_stop) {
+ wait_for_completion(&se_cmd->cmd_wait_comp);
+ se_cmd->se_tfo->release_cmd(se_cmd);
+ }
}
void core_tmr_abort_task(
@@ -178,7 +205,7 @@ void core_tmr_abort_task(
transport_wait_for_tasks(se_cmd);
transport_cmd_finish_abort(se_cmd, true);
- target_put_sess_cmd(se_cmd);
+ target_tmr_put_cmd(se_cmd);
printk("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for"
" ref_tag: %llu\n", ref_tag);
@@ -252,8 +279,12 @@ static void core_tmr_drain_tmr_list(
spin_unlock(&sess->sess_cmd_lock);
if (!rc) {
printk("LUN_RESET TMR: non-zero
kref_get_unless_zero\n");
+ spin_unlock(&sess->sess_cmd_lock);
continue;
}
+ cmd->cmd_wait_set = true;
+ spin_unlock(&sess->sess_cmd_lock);
+
list_move_tail(&tmr_p->tmr_list, &drain_tmr_list);
}
spin_unlock_irqrestore(&dev->se_tmr_lock, flags);
@@ -271,7 +302,7 @@ static void core_tmr_drain_tmr_list(
transport_wait_for_tasks(cmd);
transport_cmd_finish_abort(cmd, 1);
- target_put_sess_cmd(cmd);
+ target_tmr_put_cmd(cmd);
}
}
@@ -370,7 +401,7 @@ static void core_tmr_drain_state_list(
transport_wait_for_tasks(cmd);
core_tmr_handle_tas_abort(tmr_sess, cmd, tas);
- target_put_sess_cmd(cmd);
+ target_tmr_put_cmd(cmd);
}
}
diff --git a/drivers/target/target_core_transport.
Re: [PATCH 0/2] scsi: Fix endless loop of ATA hard resets due to VPD reads
> "Kirill" == Kirill A Shutemov writes: Kirill> I have the same problem. Kirill> Shouldn't we put quirk for that? I was hoping that Hannes' patch would do the trick so we could avoid blacklisting: https://patchwork.kernel.org/patch/8079011/ -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: What partition should the MTMKPART argument specify? Was: Re: st driver doesn't seem to grok LTO partitioning
Hi Kai, I've done more tested. Some stuff didn't work and I've got some suggested changes (there are two changes to the patch and one for the mt command). Testing results first: # echo 1 > /sys/bus/scsi/drivers/st/debug_flag # mt -f /dev/st2 stsetoption can-partitions # mt -f /dev/st1 stsetoption can-partitions # mt -f /dev/st0 stsetoption can-partitions Expect to fail LTO3: # mt -f /dev/st0 mkpartition 500 /dev/st0: Input/output error [ 3197.901583] st 5:0:1:0: [st0] Block limits 1 - 16777215 bytes. [ 3197.903613] st 5:0:1:0: [st0] Mode sense. Length 11, medium 0, WBS 10, BLL 8 [ 3197.903618] st 5:0:1:0: [st0] Density 44, tape length: 0, drv buffer: 1 [ 3197.903622] st 5:0:1:0: [st0] Block size: 0, buffer size: 4096 (1 blocks). [ 3197.903625] st 5:0:1:0: [st0] Updating partition number in status. [ 3197.906406] st 5:0:1:0: [st0] Got tape pos. blk 0 part 0. [ 3197.906429] st 5:0:1:0: [st0] Loading tape. [ 3197.929484] st 5:0:1:0: [st0] Block limits 1 - 16777215 bytes. [ 3197.931518] st 5:0:1:0: [st0] Mode sense. Length 11, medium 0, WBS 10, BLL 8 [ 3197.931524] st 5:0:1:0: [st0] Density 44, tape length: 0, drv buffer: 1 [ 3197.931527] st 5:0:1:0: [st0] Block size: 0, buffer size: 4096 (1 blocks). [ 3197.933840] st 5:0:1:0: [st0] Partition page length is 10 bytes. [ 3197.933846] st 5:0:1:0: [st0] PP: max 0, add 0, xdp 0, psum 03, pofmetc 0, rec 03, units 09, sizes: 400 65535 [ 3197.933851] st 5:0:1:0: [st0] MP: 11 08 00 00 18 03 09 00 01 90 ff ff [ 3197.933854] st 5:0:1:0: [st0] psd_cnt 1, max.parts 0, nbr_parts 0 [ 3197.933857] st 5:0:1:0: [st0] Formatting tape with two partitions (1 = 500 MB). [ 3197.933860] st 5:0:1:0: [st0] Sent partition page length is 10 bytes. needs_format: 0 [ 3197.933865] st 5:0:1:0: [st0] PP: max 0, add 1, xdp 1, psum 02, pofmetc 0, rec 03, units 00, sizes: 65535 500 [ 3197.933869] st 5:0:1:0: [st0] MP: 11 08 00 01 30 03 00 00 ff ff 01 f4 [ 3197.937706] st 5:0:1:0: [st0] Error: 802, cmd: 15 10 0 0 16 0 [ 3197.937712] st 5:0:1:0: [st0] Sense Key : Illegal Request [current] [ 3197.937716] st 5:0:1:0: [st0] Add. Sense: Invalid field in parameter list [ 3197.937719] st 5:0:1:0: [st0] Partitioning of tape failed. [ 3197.937847] st 5:0:1:0: [st0] Rewinding tape. Works DDS5: # mt -f /dev/st1 mkpartition 500 [ 3241.355474] st 6:0:3:0: [st1] Block limits 1 - 16777215 bytes. [ 3241.355775] st 6:0:3:0: [st1] Mode sense. Length 11, medium 0, WBS 10, BLL 8 [ 3241.355779] st 6:0:3:0: [st1] Density 47, tape length: 0, drv buffer: 1 [ 3241.355783] st 6:0:3:0: [st1] Block size: 0, buffer size: 4096 (1 blocks). [ 3241.355785] st 6:0:3:0: [st1] Updating partition number in status. [ 3241.356385] st 6:0:3:0: [st1] Got tape pos. blk 0 part 0. [ 3241.356397] st 6:0:3:0: [st1] Loading tape. [ 3241.357249] st 6:0:3:0: [st1] Block limits 1 - 16777215 bytes. [ 3241.357540] st 6:0:3:0: [st1] Mode sense. Length 11, medium 0, WBS 10, BLL 8 [ 3241.357544] st 6:0:3:0: [st1] Density 47, tape length: 0, drv buffer: 1 [ 3241.357547] st 6:0:3:0: [st1] Block size: 0, buffer size: 4096 (1 blocks). [ 3241.357882] st 6:0:3:0: [st1] Partition page length is 10 bytes. [ 3241.357887] st 6:0:3:0: [st1] PP: max 1, add 1, xdp 0, psum 02, pofmetc 0, rec 03, units 00, sizes: 500 65535 [ 3241.357892] st 6:0:3:0: [st1] MP: 11 08 01 01 10 03 00 00 01 f4 ff ff [ 3241.357895] st 6:0:3:0: [st1] psd_cnt 1, max.parts 1, nbr_parts 1 [ 3241.357898] st 6:0:3:0: [st1] Formatting tape with two partitions (1 = 500 MB). [ 3241.357901] st 6:0:3:0: [st1] Sent partition page length is 10 bytes. needs_format: 0 [ 3241.357906] st 6:0:3:0: [st1] PP: max 1, add 1, xdp 1, psum 02, pofmetc 0, rec 03, units 00, sizes: 500 65535 [ 3241.357910] st 6:0:3:0: [st1] MP: 11 08 01 01 30 03 00 00 01 f4 ff ff [ 3464.721058] st 6:0:3:0: [st1] Rewinding tape. # mt -f /dev/st2 mkpartition 200G Fails and doesn't print all of the messages related for partitioning: [ 3514.306582] st 8:0:0:0: [st2] Block limits 1 - 16777215 bytes. [ 3514.307126] st 8:0:0:0: [st2] Mode sense. Length 11, medium 0, WBS 10, BLL 8 [ 3514.307129] st 8:0:0:0: [st2] Density 5a, tape length: 0, drv buffer: 1 [ 3514.307132] st 8:0:0:0: [st2] Block size: 0, buffer size: 4096 (1 blocks). [ 3514.307133] st 8:0:0:0: [st2] Updating partition number in status. [ 3514.308133] st 8:0:0:0: [st2] Got tape pos. blk 0 part 0. [ 3514.308159] st 8:0:0:0: [st2] Loading tape. [ 3514.323173] st 8:0:0:0: [st2] Block limits 1 - 16777215 bytes. [ 3514.323624] st 8:0:0:0: [st2] Mode sense. Length 11, medium 0, WBS 10, BLL 8 [ 3514.323628] st 8:0:0:0: [st2] Density 5a, tape length: 0, drv buffer: 1 [ 3514.323632] st 8:0:0:0: [st2] Block size: 0, buffer size: 4096 (1 blocks). [ 3514.324507] st 8:0:0:0: [st2] Partition page length is 16 bytes. [ 3514.324513] st 8:0:0:0: [st2] PP: max 3, add 0, xdp 1, psum 03, pofmetc 4, rec 03, units 09, sizes: 2620 0 [ 3514.324518] st 8:0:0:0: [st2] MP: 11 0e 03 00 3c 03 09 00 0a 3c 00 00 [ 3514.324521] st 8:0:0:0: [st2] Sending FORMAT MEDIUM [ 3519.097142]
Re: [PATCH] Separate target visibility from reaped state information
On Tue, 2016-02-02 at 17:17 -0800, Bart Van Assche wrote:
> On 02/02/2016 04:43 PM, James Bottomley wrote:
> > On Tue, 2016-02-02 at 10:29 -0800, Bart Van Assche wrote:
> > > On 02/02/2016 03:46 AM, James Bottomley wrote:
> > > > diff --git a/drivers/scsi/scsi_sysfs.c
> > > > b/drivers/scsi/scsi_sysfs.c
> > > > index 4f18a85..00bc721 100644
> > > > --- a/drivers/scsi/scsi_sysfs.c
> > > > +++ b/drivers/scsi/scsi_sysfs.c
> > > > @@ -1272,16 +1272,18 @@ static void __scsi_remove_target(struct
> > > > scsi_target *starget)
> > > >void scsi_remove_target(struct device *dev)
> > > >{
> > > >struct Scsi_Host *shost = dev_to_shost(dev->parent);
> > > > - struct scsi_target *starget;
> > > > + struct scsi_target *starget, *last_target = NULL;
> > > >unsigned long flags;
> > > >
> > > >restart:
> > > >spin_lock_irqsave(shost->host_lock, flags);
> > > >list_for_each_entry(starget, &shost->__targets,
> > > > siblings) {
> > > > - if (starget->state == STARGET_DEL)
> > > > + if (starget->state == STARGET_DEL ||
> > > > + starget == last_target)
> > > >continue;
> > > >if (starget->dev.parent == dev || &starget->dev
> > > > ==
> > > > dev) {
> > > >kref_get(&starget->reap_ref);
> > > > + last_target = starget;
> > > >spin_unlock_irqrestore(shost->host_lock,
> > > > flags);
> > > >__scsi_remove_target(starget);
> > > >scsi_target_reap(starget);
> > >
> > > Hello James,
> > >
> > > Do you think it is a robust approach to store the pointer to the
> > > last
> > > removed target in the last_target variable ?
> >
> > Well, yes, I think it will work, if that's what you mean.
> >
> > > What if e.g. scsi_target_reap() frees the memory the
> > > last_target
> > > pointer points at and another thread reallocates a scsi_target
> > > data
> > > structure ? Can that last data structure have the same address as
> > > the
> > > contents of the last_target variable ?
> >
> > Yes, but it doesn't matter, does it? Add/Remove has always (and
> > will
> > always) be racy. Under current conditions you can still add to the
> > target after the list_for_each terminates and have
> > scsi_remove_target()
> > return with attached devices. The only way to close the race is
> > basically to forbid scanning as we shut down the host and wait for
> > all
> > in-progress scans before starting the final removals.
>
> Hello James,
>
> Although the scenario I described is unlikely if it happens it might
> be really hard to figure out what went wrong for someone who has not
> followed this discussion. This makes me wonder whether the above
> patch is really the best way to fix the reported soft lockup ...
The race you're worrying about exists without the fix, so the fix
doesn't alter the current situation as I explained. If you see another
issue, please say so (and explain it).
Thanks,
James
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v3 4/5] target: Fix remote-port TMR ABORT + se_cmd fabric stop
On Tue, 2016-02-02 at 11:54 +0100, Christoph Hellwig wrote:
> > + bool aborted = false, tas = false;
> >
> > /*
> > * Allocate an struct iscsi_conn_recovery for this connection.
> > @@ -398,7 +399,7 @@ int iscsit_prepare_cmds_for_realligance(struct
> > iscsi_conn *conn)
> >
> > iscsit_free_all_datain_reqs(cmd);
> >
> > - transport_wait_for_tasks(&cmd->se_cmd);
> > + transport_wait_for_tasks(&cmd->se_cmd, false, &aborted, &tas);
>
> please keep the existing transport_wait_for_tasks prototype and factor
> out a new helper for the transport_generic_free_cmd special case to avoid
> all this boiler plate.
Seems reasonable enough.
Moved existing code to __transport_wait_for_tasks() minus
taking/dropping the lock on entry/exit so a transport_wait_for_tasks()
wrapper works for existing callers, plus adding a new
target_wait_free_cmd() for transport_generic_free_cmd() special case.
>
> > + ret = kref_get_unless_zero(&se_cmd->cmd_kref);
> > + if (ret)
> > + se_cmd->cmd_wait_set = 1;
>
> I don't like the dual use of cmd_wait_set and the combination
> with CMD_T_FABRIC_STOP.
>
The dual use is required because there needs to be a mechanism for the
TMR path to release the command, but at the same time must also be aware
when the front-end driver needs to shutdown the command (from it's
calling context) at the same time, due to session reset.
For the session reset case, the key aspect is the fabric driver calling
target_wait_for_sess_cmds() or transport_generic_free_cmd() MUST NOT
return until se_cmd->se_tfo->release_cmd() has completed.
This is why the caller who sets ->cmd_wait_set is responsible for
calling ->release_cmd(), and not target_release_cmd_kref() callback
itself.
> How about the following alternative:
>
> pull in my prep patch to rewrite target_sess_cmd_list_set_waiting so
> that it doesn't need to iterate over all commands and set cmd_wait_set:
>
> http://www.spinics.net/lists/target-devel/msg11446.html
>
Your patch has one key assumption incorrect.
That is, it's not safe for target_wait_for_sess_cmds() to return until
all se_cmd->se_tfo->release_cmd() callbacks have been invoked. This is
because current code assumes that once this function completes, all
possible se_cmd related dereferences to se_session are finished.
The patch mentioned above ignores this requirement, and allows
target_wait_for_sess_cmds() to return before all ->release_cmd() calls
finish, while target_release_cmd_kref() completes in the back-ground
resulting in a sure-fire use-after-free.
> This gives us a free hand use a different completion for per-command
> waiting. Now your new usage of cmd_wait_set can be simplified as we
> don't need a CMD_T_FABRIC_STOP to undo it, it can simply be cleared.
> While we're at it I'd suggest using a CMD_T_ flag instead of a bitfield
> for it.
>
> > if (!(cmd->se_cmd_flags & SCF_SE_LUN_CMD)) {
> > if (wait_for_tasks && (cmd->se_cmd_flags & SCF_SCSI_TMR_CDB))
> > - transport_wait_for_tasks(cmd);
> > + transport_wait_for_tasks(cmd, true, &aborted, &tas);
> >
> > - ret = transport_put_cmd(cmd);
> > + if (!aborted || tas)
> > + ret = transport_put_cmd(cmd);
> > } else {
> > if (wait_for_tasks)
> > - transport_wait_for_tasks(cmd);
> > + transport_wait_for_tasks(cmd, true, &aborted, &tas);
> > /*
> > * Handle WRITE failure case where transport_generic_new_cmd()
> > * has already added se_cmd to state_list, but fabric has
> > @@ -2454,9 +2456,21 @@ int transport_generic_free_cmd(struct se_cmd *cmd,
> > int wait_for_tasks)
> > if (cmd->se_lun)
> > transport_lun_remove_cmd(cmd);
> >
> > - ret = transport_put_cmd(cmd);
> > + if (!aborted || tas)
> > + ret = transport_put_cmd(cmd);
> > }
>
> FYI, this can be simplified a bit more.
>
> Call transport_wait_for_tasks
>
> if (wait_for_tasks &&
> (cmd->se_cmd_flags & (SCF_SE_LUN_CMD | SCF_SCSI_TMR_CDB))) {
>
> and then the
>
> if (!aborted || tas)
> et = transport_put_cmd(cmd);
>
> can be moved behind the conditional for LUN_CMDs as well.
>
That doesn't quite work, because target_remove_from_state_list() +
transport_lun_remove_cmd() must be called before the potentially final
transport_put_cmd() -> target_put_sess_cmd() callback.
Also, SCF_SCSI_TMR_CDB is still slightly different in this regard, as it
doesn't take se_lun->lun_ref atm.
That said, I'm in agreement that this can be cleaned up further, but
given this patch will need to back-ported stable, I'd prefer to keep
additional cleanups as separate post bug-fix improvements.
> > - return ret;
> > + /*
> > +* If the task has been internally aborted due to TMR ABORT_TASK
> > +* or LUN_RESET, target_core_tmr.c is re
Re: [PATCH] Separate target visibility from reaped state information
On 02/02/2016 04:43 PM, James Bottomley wrote:
On Tue, 2016-02-02 at 10:29 -0800, Bart Van Assche wrote:
On 02/02/2016 03:46 AM, James Bottomley wrote:
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 4f18a85..00bc721 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1272,16 +1272,18 @@ static void __scsi_remove_target(struct
scsi_target *starget)
void scsi_remove_target(struct device *dev)
{
struct Scsi_Host *shost = dev_to_shost(dev->parent);
- struct scsi_target *starget;
+ struct scsi_target *starget, *last_target = NULL;
unsigned long flags;
restart:
spin_lock_irqsave(shost->host_lock, flags);
list_for_each_entry(starget, &shost->__targets, siblings) {
- if (starget->state == STARGET_DEL)
+ if (starget->state == STARGET_DEL ||
+ starget == last_target)
continue;
if (starget->dev.parent == dev || &starget->dev ==
dev) {
kref_get(&starget->reap_ref);
+ last_target = starget;
spin_unlock_irqrestore(shost->host_lock,
flags);
__scsi_remove_target(starget);
scsi_target_reap(starget);
Hello James,
Do you think it is a robust approach to store the pointer to the last
removed target in the last_target variable ?
Well, yes, I think it will work, if that's what you mean.
What if e.g. scsi_target_reap() frees the memory the last_target
pointer points at and another thread reallocates a scsi_target data
structure ? Can that last data structure have the same address as the
contents of the last_target variable ?
Yes, but it doesn't matter, does it? Add/Remove has always (and will
always) be racy. Under current conditions you can still add to the
target after the list_for_each terminates and have scsi_remove_target()
return with attached devices. The only way to close the race is
basically to forbid scanning as we shut down the host and wait for all
in-progress scans before starting the final removals.
Hello James,
Although the scenario I described is unlikely if it happens it might be
really hard to figure out what went wrong for someone who has not
followed this discussion. This makes me wonder whether the above patch
is really the best way to fix the reported soft lockup ...
Thanks,
Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V5 01/10] aacraid: SCSI blk tag support
From: Raghava Aditya Renukunta
The method to allocate and free FIB's in the present code utilizes
spinlocks.Multiple IO's have to wait on the spinlock to acquire or
free fibs creating a performance bottleneck.
An alternative solution would be to use block layer tags to keep track
of the fibs allocated and freed. To this end aac_fib_alloc_tag was
created to utilize the blk layer tags to plug into the Fib pool.These
functions are used exclusively in the IO path. 8 fibs are reserved for
the use of AIF management software and utilize the previous spinlock based
implementations.
Changes in V2:
Removed aac_fib_free_tag since it was a stub function
Moved population of fib fields that are constant to aac_fib_setup
Changes in V3:
None
Changes in V4:
None
Changes in V5:
None
Signed-off-by: Raghava Aditya Renukunta
Reviewed-by: Shane Seymour
Reviewed-by: Johannes Thumshirn
Reviewed-by: Tomas Henzl
---
drivers/scsi/aacraid/aachba.c | 27 ---
drivers/scsi/aacraid/aacraid.h | 1 +
drivers/scsi/aacraid/commsup.c | 32 +---
drivers/scsi/aacraid/dpcsup.c | 2 --
drivers/scsi/aacraid/linit.c | 2 ++
5 files changed, 44 insertions(+), 20 deletions(-)
diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index e4c2437..7dfd0fa 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -323,7 +323,6 @@ static inline int aac_valid_context(struct scsi_cmnd
*scsicmd,
if (unlikely(!scsicmd || !scsicmd->scsi_done)) {
dprintk((KERN_WARNING "aac_valid_context: scsi command
corrupt\n"));
aac_fib_complete(fibptr);
- aac_fib_free(fibptr);
return 0;
}
scsicmd->SCp.phase = AAC_OWNER_MIDLEVEL;
@@ -331,7 +330,6 @@ static inline int aac_valid_context(struct scsi_cmnd
*scsicmd,
if (unlikely(!device || !scsi_device_online(device))) {
dprintk((KERN_WARNING "aac_valid_context: scsi device
corrupt\n"));
aac_fib_complete(fibptr);
- aac_fib_free(fibptr);
return 0;
}
return 1;
@@ -541,7 +539,6 @@ static void get_container_name_callback(void *context,
struct fib * fibptr)
scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8 | SAM_STAT_GOOD;
aac_fib_complete(fibptr);
- aac_fib_free(fibptr);
scsicmd->scsi_done(scsicmd);
}
@@ -557,7 +554,8 @@ static int aac_get_container_name(struct scsi_cmnd *
scsicmd)
dev = (struct aac_dev *)scsicmd->device->host->hostdata;
- if (!(cmd_fibcontext = aac_fib_alloc(dev)))
+ cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
+ if (!cmd_fibcontext)
return -ENOMEM;
aac_fib_init(cmd_fibcontext);
@@ -586,7 +584,6 @@ static int aac_get_container_name(struct scsi_cmnd *
scsicmd)
printk(KERN_WARNING "aac_get_container_name: aac_fib_send failed with
status: %d.\n", status);
aac_fib_complete(cmd_fibcontext);
- aac_fib_free(cmd_fibcontext);
return -1;
}
@@ -1024,7 +1021,6 @@ static void get_container_serial_callback(void *context,
struct fib * fibptr)
scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8 | SAM_STAT_GOOD;
aac_fib_complete(fibptr);
- aac_fib_free(fibptr);
scsicmd->scsi_done(scsicmd);
}
@@ -1040,7 +1036,8 @@ static int aac_get_container_serial(struct scsi_cmnd *
scsicmd)
dev = (struct aac_dev *)scsicmd->device->host->hostdata;
- if (!(cmd_fibcontext = aac_fib_alloc(dev)))
+ cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
+ if (!cmd_fibcontext)
return -ENOMEM;
aac_fib_init(cmd_fibcontext);
@@ -1068,7 +1065,6 @@ static int aac_get_container_serial(struct scsi_cmnd *
scsicmd)
printk(KERN_WARNING "aac_get_container_serial: aac_fib_send failed with
status: %d.\n", status);
aac_fib_complete(cmd_fibcontext);
- aac_fib_free(cmd_fibcontext);
return -1;
}
@@ -1869,7 +1865,6 @@ static void io_callback(void *context, struct fib *
fibptr)
break;
}
aac_fib_complete(fibptr);
- aac_fib_free(fibptr);
scsicmd->scsi_done(scsicmd);
}
@@ -1954,7 +1949,8 @@ static int aac_read(struct scsi_cmnd * scsicmd)
/*
* Alocate and initialize a Fib
*/
- if (!(cmd_fibcontext = aac_fib_alloc(dev))) {
+ cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
+ if (!cmd_fibcontext) {
printk(KERN_WARNING "aac_read: fib allocation failed\n");
return -1;
}
@@ -2051,7 +2047,8 @@ static int aac_write(struct scsi_cmnd * scsicmd)
/*
* Allocate and initialize a Fib then setup a BlockWrite command
*/
- if (!(cmd_fibcontext = aac_fib_alloc(dev))) {
+ cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
+ if (!cmd_fibcontext) {
[PATCH V5 06/10] aacraid: Fundamental reset support for Series 7
From: Raghava Aditya Renukunta
Series 7 does not support PCI hot reset used by EEH.
Enabled fundamental reset only for Series 7
Changes in V2:
None
Changes in V3:
None
Changes in V4:
None
Changes in V5:
None
Signed-off-by: Raghava Aditya Renukunta
Reviewed-by: Johannes Thumshirn
Reviewed-by: Tomas Henzl
---
drivers/scsi/aacraid/linit.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 5117220..48e2a79 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -1135,6 +1135,12 @@ static int aac_probe_one(struct pci_dev *pdev, const
struct pci_device_id *id)
u64 dmamask;
extern int aac_sync_mode;
+ /*
+* Only series 7 needs freset.
+*/
+if (pdev->device == PMC_DEVICE_S7)
+ pdev->needs_freset = 1;
+
list_for_each_entry(aac, &aac_devices, entry) {
if (aac->id > unique_id)
break;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V5 09/10] aacraid: Fix character device re-initialization
From: Raghava Aditya Renukunta
During EEH PCI hotplug activity kernel unloads and loads the driver,
causing character device to be unregistered(aac_remove_one).When the
driver is loaded back using aac_probe_one the character device needs
to be registered again for the AIF management tools to work.
Fixed by adding code to register character device in aac_probe_one if
it is unregistered in aac_remove_one.
Changes in V2:
Added macros to track character device state
Changes in V3:
None
Changes in V4:
None
Changes in V5:
None
Signed-off-by: Raghava Aditya Renukunta
Reviewed-by: Shane Seymour
Reviewed-by: Johannes Thumshirn
---
drivers/scsi/aacraid/aacraid.h | 7 +++
drivers/scsi/aacraid/linit.c | 21 ++---
2 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 75bc65e..9cdf4d2 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -94,6 +94,13 @@ enum {
#define aac_phys_to_logical(x) ((x)+1)
#define aac_logical_to_phys(x) ((x)?(x)-1:0)
+/*
+ * These macros are for keeping track of
+ * character device state.
+ */
+#define AAC_CHARDEV_UNREGISTERED (-1)
+#define AAC_CHARDEV_NEEDS_REINIT (-2)
+
/* #define AAC_DETAILED_STATUS_INFO */
struct diskparm
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 2c7405a..6a5d00f 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -80,7 +80,7 @@ MODULE_VERSION(AAC_DRIVER_FULL_VERSION);
static DEFINE_MUTEX(aac_mutex);
static LIST_HEAD(aac_devices);
-static int aac_cfg_major = -1;
+static int aac_cfg_major = AAC_CHARDEV_UNREGISTERED;
char aac_driver_version[] = AAC_DRIVER_FULL_VERSION;
/*
@@ -1115,6 +1115,13 @@ static void __aac_shutdown(struct aac_dev * aac)
else if (aac->max_msix > 1)
pci_disable_msix(aac->pdev);
}
+static void aac_init_char(void)
+{
+ aac_cfg_major = register_chrdev(0, "aac", &aac_cfg_fops);
+ if (aac_cfg_major < 0) {
+ pr_err("aacraid: unable to register \"aac\" device.\n");
+ }
+}
static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
{
@@ -1172,6 +1179,9 @@ static int aac_probe_one(struct pci_dev *pdev, const
struct pci_device_id *id)
shost->max_cmd_len = 16;
shost->use_cmd_list = 1;
+ if (aac_cfg_major == AAC_CHARDEV_NEEDS_REINIT)
+ aac_init_char();
+
aac = (struct aac_dev *)shost->hostdata;
aac->base_start = pci_resource_start(pdev, 0);
aac->scsi_host_ptr = shost;
@@ -1511,7 +1521,7 @@ static void aac_remove_one(struct pci_dev *pdev)
pci_disable_device(pdev);
if (list_empty(&aac_devices)) {
unregister_chrdev(aac_cfg_major, "aac");
- aac_cfg_major = -1;
+ aac_cfg_major = AAC_CHARDEV_NEEDS_REINIT;
}
}
@@ -1671,11 +1681,8 @@ static int __init aac_init(void)
if (error < 0)
return error;
- aac_cfg_major = register_chrdev( 0, "aac", &aac_cfg_fops);
- if (aac_cfg_major < 0) {
- printk(KERN_WARNING
- "aacraid: unable to register \"aac\" device.\n");
- }
+ aac_init_char();
+
return 0;
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V5 07/10] aacraid: Created new mutex for ioctl path
aac_mutex was used to create protect the ioctl path for only the
compat path, it would be make more sense to place mutex in
aac_do_ioctl, which is the main ioctl function call that handles
all ioctl commands.
Created new mutex ioctl_mutex in struct aac_dev to protect switch
case in aac_do_ioctl and removed aac_mutex from aac_cfg_ioctl and
aac_compat_do_ioctl
Signed-off-by: Raghava Aditya Renukunta
---
drivers/scsi/aacraid/aacraid.h | 1 +
drivers/scsi/aacraid/commctrl.c | 4
drivers/scsi/aacraid/linit.c| 12 ++--
3 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 2916288..75bc65e 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -1124,6 +1124,7 @@ struct aac_dev
struct fib *free_fib;
spinlock_t fib_lock;
+ struct mutexioctl_mutex;
struct aac_queue_block *queues;
/*
* The user API will use an IOCTL to register itself to receive
diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 54195a1..4d5f4e7 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -855,6 +855,8 @@ int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user
*arg)
{
int status;
+ mutex_lock(&dev->ioctl_mutex);
+
/*
* HBA gets first crack
*/
@@ -890,6 +892,8 @@ int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user
*arg)
status = -ENOTTY;
break;
}
+ mutex_unlock(&dev->ioctl_mutex);
+
return status;
}
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 48e2a79..5f08bcf 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -703,23 +703,15 @@ static int aac_cfg_open(struct inode *inode, struct file
*file)
static long aac_cfg_ioctl(struct file *file,
unsigned int cmd, unsigned long arg)
{
- int ret;
- struct aac_dev *aac;
- aac = (struct aac_dev *)file->private_data;
if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
return -EPERM;
- mutex_lock(&aac_mutex);
- ret = aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
- mutex_unlock(&aac_mutex);
-
- return ret;
+ return aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
}
#ifdef CONFIG_COMPAT
static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned
long arg)
{
long ret;
- mutex_lock(&aac_mutex);
switch (cmd) {
case FSACTL_MINIPORT_REV_CHECK:
case FSACTL_SENDFIB:
@@ -753,7 +745,6 @@ static long aac_compat_do_ioctl(struct aac_dev *dev,
unsigned cmd, unsigned long
ret = -ENOIOCTLCMD;
break;
}
- mutex_unlock(&aac_mutex);
return ret;
}
@@ -1194,6 +1185,7 @@ static int aac_probe_one(struct pci_dev *pdev, const
struct pci_device_id *id)
goto out_free_host;
spin_lock_init(&aac->fib_lock);
+ mutex_init(&aac->ioctl_mutex);
/*
* Map in the registers from the adapter.
*/
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V5 08/10] aacraid: Fix AIF triggered IOP_RESET
From: Raghava Aditya Renukunta
while driver removal is in progress or PCI shutdown is invoked, driver
kills AIF aacraid thread, but IOCTL requests from the management tools
re-start AIF thread leading to IOP_RESET.
Fixed by setting adapter_shutdown flag when PCI shutdown is invoked.
Changes in V2:
Set adapter_shutdown flag before shutdown command is sent to \
controller
Changes in V3:
Call aac_send_shut_shutdown first thing in __aac_shutdown
Convert adapter_shutdown to atomic_t variable to prevent \
SMP coherency issues(race conditions)
Changes in V4:
Used mutex to protect ioctl path and adapter_shutdown to prevent \
race conditions.
Changes in V5:
Moved replacement of aac_mutex with ioctl_mutex to previous patch
Signed-off-by: Raghava Aditya Renukunta
Reviewed-by: Shane Seymour
Reviewed-by: Johannes Thumshirn
Reviewed-by: Tomas Henzl
---
drivers/scsi/aacraid/commctrl.c | 3 +++
drivers/scsi/aacraid/comminit.c | 6 --
drivers/scsi/aacraid/linit.c| 5 +++--
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 4d5f4e7..f6692d1 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -857,6 +857,9 @@ int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user
*arg)
mutex_lock(&dev->ioctl_mutex);
+ if (dev->adapter_shutdown)
+ return -EACCES;
+
/*
* HBA gets first crack
*/
diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c
index 0e954e3..2b4e753 100644
--- a/drivers/scsi/aacraid/comminit.c
+++ b/drivers/scsi/aacraid/comminit.c
@@ -212,8 +212,11 @@ int aac_send_shutdown(struct aac_dev * dev)
return -ENOMEM;
aac_fib_init(fibctx);
- cmd = (struct aac_close *) fib_data(fibctx);
+ mutex_lock(&dev->ioctl_mutex);
+ dev->adapter_shutdown = 1;
+ mutex_unlock(&dev->ioctl_mutex);
+ cmd = (struct aac_close *) fib_data(fibctx);
cmd->command = cpu_to_le32(VM_CloseAll);
cmd->cid = cpu_to_le32(0xfffe);
@@ -229,7 +232,6 @@ int aac_send_shutdown(struct aac_dev * dev)
/* FIB should be freed only after getting the response from the F/W */
if (status != -ERESTARTSYS)
aac_fib_free(fibctx);
- dev->adapter_shutdown = 1;
if ((dev->pdev->device == PMC_DEVICE_S7 ||
dev->pdev->device == PMC_DEVICE_S8 ||
dev->pdev->device == PMC_DEVICE_S9) &&
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 5f08bcf..2c7405a 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -703,7 +703,7 @@ static int aac_cfg_open(struct inode *inode, struct file
*file)
static long aac_cfg_ioctl(struct file *file,
unsigned int cmd, unsigned long arg)
{
- if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
+ if (!capable(CAP_SYS_RAWIO))
return -EPERM;
return aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
}
@@ -1069,6 +1069,8 @@ static void __aac_shutdown(struct aac_dev * aac)
int i;
int cpu;
+ aac_send_shutdown(aac);
+
if (aac->aif_thread) {
int i;
/* Clear out events first */
@@ -1080,7 +1082,6 @@ static void __aac_shutdown(struct aac_dev * aac)
}
kthread_stop(aac->thread);
}
- aac_send_shutdown(aac);
aac_adapter_disable_int(aac);
cpu = cpumask_first(cpu_online_mask);
if (aac->pdev->device == PMC_DEVICE_S6 ||
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V5 05/10] aacraid: Set correct msix count for EEH recovery
From: Raghava Aditya Renukunta
During EEH recovery number of online CPU's might change thereby changing
the number of MSIx vectors. Since each fib is allocated to a vector,
changes in the number of vectors causes fib to be sent thru invalid
vectors.In addition the correct number of MSIx vectors is not
updated in the INIT struct sent to the controller, when it is
reinitialized.
Fixed by reassigning vectors to fibs based on the updated number of MSIx
vectors and updating the INIT structure before sending to controller.
Changes in V2:
Replaced fib vector allocation code with aac_fib_vector_assign
Changes in V3:
None
Changes in V4:
None
Changes in V5:
None
Fixes: MSI-X vector calculation for suspend/resume
Cc: sta...@vger.kernel.org
Signed-off-by: Raghava Aditya Renukunta
Reviewed-by: Shane Seymour
Reviewed-by: Johannes Thumshirn
---
drivers/scsi/aacraid/linit.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 822b695..5117220 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -1409,8 +1409,18 @@ static int aac_acquire_resources(struct aac_dev *dev)
aac_adapter_enable_int(dev);
- if (!dev->sync_mode)
+ /*max msix may change after EEH
+* Re-assign vectors to fibs
+*/
+ aac_fib_vector_assign(dev);
+
+ if (!dev->sync_mode) {
+ /* After EEH recovery or suspend resume, max_msix count
+* may change, therfore updating in init as well.
+*/
aac_adapter_start(dev);
+ dev->init->Sa_MSIXVectors = cpu_to_le32(dev->max_msix);
+ }
return 0;
error_iounmap:
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V5 04/10] aacraid: Fix memory leak in aac_fib_map_free
From: Raghava Aditya Renukunta
aac_fib_map_free() calls pci_free_consistent() without checking that
dev->hw_fib_va is not NULL and dev->max_fib_size is not zero.If they
are indeed NULL/0, this will result in a hang as pci_free_consistent()
will attempt to invalidate cache for the entire 64-bit address space
(which would take a very long time).
Fixed by adding a check to make sure that dev->hw_fib_va and
dev->max_fib_size are not NULL and 0 respectively.
Changes in V2:
None
Changes in V3:
None
Changes in V4:
None
Changes in V5:
None
Fixes: 9ad5204d6 - "[SCSI]aacraid: incorrect dma mapping mask
during blinked recover or user initiated reset"
Cc: sta...@vger.kernel.org
Signed-off-by: Raghava Aditya Renukunta
Reviewed-by: Johannes Thumshirn
Reviewed-by: Tomas Henzl
---
drivers/scsi/aacraid/commsup.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index 07a42a3..511bbc5 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -83,9 +83,12 @@ static int fib_map_alloc(struct aac_dev *dev)
void aac_fib_map_free(struct aac_dev *dev)
{
- pci_free_consistent(dev->pdev,
- dev->max_fib_size * (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB),
- dev->hw_fib_va, dev->hw_fib_pa);
+ if (dev->hw_fib_va && dev->max_fib_size) {
+ pci_free_consistent(dev->pdev,
+ (dev->max_fib_size *
+ (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB)),
+ dev->hw_fib_va, dev->hw_fib_pa);
+ }
dev->hw_fib_va = NULL;
dev->hw_fib_pa = 0;
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V5 00/10] aacraid: Patchset for aacraid driver version 41052
This patchset includes the following changes (bug fixes and new feature support) specific to aacraid driver. V2: Removed aac_fib_free_tag function Setup relevant fib variables only once Created aac_fib_vector_assign function Made EEH functions static Added character device status macros changed location of aac->shutdown to prevent race condition Withdrew patch that disables device ID wild card binding Added Reviewed-by, Cc and Fixes tags from mailing list V3: Moved aac_send_shutdown to top of __aac_shutdown Converted aac->adapter_shutdown to atomic V4: Reverted adapter_shutdown from atomic Used mutex to prevent shutdown race conditions on ioctl path V5: Created new patch for ioctl_mutex Moved ioctl_mutex to aac_do_ioctl Fixed CONFIG_PM compilation issue Raghava Aditya Renukunta (10): [SCSI] aacraid: SCSI blk tag support [SCSI] aacraid: Fix RRQ overload [SCSI] aacraid: Added EEH support [SCSI] aacraid: Fix memory leak in aac_fib_map_free [SCSI] aacraid: Set correct msix count for EEH recovery [SCSI] aacraid: Fundamental reset support for Series 7 [SCSI] aacraid: Created new mutex for ioctl path [SCSI] aacraid: Fix AIF triggered IOP_RESET [SCSI] aacraid: Fix character device re-initialization [SCSI] aacraid: Update driver version drivers/scsi/aacraid/aachba.c | 27 +++--- drivers/scsi/aacraid/aacraid.h | 14 ++- drivers/scsi/aacraid/commctrl.c | 7 ++ drivers/scsi/aacraid/comminit.c | 6 +- drivers/scsi/aacraid/commsup.c | 69 -- drivers/scsi/aacraid/dpcsup.c | 2 - drivers/scsi/aacraid/linit.c| 198 +++- drivers/scsi/aacraid/src.c | 30 ++ 8 files changed, 283 insertions(+), 70 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V5 02/10] aacraid: Fix RRQ overload
From: Raghava Aditya Renukunta
The driver utilizes an array of atomic variables to keep track of
IO submissions to each vector. To submit an IO multiple threads
iterate through the array to find a vector which has empty slots
to send an IO. The reading and updating of the variable is not atomic,
causing race conditions when a thread uses a full vector to
submit an IO.
Fixed by mapping each FIB to a vector, the submission path then uses
said vector to submit IO thereby removing the possibly of a race
condition.The vector assignment is started from 1 since vector 0 is
reserved for the use of AIF management FIBS.If the number of MSIx
vectors is 1 (MSI or INTx mode) then all the fibs are allocated to
vector 0.
Changes in V2:
Created function aac_fib_vector_assign to assign vector numbers to
fib and to prevent code duplication
Changes in V3:
None
Changes in V4:
None
Changes in V5:
None
Fixes: 495c0217 "aacraid: MSI-x support"
Cc: sta...@vger.kernel.org # v4.1
Signed-off-by: Raghava Aditya Renukunta
Reviewed-by: Johannes Thumshirn
Reviewed-by: Tomas Henzl
---
drivers/scsi/aacraid/aacraid.h | 2 ++
drivers/scsi/aacraid/commsup.c | 28
drivers/scsi/aacraid/src.c | 30 +++---
3 files changed, 37 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index f51f0a0..fff1306 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -944,6 +944,7 @@ struct fib {
*/
struct list_headfiblink;
void*data;
+ u32 vector_no;
struct hw_fib *hw_fib_va; /* Actual shared object
*/
dma_addr_t hw_fib_pa; /* physical address of
hw_fib*/
};
@@ -2113,6 +2114,7 @@ static inline unsigned int cap_to_cyls(sector_t capacity,
unsigned divisor)
int aac_acquire_irq(struct aac_dev *dev);
void aac_free_irq(struct aac_dev *dev);
const char *aac_driverinfo(struct Scsi_Host *);
+void aac_fib_vector_assign(struct aac_dev *dev);
struct fib *aac_fib_alloc(struct aac_dev *dev);
struct fib *aac_fib_alloc_tag(struct aac_dev *dev, struct scsi_cmnd *scmd);
int aac_fib_setup(struct aac_dev *dev);
diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index 46a2a2f..07a42a3 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -90,6 +90,28 @@ void aac_fib_map_free(struct aac_dev *dev)
dev->hw_fib_pa = 0;
}
+void aac_fib_vector_assign(struct aac_dev *dev)
+{
+ u32 i = 0;
+ u32 vector = 1;
+ struct fib *fibptr = NULL;
+
+ for (i = 0, fibptr = &dev->fibs[i];
+ i < (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB);
+ i++, fibptr++) {
+ if ((dev->max_msix == 1) ||
+ (i > ((dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB - 1)
+ - dev->vector_cap))) {
+ fibptr->vector_no = 0;
+ } else {
+ fibptr->vector_no = vector;
+ vector++;
+ if (vector == dev->max_msix)
+ vector = 1;
+ }
+ }
+}
+
/**
* aac_fib_setup - setup the fibs
* @dev: Adapter to set up
@@ -152,6 +174,12 @@ int aac_fib_setup(struct aac_dev * dev)
hw_fib_pa = hw_fib_pa +
dev->max_fib_size + sizeof(struct aac_fib_xporthdr);
}
+
+ /*
+*Assign vector numbers to fibs
+*/
+ aac_fib_vector_assign(dev);
+
/*
* Add the fib chain to the free list
*/
diff --git a/drivers/scsi/aacraid/src.c b/drivers/scsi/aacraid/src.c
index 2aa34ea..bc0203f 100644
--- a/drivers/scsi/aacraid/src.c
+++ b/drivers/scsi/aacraid/src.c
@@ -156,8 +156,8 @@ static irqreturn_t aac_src_intr_message(int irq, void
*dev_id)
break;
if (dev->msi_enabled && dev->max_msix > 1)
atomic_dec(&dev->rrq_outstanding[vector_no]);
- aac_intr_normal(dev, handle-1, 0, isFastResponse, NULL);
dev->host_rrq[index++] = 0;
+ aac_intr_normal(dev, handle-1, 0, isFastResponse, NULL);
if (index == (vector_no + 1) * dev->vector_cap)
index = vector_no * dev->vector_cap;
dev->host_rrq_idx[vector_no] = index;
@@ -452,36 +452,20 @@ static int aac_src_deliver_message(struct fib *fib)
#endif
u16 hdr_size = le16_to_cpu(fib->hw_fib_va->header.Size);
+ u16 vector_no;
atomic_inc(&q->numpending);
if (dev->msi_enabled && fib->hw_fib_va->header.Command != AifRequest &&
dev->max_msix > 1) {
- u_int16_t vector_no, first_choice = 0x;
-
-
[PATCH V5 03/10] aacraid: Added EEH support
From: Raghava Aditya Renukunta
Added support for PCI EEH(extended error handling).
Changes in V2:
Made local functions static
Removed call to aac_fib_free_tag
Set adapter_shutdown flag when PCI error detected
Changes in V3:
None
Changes in V4:
Removed setting of adapter_shutdown flag when \
PCI error detected
Changes in V5:
Removed CONFIG_PM macro conditional compilation for \
aac_release_resources and aac_acquire_resources
Signed-off-by: Raghava Aditya Renukunta
Reviewed-by: Tomas Henzl
Reviewed-by: Johannes Thumshirn
---
drivers/scsi/aacraid/aacraid.h | 1 +
drivers/scsi/aacraid/linit.c | 140 -
2 files changed, 140 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index fff1306..2916288 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -1235,6 +1235,7 @@ struct aac_dev
struct msix_entry msixentry[AAC_MAX_MSIX];
struct aac_msix_ctx aac_msix[AAC_MAX_MSIX]; /* context */
u8 adapter_shutdown;
+ u32 handle_pci_error;
};
#define aac_adapter_interrupt(dev) \
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 129a515..822b695 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -38,6 +38,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1298,6 +1299,9 @@ static int aac_probe_one(struct pci_dev *pdev, const
struct pci_device_id *id)
goto out_deinit;
scsi_scan_host(shost);
+ pci_enable_pcie_error_reporting(pdev);
+ pci_save_state(pdev);
+
return 0;
out_deinit:
@@ -1319,7 +1323,6 @@ static int aac_probe_one(struct pci_dev *pdev, const
struct pci_device_id *id)
return error;
}
-#if (defined(CONFIG_PM))
static void aac_release_resources(struct aac_dev *aac)
{
int i;
@@ -1414,6 +1417,8 @@ error_iounmap:
return -1;
}
+
+#if (defined(CONFIG_PM))
static int aac_suspend(struct pci_dev *pdev, pm_message_t state)
{
@@ -1501,6 +1506,138 @@ static void aac_remove_one(struct pci_dev *pdev)
}
}
+static void aac_flush_ios(struct aac_dev *aac)
+{
+ int i;
+ struct scsi_cmnd *cmd;
+
+ for (i = 0; i < aac->scsi_host_ptr->can_queue; i++) {
+ cmd = (struct scsi_cmnd *)aac->fibs[i].callback_data;
+ if (cmd && (cmd->SCp.phase == AAC_OWNER_FIRMWARE)) {
+ scsi_dma_unmap(cmd);
+
+ if (aac->handle_pci_error)
+ cmd->result = DID_NO_CONNECT << 16;
+ else
+ cmd->result = DID_RESET << 16;
+
+ cmd->scsi_done(cmd);
+ }
+ }
+}
+
+static pci_ers_result_t aac_pci_error_detected(struct pci_dev *pdev,
+ enum pci_channel_state error)
+{
+ struct Scsi_Host *shost = pci_get_drvdata(pdev);
+ struct aac_dev *aac = shost_priv(shost);
+
+ dev_err(&pdev->dev, "aacraid: PCI error detected %x\n", error);
+
+ switch (error) {
+ case pci_channel_io_normal:
+ return PCI_ERS_RESULT_CAN_RECOVER;
+ case pci_channel_io_frozen:
+ aac->handle_pci_error = 1;
+
+ scsi_block_requests(aac->scsi_host_ptr);
+ aac_flush_ios(aac);
+ aac_release_resources(aac);
+
+ pci_disable_pcie_error_reporting(pdev);
+ aac_adapter_ioremap(aac, 0);
+
+ return PCI_ERS_RESULT_NEED_RESET;
+ case pci_channel_io_perm_failure:
+ aac->handle_pci_error = 1;
+
+ aac_flush_ios(aac);
+ return PCI_ERS_RESULT_DISCONNECT;
+ }
+
+ return PCI_ERS_RESULT_NEED_RESET;
+}
+
+static pci_ers_result_t aac_pci_mmio_enabled(struct pci_dev *pdev)
+{
+ dev_err(&pdev->dev, "aacraid: PCI error - mmio enabled\n");
+ return PCI_ERS_RESULT_NEED_RESET;
+}
+
+static pci_ers_result_t aac_pci_slot_reset(struct pci_dev *pdev)
+{
+ dev_err(&pdev->dev, "aacraid: PCI error - slot reset\n");
+ pci_restore_state(pdev);
+ if (pci_enable_device(pdev)) {
+ dev_warn(&pdev->dev,
+ "aacraid: failed to enable slave\n");
+ goto fail_device;
+ }
+
+ pci_set_master(pdev);
+
+ if (pci_enable_device_mem(pdev)) {
+ dev_err(&pdev->dev, "pci_enable_device_mem failed\n");
+ goto fail_device;
+ }
+
+ return PCI_ERS_RESULT_RECOVERED;
+
+fail_device:
+ dev_err(&pdev->dev, "aacraid: PCI error - slot reset failed\n");
+ return PCI_ERS_RESULT_DISCONNECT;
+}
+
+
+static void aac_pci_resume(struct pci_dev *pdev)
+{
+ struct Scsi_Host *shost = pci_get_drvdata(pdev);
+ struct scsi_device *sdev = NULL;
+ struct aac_dev *aac = (s
[PATCH V5 10/10] aacraid: Update driver version
From: Raghava Aditya Renukunta
Updated diver version to 41052
Changes in V2:
None
Changes in V3:
None
Changes in V4:
None
Changes in V5:
None
Signed-off-by: Raghava Aditya Renukunta
Reviewed-by: Johannes Thumshirn
---
drivers/scsi/aacraid/aacraid.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 9cdf4d2..efa493c 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -62,7 +62,7 @@ enum {
#definePMC_GLOBAL_INT_BIT0 0x0001
#ifndef AAC_DRIVER_BUILD
-# define AAC_DRIVER_BUILD 41010
+# define AAC_DRIVER_BUILD 41052
# define AAC_DRIVER_BRANCH "-ms"
#endif
#define MAXIMUM_NUM_CONTAINERS 32
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Separate target visibility from reaped state information
On Tue, 2016-02-02 at 10:29 -0800, Bart Van Assche wrote:
> On 02/02/2016 03:46 AM, James Bottomley wrote:
> > diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> > index 4f18a85..00bc721 100644
> > --- a/drivers/scsi/scsi_sysfs.c
> > +++ b/drivers/scsi/scsi_sysfs.c
> > @@ -1272,16 +1272,18 @@ static void __scsi_remove_target(struct
> > scsi_target *starget)
> > void scsi_remove_target(struct device *dev)
> > {
> > struct Scsi_Host *shost = dev_to_shost(dev->parent);
> > - struct scsi_target *starget;
> > + struct scsi_target *starget, *last_target = NULL;
> > unsigned long flags;
> >
> > restart:
> > spin_lock_irqsave(shost->host_lock, flags);
> > list_for_each_entry(starget, &shost->__targets, siblings) {
> > - if (starget->state == STARGET_DEL)
> > + if (starget->state == STARGET_DEL ||
> > + starget == last_target)
> > continue;
> > if (starget->dev.parent == dev || &starget->dev ==
> > dev) {
> > kref_get(&starget->reap_ref);
> > + last_target = starget;
> > spin_unlock_irqrestore(shost->host_lock,
> > flags);
> > __scsi_remove_target(starget);
> > scsi_target_reap(starget);
>
> Hello James,
>
> Do you think it is a robust approach to store the pointer to the last
> removed target in the last_target variable ?
Well, yes, I think it will work, if that's what you mean.
> What if e.g. scsi_target_reap() frees the memory the last_target
> pointer points at and another thread reallocates a scsi_target data
> structure ? Can that last data structure have the same address as the
> contents of the last_target variable ?
Yes, but it doesn't matter, does it? Add/Remove has always (and will
always) be racy. Under current conditions you can still add to the
target after the list_for_each terminates and have scsi_remove_target()
return with attached devices. The only way to close the race is
basically to forbid scanning as we shut down the host and wait for all
in-progress scans before starting the final removals.
James
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 111441] iscsi fails to attach to targets
https://bugzilla.kernel.org/show_bug.cgi?id=111441 --- Comment #14 from Serguei Bezverkhi --- Here you go, same output to compare for 4.4.1 and 3.10.0 Linux 4.4.1 #1 SMP Tue Feb 2 16:15:36 EST 2016 [root@sbezverk-osp-3 ~(keystone_admin)]# sg_inq -Hi /dev/sdc VPD INQUIRY: Device Identification page Designation descriptor number 1, descriptor length: 20 designator_type: NAA, code_set: Binary associated with the addressed logical unit designator header(hex): 01 03 00 10 designator: 00 60 01 40 56 5f e9 25 e7 94 a4 20 69 2c 0c b3 c6`.@V_.%... i,... Designation descriptor number 2, descriptor length: 60 designator_type: T10 vendor identification, code_set: ASCII associated with the addressed logical unit designator header(hex): 02 01 00 38 designator: 00 4c 49 4f 2d 4f 52 47 00 73 61 6e 2d 64 69 73 6bLIO-ORG.san-disk 10 2d 32 3a 36 35 66 65 39 32 35 65 2d 37 39 34 61-2:65fe925e-794a 20 2d 34 32 30 36 2d 39 32 63 30 2d 63 62 33 63 36-4206-92c0-cb3c6 30 39 61 35 33 34 39 61 00 9a5349a. Designation descriptor number 3, descriptor length: 8 transport: Internet SCSI (iSCSI) designator_type: Relative target port, code_set: Binary associated with the target port designator header(hex): 51 94 00 04 designator: 00 00 00 00 01 Designation descriptor number 4, descriptor length: 8 transport: Internet SCSI (iSCSI) designator_type: Target port group, code_set: Binary associated with the target port designator header(hex): 51 95 00 04 designator: 00 00 00 00 00 Designation descriptor number 5, descriptor length: 8 designator_type: Logical unit group, code_set: Binary associated with the addressed logical unit designator header(hex): 01 06 00 04 designator: 00 00 00 00 00 Designation descriptor number 6, descriptor length: 80 transport: Internet SCSI (iSCSI) designator_type: SCSI name string, code_set: UTF-8 associated with the target port designator header(hex): 53 98 00 4c designator: 00 69 71 6e 2e 32 30 30 33 2d 30 31 2e 6f 72 67 2eiqn.2003-01.org. 10 6c 69 6e 75 78 2d 69 73 63 73 69 2e 73 62 65 7alinux-iscsi.sbez 20 76 65 72 6b 2d 73 61 6e 2d 31 2e 78 38 36 36 34verk-san-1.x8664 30 3a 73 6e 2e 33 64 66 63 66 66 62 64 66 66 34 33:sn.3dfcffbdff43 40 2c 74 2c 30 78 30 30 30 31 00 00 00,t,0x0001... Designation descriptor number 7, descriptor length: 72 transport: Internet SCSI (iSCSI) designator_type: SCSI name string, code_set: UTF-8 associated with the target device that contains addressed lu designator header(hex): 53 a8 00 44 designator: 00 69 71 6e 2e 32 30 30 33 2d 30 31 2e 6f 72 67 2eiqn.2003-01.org. 10 6c 69 6e 75 78 2d 69 73 63 73 69 2e 73 62 65 7alinux-iscsi.sbez 20 76 65 72 6b 2d 73 61 6e 2d 31 2e 78 38 36 36 34verk-san-1.x8664 30 3a 73 6e 2e 33 64 66 63 66 66 62 64 66 66 34 33:sn.3dfcffbdff43 40 00 00 00 00 Linux 3.10.0-327.4.5.el7.x86_64 #1 SMP Thu Jan 21 04:10:29 EST 2016 [root@sbezverk-osp-3 ~(keystone_admin)]# sg_inq -Hi /dev/sdc VPD INQUIRY: Device Identification page Designation descriptor number 1, descriptor length: 20 designator_type: NAA, code_set: Binary associated with the addressed logical unit designator header(hex): 01 03 00 10 designator: 00 60 01 40 56 5f e9 25 e7 94 a4 20 69 2c 0c b3 c6`.@V_.%... i,... Designation descriptor number 2, descriptor length: 60 designator_type: T10 vendor identification, code_set: ASCII associated with the addressed logical unit designator header(hex): 02 01 00 38 designator: 00 4c 49 4f 2d 4f 52 47 00 73 61 6e 2d 64 69 73 6bLIO-ORG.san-disk 10 2d 32 3a 36 35 66 65 39 32 35 65 2d 37 39 34 61-2:65fe925e-794a 20 2d 34 32 30 36 2d 39 32 63 30 2d 63 62 33 63 36-4206-92c0-cb3c6 30 39 61 35 33 34 39 61 00 9a5349a. Designation descriptor number 3, descriptor length: 8 transport: Internet SCSI (iSCSI) designator_type: Relative target port, code_set: Binary associated with the target port designator header(hex): 51 94 00 04 designator: 00 00 00 00 01 Designation descriptor number 4, descriptor length: 8 transport: Internet SCSI (iSCSI) designator_type: Target port group, code_set: Binary associated with the target port designator header(hex): 51 95 00 04 designator: 00 00 00 00 00 Designation descriptor number 5, descriptor length: 8 designator_type: Logical unit group, code_set: Binary associated with the addressed logical unit d
RE: [Bug 111441] New: iscsi fails to attach to targets
Here you go, same output to compare for 4.4.1 and 3.10.0 Linux 4.4.1 #1 SMP Tue Feb 2 16:15:36 EST 2016 [root@sbezverk-osp-3 ~(keystone_admin)]# sg_inq -Hi /dev/sdc VPD INQUIRY: Device Identification page Designation descriptor number 1, descriptor length: 20 designator_type: NAA, code_set: Binary associated with the addressed logical unit designator header(hex): 01 03 00 10 designator: 00 60 01 40 56 5f e9 25 e7 94 a4 20 69 2c 0c b3 c6`.@V_.%... i,... Designation descriptor number 2, descriptor length: 60 designator_type: T10 vendor identification, code_set: ASCII associated with the addressed logical unit designator header(hex): 02 01 00 38 designator: 00 4c 49 4f 2d 4f 52 47 00 73 61 6e 2d 64 69 73 6bLIO-ORG.san-disk 10 2d 32 3a 36 35 66 65 39 32 35 65 2d 37 39 34 61-2:65fe925e-794a 20 2d 34 32 30 36 2d 39 32 63 30 2d 63 62 33 63 36-4206-92c0-cb3c6 30 39 61 35 33 34 39 61 00 9a5349a. Designation descriptor number 3, descriptor length: 8 transport: Internet SCSI (iSCSI) designator_type: Relative target port, code_set: Binary associated with the target port designator header(hex): 51 94 00 04 designator: 00 00 00 00 01 Designation descriptor number 4, descriptor length: 8 transport: Internet SCSI (iSCSI) designator_type: Target port group, code_set: Binary associated with the target port designator header(hex): 51 95 00 04 designator: 00 00 00 00 00 Designation descriptor number 5, descriptor length: 8 designator_type: Logical unit group, code_set: Binary associated with the addressed logical unit designator header(hex): 01 06 00 04 designator: 00 00 00 00 00 Designation descriptor number 6, descriptor length: 80 transport: Internet SCSI (iSCSI) designator_type: SCSI name string, code_set: UTF-8 associated with the target port designator header(hex): 53 98 00 4c designator: 00 69 71 6e 2e 32 30 30 33 2d 30 31 2e 6f 72 67 2eiqn.2003-01.org. 10 6c 69 6e 75 78 2d 69 73 63 73 69 2e 73 62 65 7alinux-iscsi.sbez 20 76 65 72 6b 2d 73 61 6e 2d 31 2e 78 38 36 36 34verk-san-1.x8664 30 3a 73 6e 2e 33 64 66 63 66 66 62 64 66 66 34 33:sn.3dfcffbdff43 40 2c 74 2c 30 78 30 30 30 31 00 00 00,t,0x0001... Designation descriptor number 7, descriptor length: 72 transport: Internet SCSI (iSCSI) designator_type: SCSI name string, code_set: UTF-8 associated with the target device that contains addressed lu designator header(hex): 53 a8 00 44 designator: 00 69 71 6e 2e 32 30 30 33 2d 30 31 2e 6f 72 67 2eiqn.2003-01.org. 10 6c 69 6e 75 78 2d 69 73 63 73 69 2e 73 62 65 7alinux-iscsi.sbez 20 76 65 72 6b 2d 73 61 6e 2d 31 2e 78 38 36 36 34verk-san-1.x8664 30 3a 73 6e 2e 33 64 66 63 66 66 62 64 66 66 34 33:sn.3dfcffbdff43 40 00 00 00 00 Linux 3.10.0-327.4.5.el7.x86_64 #1 SMP Thu Jan 21 04:10:29 EST 2016 [root@sbezverk-osp-3 ~(keystone_admin)]# sg_inq -Hi /dev/sdc VPD INQUIRY: Device Identification page Designation descriptor number 1, descriptor length: 20 designator_type: NAA, code_set: Binary associated with the addressed logical unit designator header(hex): 01 03 00 10 designator: 00 60 01 40 56 5f e9 25 e7 94 a4 20 69 2c 0c b3 c6`.@V_.%... i,... Designation descriptor number 2, descriptor length: 60 designator_type: T10 vendor identification, code_set: ASCII associated with the addressed logical unit designator header(hex): 02 01 00 38 designator: 00 4c 49 4f 2d 4f 52 47 00 73 61 6e 2d 64 69 73 6bLIO-ORG.san-disk 10 2d 32 3a 36 35 66 65 39 32 35 65 2d 37 39 34 61-2:65fe925e-794a 20 2d 34 32 30 36 2d 39 32 63 30 2d 63 62 33 63 36-4206-92c0-cb3c6 30 39 61 35 33 34 39 61 00 9a5349a. Designation descriptor number 3, descriptor length: 8 transport: Internet SCSI (iSCSI) designator_type: Relative target port, code_set: Binary associated with the target port designator header(hex): 51 94 00 04 designator: 00 00 00 00 01 Designation descriptor number 4, descriptor length: 8 transport: Internet SCSI (iSCSI) designator_type: Target port group, code_set: Binary associated with the target port designator header(hex): 51 95 00 04 designator: 00 00 00 00 00 Designation descriptor number 5, descriptor length: 8 designator_type: Logical unit group, code_set: Binary associated with the addressed logical unit designator header(hex): 01 06 00 04 designator: 00 00 00 00 00
Re: [PATCH-v3 1/5] target: Fix LUN_RESET active I/O handling for ACK_KREF
On Tue, 2016-02-02 at 10:37 +0100, Christoph Hellwig wrote: > > @@ -282,6 +305,16 @@ static void core_tmr_drain_state_list( > > if (prout_cmd == cmd) > > continue; > > > > + sess = cmd->se_sess; > > + if (WARN_ON_ONCE(!sess)) > > + continue; > > + > > + spin_lock(&sess->sess_cmd_lock); > > + rc = __target_check_io_state(cmd); > > + spin_unlock(&sess->sess_cmd_lock); > > + if (!rc) > > + continue; > > + > > I still don't understand why we care about the session or sess_cmd_lock > here. There isn't anything in __target_check_io_state that relies > on them (except for the assert checking the lock). Like I said earlier: "I'm still working on -v3 series to handle se_session shutdown during this specific multi-port LUN_RESET case, and considering using the existing se_cmd->cmd_wait_set bit to signal when this special case happens. Currently ->sess_cmd_lock is held in target_release_cmd_kref() and target_sess_cmd_list_set_waiting() while checking and setting this value." -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 111441] iscsi fails to attach to targets
https://bugzilla.kernel.org/show_bug.cgi?id=111441
--- Comment #13 from nab ---
On Mon, 2016-02-01 at 10:55 -0600, Mike Christie wrote:
> On 01/30/2016 01:38 AM, Nicholas A. Bellinger wrote:
> > On Fri, 2016-01-29 at 17:32 -0600, Mike Christie wrote:
> >> On 01/29/2016 04:21 PM, Serguei Bezverkhi (sbezverk) wrote:
> >>> HI Mike,
> >>>
> >>> I tried your patch and it is has eliminated first traceback but I still
> >>> do not see my remote targets.
> >>>
> >>
> >> That is sort of expected. Your target is not setup for ALUA properly. It
> >> says it supports ALUA, but when scsi_dh_alua asks about the ports it is
> >> reporting there are none. Ccing the people that made the patch that
> >> added the issue and own the code.
> >>
> >> Hey Christoph and Hannes,
> >>
> >> The dh/alua changes that added this:
> >>
> >> error = scsi_dh_add_device(sdev);
> >> if (error) {
> >> sdev_printk(KERN_INFO, sdev,
> >> "failed to add device handler: %d\n",
> >> error);
> >> return error;
> >> }
> >>
> >> to scsi_sysfs_add_sdev are adding a regression.
> >>
> >> 1. If that fails, then we forget to do device_del before doing the
> >> return. My patch in this thread added that back, so we do not see the
> >> sysfs oopses anymore. But.
> >>
> >> 2. It looks like in older kernels, we would allow misconfigured targets
> >> like this one to still setup devices. Do we want that old behavior back?
> >> Should we just ignore the return value from scsi_dh_add_device above?
> >> Note that in this case, it is LIO so it can be easily fixed on the
> >> target side by just setting it up properly. I do not think other targets
> >> would hit this type of issue.
> >>
> >
> > Btw, what does misconfigured mean here wrt target ALUA..?
>
> [ 25.833195] sd 6:0:0:4: alua: supports implicit and explicit TPGS
> [ 25.833360] sd 6:0:0:4: alua: No target port descriptors found
> [ 25.833363] sd 6:0:0:4: alua: Attach failed (-22)
> [ 25.833365] sd 6:0:0:4: failed to add device handler: -22
>
Strange, this hasn't changed in forever on the target side..
> He has LIO configured to report it supports implicit/explicit ALUA, but
> the ports do not seem to be configured.
>
> For the LIO config side, are his LUNs just not in a the default_lu_gp or
> any other group?
So every non-PSCSI backend device becomes part of default_lu_gp +
default_tg_pt_gp and automatically shows up in EVPD=0x83, without user
needing to do any additional configuration.
Here's what the output looks like:
root@haakon3:/usr/src/target-pending.git# sg_inq -Hi /dev/sdb
VPD INQUIRY: Device Identification page
Designation descriptor number 3, descriptor length: 8
transport: Serial Attached SCSI Protocol (SPL-2)
designator_type: Relative target port, code_set: Binary
associated with the target port
designator header(hex): 61 94 00 04
designator:
00 00 00 00 02
Designation descriptor number 4, descriptor length: 8
transport: Serial Attached SCSI Protocol (SPL-2)
designator_type: Target port group, code_set: Binary
associated with the target port
designator header(hex): 61 95 00 04
designator:
00 00 00 00 00
Designation descriptor number 5, descriptor length: 8
designator_type: Logical unit group, code_set: Binary
associated with the addressed logical unit
designator header(hex): 01 06 00 04
designator:
00 00 00 00 00
So AFAICT, the relative target port, target port group, and logical unit
group being returned from target on v4.5-rc1 code looks correct.
Serguei, can you confirm with 'sg_inq -Hi /dev/sdX' output on your side
with the v3.10 based target..?
AFAICT the parsing in scsi_vpd_tpg_id() from commit a8aa3978 looks
correct too.
Hannes, any ideas..?
--
You are receiving this mail because:
You are the assignee for the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug 111441] New: iscsi fails to attach to targets
On Mon, 2016-02-01 at 10:55 -0600, Mike Christie wrote:
> On 01/30/2016 01:38 AM, Nicholas A. Bellinger wrote:
> > On Fri, 2016-01-29 at 17:32 -0600, Mike Christie wrote:
> >> On 01/29/2016 04:21 PM, Serguei Bezverkhi (sbezverk) wrote:
> >>> HI Mike,
> >>>
> >>> I tried your patch and it is has eliminated first traceback but I still
> >>> do not see my remote targets.
> >>>
> >>
> >> That is sort of expected. Your target is not setup for ALUA properly. It
> >> says it supports ALUA, but when scsi_dh_alua asks about the ports it is
> >> reporting there are none. Ccing the people that made the patch that
> >> added the issue and own the code.
> >>
> >> Hey Christoph and Hannes,
> >>
> >> The dh/alua changes that added this:
> >>
> >> error = scsi_dh_add_device(sdev);
> >> if (error) {
> >> sdev_printk(KERN_INFO, sdev,
> >> "failed to add device handler: %d\n",
> >> error);
> >> return error;
> >> }
> >>
> >> to scsi_sysfs_add_sdev are adding a regression.
> >>
> >> 1. If that fails, then we forget to do device_del before doing the
> >> return. My patch in this thread added that back, so we do not see the
> >> sysfs oopses anymore. But.
> >>
> >> 2. It looks like in older kernels, we would allow misconfigured targets
> >> like this one to still setup devices. Do we want that old behavior back?
> >> Should we just ignore the return value from scsi_dh_add_device above?
> >> Note that in this case, it is LIO so it can be easily fixed on the
> >> target side by just setting it up properly. I do not think other targets
> >> would hit this type of issue.
> >>
> >
> > Btw, what does misconfigured mean here wrt target ALUA..?
>
> [ 25.833195] sd 6:0:0:4: alua: supports implicit and explicit TPGS
> [ 25.833360] sd 6:0:0:4: alua: No target port descriptors found
> [ 25.833363] sd 6:0:0:4: alua: Attach failed (-22)
> [ 25.833365] sd 6:0:0:4: failed to add device handler: -22
>
Strange, this hasn't changed in forever on the target side..
> He has LIO configured to report it supports implicit/explicit ALUA, but
> the ports do not seem to be configured.
>
> For the LIO config side, are his LUNs just not in a the default_lu_gp or
> any other group?
So every non-PSCSI backend device becomes part of default_lu_gp +
default_tg_pt_gp and automatically shows up in EVPD=0x83, without user
needing to do any additional configuration.
Here's what the output looks like:
root@haakon3:/usr/src/target-pending.git# sg_inq -Hi /dev/sdb
VPD INQUIRY: Device Identification page
Designation descriptor number 3, descriptor length: 8
transport: Serial Attached SCSI Protocol (SPL-2)
designator_type: Relative target port, code_set: Binary
associated with the target port
designator header(hex): 61 94 00 04
designator:
00 00 00 00 02
Designation descriptor number 4, descriptor length: 8
transport: Serial Attached SCSI Protocol (SPL-2)
designator_type: Target port group, code_set: Binary
associated with the target port
designator header(hex): 61 95 00 04
designator:
00 00 00 00 00
Designation descriptor number 5, descriptor length: 8
designator_type: Logical unit group, code_set: Binary
associated with the addressed logical unit
designator header(hex): 01 06 00 04
designator:
00 00 00 00 00
So AFAICT, the relative target port, target port group, and logical unit
group being returned from target on v4.5-rc1 code looks correct.
Serguei, can you confirm with 'sg_inq -Hi /dev/sdX' output on your side
with the v3.10 based target..?
AFAICT the parsing in scsi_vpd_tpg_id() from commit a8aa3978 looks
correct too.
Hannes, any ideas..?
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add support for DWC UFS Host Controller
Hi Joao, On Wed, Feb 3, 2016 at 1:47 AM, Joao Pinto wrote: > Hi, > In order to make a ufs-dwc-pci glue driver I will need to create a "pci driver > lib" like we have already for platform (ufshcd-pltfm.c). Should I call the > samsung glue driver ufs-samsung-pci.c which will use common pci functions > from a > ufshcd-pci.c? Agree? That sounds sensible. Thanks, -- Julian Calaby Email: julian.cal...@gmail.com Profile: http://www.google.com/profiles/julian.calaby/ -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] target: fix cast from pointer to phys_addr_t
(Adding Andy CC') On Mon, 2016-02-01 at 17:29 +0100, Arnd Bergmann wrote: > The uio_mem structure has a member that is a phys_addr_t, but can > be a number of other types too. The target core driver attempts > to assign a pointer from vmalloc() to it, by casting it to > phys_addr_t, but that causes a warning when phys_addr_t is longer > than a pointer: > > drivers/target/target_core_user.c: In function 'tcmu_configure_device': > drivers/target/target_core_user.c:906:22: error: cast from pointer to integer > of different size [-Werror=pointer-to-int-cast] > > This adds another cast to uintptr_t to shut up the warning. > A nicer fix might be to have additional fields in uio_mem > for the different purposes, so we can assign a pointer directly. > > Signed-off-by: Arnd Bergmann > --- > This has been around for a long while, but did not trigger in > my randconfig testing until now, don't know why. > > diff --git a/drivers/target/target_core_user.c > b/drivers/target/target_core_user.c > index dd600e5ead71..94f5154ac788 100644 > --- a/drivers/target/target_core_user.c > +++ b/drivers/target/target_core_user.c > @@ -903,7 +903,7 @@ static int tcmu_configure_device(struct se_device *dev) > info->version = __stringify(TCMU_MAILBOX_VERSION); > > info->mem[0].name = "tcm-user command & data buffer"; > - info->mem[0].addr = (phys_addr_t) udev->mb_addr; > + info->mem[0].addr = (phys_addr_t)(uintptr_t)udev->mb_addr; > info->mem[0].size = TCMU_RING_SIZE; > info->mem[0].memtype = UIO_MEM_VIRTUAL; > > Applied to target-pending/master. Thanks Arnd. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 111441] iscsi fails to attach to targets
https://bugzilla.kernel.org/show_bug.cgi?id=111441
--- Comment #12 from Serguei Bezverkhi ---
HI Mike,
It is working now, it spits alua related error messages but it attaches remote
and local targets, so my OpenStack is back to life. Please let me know if you
plan to commit this fix upstream.
[ 27.579303] scsi 3:0:0:2: Direct-Access LIO-ORG san-disk-2 4.0
PQ: 0 ANSI: 5
[ 27.579797] sd 3:0:0:2: alua: supports implicit and explicit TPGS
[ 27.579932] sd 3:0:0:2: [sdc] 20507809792 512-byte logical blocks: (10.4
TB/9.54 TiB)
[ 27.579975] sd 3:0:0:2: alua: No target port descriptors found
[ 27.579977] sd 3:0:0:2: alua: Attach failed (-22)
[ 27.580640] sd 3:0:0:2: failed to add device handler: -22
[ 27.580897] sd 3:0:0:2: [sdc] Write Protect is off
[ 27.580899] sd 3:0:0:2: [sdc] Mode Sense: 43 00 10 08
[ 27.580922] sd 3:0:0:2: Attached scsi generic sg3 type 0
[ 27.581041] sd 3:0:0:2: [sdc] Write cache: enabled, read cache: enabled,
supports DPO and FUA
[ 27.630156] sd 3:0:0:2: [sdc] Attached SCSI disk
Thank you very much
Serguei
Serguei Bezverkhi,
TECHNICAL LEADER.SERVICES
Global SP Services
sbezv...@cisco.com
Phone: +1 416 306 7312
Mobile: +1 514 234 7374
CCIE (R&S,SP,Sec) - #9527
Cisco.com
Think before you print.
This email may contain confidential and privileged material for the sole use of
the intended recipient. Any review, use, distribution or disclosure by others
is strictly prohibited. If you are not the intended recipient (or authorized to
receive for the recipient), please contact the sender by reply email and delete
all copies of this message.
Please click here for Company Registration Information.
-Original Message-
From: Mike Christie [mailto:micha...@cs.wisc.edu]
Sent: Tuesday, February 02, 2016 3:12 PM
To: Christoph Hellwig
Cc: Serguei Bezverkhi (sbezverk) ;
bugzilla-dae...@bugzilla.kernel.org; linux-scsi@vger.kernel.org; Hannes
Reinecke
Subject: Re: [Bug 111441] New: iscsi fails to attach to targets
On 02/02/2016 12:09 PM, Christoph Hellwig wrote:
> On Fri, Jan 29, 2016 at 05:32:54PM -0600, Mike Christie wrote:
>> Hey Christoph and Hannes,
>>
>> The dh/alua changes that added this:
>>
>> error = scsi_dh_add_device(sdev);
>> if (error) {
>> sdev_printk(KERN_INFO, sdev,
>> "failed to add device handler: %d\n",
>> error);
>> return error;
>> }
>>
>> to scsi_sysfs_add_sdev are adding a regression.
>>
>> 1. If that fails, then we forget to do device_del before doing the
>> return. My patch in this thread added that back, so we do not see the
>> sysfs oopses anymore. But.
>
> Ok.
>
>> 2. It looks like in older kernels, we would allow misconfigured
>> targets like this one to still setup devices. Do we want that old behavior
>> back?
>> Should we just ignore the return value from scsi_dh_add_device above?
>> Note that in this case, it is LIO so it can be easily fixed on the
>> target side by just setting it up properly. I do not think other
>> targets would hit this type of issue.
>
> Be liberal in what you accept.. I guess we need to continue allowing
> to connect to these broken targets, but a warning would be useful.
>
> Can you send a patch?
Serguei, can you try the attached patch? Drop the other one I sent.
--
You are receiving this mail because:
You are the assignee for the bug.--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [Bug 111441] New: iscsi fails to attach to targets
HI Mike,
It is working now, it spits alua related error messages but it attaches remote
and local targets, so my OpenStack is back to life. Please let me know if you
plan to commit this fix upstream.
[ 27.579303] scsi 3:0:0:2: Direct-Access LIO-ORG san-disk-2 4.0
PQ: 0 ANSI: 5
[ 27.579797] sd 3:0:0:2: alua: supports implicit and explicit TPGS
[ 27.579932] sd 3:0:0:2: [sdc] 20507809792 512-byte logical blocks: (10.4
TB/9.54 TiB)
[ 27.579975] sd 3:0:0:2: alua: No target port descriptors found
[ 27.579977] sd 3:0:0:2: alua: Attach failed (-22)
[ 27.580640] sd 3:0:0:2: failed to add device handler: -22
[ 27.580897] sd 3:0:0:2: [sdc] Write Protect is off
[ 27.580899] sd 3:0:0:2: [sdc] Mode Sense: 43 00 10 08
[ 27.580922] sd 3:0:0:2: Attached scsi generic sg3 type 0
[ 27.581041] sd 3:0:0:2: [sdc] Write cache: enabled, read cache: enabled,
supports DPO and FUA
[ 27.630156] sd 3:0:0:2: [sdc] Attached SCSI disk
Thank you very much
Serguei
Serguei Bezverkhi,
TECHNICAL LEADER.SERVICES
Global SP Services
sbezv...@cisco.com
Phone: +1 416 306 7312
Mobile: +1 514 234 7374
CCIE (R&S,SP,Sec) - #9527
Cisco.com
Think before you print.
This email may contain confidential and privileged material for the sole use of
the intended recipient. Any review, use, distribution or disclosure by others
is strictly prohibited. If you are not the intended recipient (or authorized to
receive for the recipient), please contact the sender by reply email and delete
all copies of this message.
Please click here for Company Registration Information.
-Original Message-
From: Mike Christie [mailto:micha...@cs.wisc.edu]
Sent: Tuesday, February 02, 2016 3:12 PM
To: Christoph Hellwig
Cc: Serguei Bezverkhi (sbezverk) ;
bugzilla-dae...@bugzilla.kernel.org; linux-scsi@vger.kernel.org; Hannes
Reinecke
Subject: Re: [Bug 111441] New: iscsi fails to attach to targets
On 02/02/2016 12:09 PM, Christoph Hellwig wrote:
> On Fri, Jan 29, 2016 at 05:32:54PM -0600, Mike Christie wrote:
>> Hey Christoph and Hannes,
>>
>> The dh/alua changes that added this:
>>
>> error = scsi_dh_add_device(sdev);
>> if (error) {
>> sdev_printk(KERN_INFO, sdev,
>> "failed to add device handler: %d\n",
>> error);
>> return error;
>> }
>>
>> to scsi_sysfs_add_sdev are adding a regression.
>>
>> 1. If that fails, then we forget to do device_del before doing the
>> return. My patch in this thread added that back, so we do not see the
>> sysfs oopses anymore. But.
>
> Ok.
>
>> 2. It looks like in older kernels, we would allow misconfigured
>> targets like this one to still setup devices. Do we want that old behavior
>> back?
>> Should we just ignore the return value from scsi_dh_add_device above?
>> Note that in this case, it is LIO so it can be easily fixed on the
>> target side by just setting it up properly. I do not think other
>> targets would hit this type of issue.
>
> Be liberal in what you accept.. I guess we need to continue allowing
> to connect to these broken targets, but a warning would be useful.
>
> Can you send a patch?
Serguei, can you try the attached patch? Drop the other one I sent.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [Bug 111441] New: iscsi fails to attach to targets
Sure thing, I will test it tonight and let you know the result.
Thank you
Serguei
-Original Message-
From: Mike Christie [mailto:micha...@cs.wisc.edu]
Sent: Tuesday, February 02, 2016 3:12 PM
To: Christoph Hellwig
Cc: Serguei Bezverkhi (sbezverk) ;
bugzilla-dae...@bugzilla.kernel.org; linux-scsi@vger.kernel.org; Hannes
Reinecke
Subject: Re: [Bug 111441] New: iscsi fails to attach to targets
On 02/02/2016 12:09 PM, Christoph Hellwig wrote:
> On Fri, Jan 29, 2016 at 05:32:54PM -0600, Mike Christie wrote:
>> Hey Christoph and Hannes,
>>
>> The dh/alua changes that added this:
>>
>> error = scsi_dh_add_device(sdev);
>> if (error) {
>> sdev_printk(KERN_INFO, sdev,
>> "failed to add device handler: %d\n",
>> error);
>> return error;
>> }
>>
>> to scsi_sysfs_add_sdev are adding a regression.
>>
>> 1. If that fails, then we forget to do device_del before doing the
>> return. My patch in this thread added that back, so we do not see the
>> sysfs oopses anymore. But.
>
> Ok.
>
>> 2. It looks like in older kernels, we would allow misconfigured
>> targets like this one to still setup devices. Do we want that old behavior
>> back?
>> Should we just ignore the return value from scsi_dh_add_device above?
>> Note that in this case, it is LIO so it can be easily fixed on the
>> target side by just setting it up properly. I do not think other
>> targets would hit this type of issue.
>
> Be liberal in what you accept.. I guess we need to continue allowing
> to connect to these broken targets, but a warning would be useful.
>
> Can you send a patch?
Serguei, can you try the attached patch? Drop the other one I sent.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 111441] iscsi fails to attach to targets
https://bugzilla.kernel.org/show_bug.cgi?id=111441
--- Comment #11 from Serguei Bezverkhi ---
Sure thing, I will test it tonight and let you know the result.
Thank you
Serguei
-Original Message-
From: Mike Christie [mailto:micha...@cs.wisc.edu]
Sent: Tuesday, February 02, 2016 3:12 PM
To: Christoph Hellwig
Cc: Serguei Bezverkhi (sbezverk) ;
bugzilla-dae...@bugzilla.kernel.org; linux-scsi@vger.kernel.org; Hannes
Reinecke
Subject: Re: [Bug 111441] New: iscsi fails to attach to targets
On 02/02/2016 12:09 PM, Christoph Hellwig wrote:
> On Fri, Jan 29, 2016 at 05:32:54PM -0600, Mike Christie wrote:
>> Hey Christoph and Hannes,
>>
>> The dh/alua changes that added this:
>>
>> error = scsi_dh_add_device(sdev);
>> if (error) {
>> sdev_printk(KERN_INFO, sdev,
>> "failed to add device handler: %d\n",
>> error);
>> return error;
>> }
>>
>> to scsi_sysfs_add_sdev are adding a regression.
>>
>> 1. If that fails, then we forget to do device_del before doing the
>> return. My patch in this thread added that back, so we do not see the
>> sysfs oopses anymore. But.
>
> Ok.
>
>> 2. It looks like in older kernels, we would allow misconfigured
>> targets like this one to still setup devices. Do we want that old behavior
>> back?
>> Should we just ignore the return value from scsi_dh_add_device above?
>> Note that in this case, it is LIO so it can be easily fixed on the
>> target side by just setting it up properly. I do not think other
>> targets would hit this type of issue.
>
> Be liberal in what you accept.. I guess we need to continue allowing
> to connect to these broken targets, but a warning would be useful.
>
> Can you send a patch?
Serguei, can you try the attached patch? Drop the other one I sent.
--
You are receiving this mail because:
You are the assignee for the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug 111441] New: iscsi fails to attach to targets
On 02/02/2016 12:09 PM, Christoph Hellwig wrote:
> On Fri, Jan 29, 2016 at 05:32:54PM -0600, Mike Christie wrote:
>> Hey Christoph and Hannes,
>>
>> The dh/alua changes that added this:
>>
>> error = scsi_dh_add_device(sdev);
>> if (error) {
>> sdev_printk(KERN_INFO, sdev,
>> "failed to add device handler: %d\n",
>> error);
>> return error;
>> }
>>
>> to scsi_sysfs_add_sdev are adding a regression.
>>
>> 1. If that fails, then we forget to do device_del before doing the
>> return. My patch in this thread added that back, so we do not see the
>> sysfs oopses anymore. But.
>
> Ok.
>
>> 2. It looks like in older kernels, we would allow misconfigured targets
>> like this one to still setup devices. Do we want that old behavior back?
>> Should we just ignore the return value from scsi_dh_add_device above?
>> Note that in this case, it is LIO so it can be easily fixed on the
>> target side by just setting it up properly. I do not think other targets
>> would hit this type of issue.
>
> Be liberal in what you accept.. I guess we need to continue allowing
> to connect to these broken targets, but a warning would be useful.
>
> Can you send a patch?
Serguei, can you try the attached patch? Drop the other one I sent.
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 21930c9..7dcc4bf 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1059,9 +1059,12 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
error = scsi_dh_add_device(sdev);
if (error) {
+ /*
+ * Allow device setup to succeed. Attachment can be retried
+ * from userpsace or device can be still used in degraded mode.
+ */
sdev_printk(KERN_INFO, sdev,
"failed to add device handler: %d\n", error);
- return error;
}
device_enable_async_suspend(&sdev->sdev_dev);
Re: [PATCHv4 22/23] scsi_dh_alua: update 'access_state' field
This looks ok to me, but the context will change a bit due to changes in previous patches. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [LSF/MM TOPIC] multiqueue and interrupt assignment
> -Original Message-
> From: linux-block-ow...@vger.kernel.org [mailto:linux-block-
> ow...@vger.kernel.org] On Behalf Of Hannes Reinecke
> Sent: Tuesday, February 2, 2016 10:31 AM
> To: lsf...@lists.linux-foundation.org; linux-scsi@vger.kernel.org; linux-
> bl...@vger.kernel.org
> Subject: [LSF/MM TOPIC] multiqueue and interrupt assignment
>
...
> As a side note, what does blk-mq do if the interrupt affinity is
> _deliberately_ set wrong? IE if the completions for one command
> arrive on completely the wrong queue? Discard the completion? Move
> it to the correct queue?
It sends an interprocessor interrupt (IPI) to the designated
processor and processes the completion there.
CPUs overloaded by this kind of work can lead to an unusable
system - see http://marc.info/?l=linux-kernel&m=141030836723181&w=2
---
Robert Elliott, HPE Persistent Memory
N�r��yb�X��ǧv�^�){.n�+{���"�{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�mzZ+�ݢj"��!�i
Re: [PATCH] Separate target visibility from reaped state information
On 02/02/2016 03:46 AM, James Bottomley wrote:
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 4f18a85..00bc721 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1272,16 +1272,18 @@ static void __scsi_remove_target(struct scsi_target
*starget)
void scsi_remove_target(struct device *dev)
{
struct Scsi_Host *shost = dev_to_shost(dev->parent);
- struct scsi_target *starget;
+ struct scsi_target *starget, *last_target = NULL;
unsigned long flags;
restart:
spin_lock_irqsave(shost->host_lock, flags);
list_for_each_entry(starget, &shost->__targets, siblings) {
- if (starget->state == STARGET_DEL)
+ if (starget->state == STARGET_DEL ||
+ starget == last_target)
continue;
if (starget->dev.parent == dev || &starget->dev == dev) {
kref_get(&starget->reap_ref);
+ last_target = starget;
spin_unlock_irqrestore(shost->host_lock, flags);
__scsi_remove_target(starget);
scsi_target_reap(starget);
Hello James,
Do you think it is a robust approach to store the pointer to the last
removed target in the last_target variable ? What if e.g.
scsi_target_reap() frees the memory the last_target pointer points at
and another thread reallocates a scsi_target data structure ? Can that
last data structure have the same address as the contents of the
last_target variable ?
Thanks,
Bart.
PLEASE NOTE: The information contained in this electronic mail message is
intended only for the use of the designated recipient(s) named above. If the
reader of this message is not the intended recipient, you are hereby notified
that you have received this message in error and that any review,
dissemination, distribution, or copying of this message is strictly prohibited.
If you have received this communication in error, please notify the sender by
telephone or e-mail (as shown above) immediately and destroy any and all copies
of this message in your possession (whether hard copies or electronically
stored copies).
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [LSF/MM TOPIC] multiqueue and interrupt assignment
On 02/02/2016 08:31 AM, Hannes Reinecke wrote: here's another topic which I've hit during my performance tests: How should interrupt affinity be handled with blk-multiqueue? The problem is that the blk-multiqueue assumes a certain CPU-to-queue mapping, _and_ the 'queue' in blk-mq syntax is actually a submission/completion queue pair. To achieve optimal performance one should set the interrupt affinity for a given (hardware) queue to the matchine (blk-mq) queue. But typically the interrupt affinity has to be set during HBA setup ie way before any queues are allocated. Which means we have three choices: - outguess the blk-mq algorithm in the driver and set the interrupt affinity during HBA setup - Add some callbacks to coordinate interrupt affinity between driver and blk-mq - Defer it to manual assignment, but inferring the risk of a suboptimal performance. At LSF/MM I would like to have a discussion on how the interrupt affinity should be handled for blk-mq, and whether a generic method is possible or desirable. Also there is the issue of certain drivers (eg lpfc) which normally do interrupt affinity themselves, but disable it for multiqueue. Which results in abysmal performance when comparing single queue against multiqueue :-( As a side note, what does blk-mq do if the interrupt affinity is _deliberately_ set wrong? IE if the completions for one command arrive on completely the wrong queue? Discard the completion? Move it to the correct queue? Hello Hannes, This topic indeed needs further attention. I also encountered this challenge while adding scsi-mq support to the SRP initiator driver. What I learned while working on the SRP driver is the following: - Although I agree that requests and interrupts should be processed on the same processor (same physical chip) if the request has been submitted from the CPU closest to the HBA, I'm not convinced that processing request completions and interrupts on the same CPU core yields the best performance. I would appreciate it if there would remain some freedom in how to assign interrupts to CPU cores. - In several older NUMA systems (Nehalem) the distance from processor to PCI adapter is the same for all processors. However, in current NUMA systems (Sandy Bridge and later) typically only from one processor access latency to a given PCI adapter is optimal. The question then becomes which code should hit the QPI latency penalty: the interrupt handler or the blk-mq request completion processing code ? - All HBAs I know of support reassignment of an interrupt to another CPU core through /proc/irq//smp_affinity so I was surprised to read that you encountered a HBA for which CPU affinity has to be set at driver load time ? - For HBAs that support multiple MSI-X vectors we need an approach for associating blk-mq hw-queues with MSI-X vectors. The approach implemented in the ib_srp driver is that that driver assumes that MSI-X vectors have been spread evenly over physical processors. The ib_srp driver then selects an MSI-X vector per hwqueue based on that assumption. Since neither the kernel nor irqbalance currently support this approach I wrote a script to implement this (see also http://thread.gmane.org/gmane.linux.kernel.device-mapper.devel/21312/focus=98409). - We need support in irqbalance for HBAs that support multiple MSI-X vectors. Last time I checked irqbalance did not support this concept which means that it even could happen that irqbalance assigned multiple of these interrupt vectors to the same CPU core, something that doesn't make sense to me. A previous discussion about this topic is available in the following e-mail thread: Christoph Hellwig, [TECH TOPIC] IRQ affinity, linux-rdma and linux-kernel mailing lists, July 2015 (http://thread.gmane.org/gmane.linux.drivers.rdma/27418). I would appreciate it if Matthew Wilcox' proposal could be discussed further during the LSF/MM (http://thread.gmane.org/gmane.linux.drivers.rdma/27418). Thanks, Bart. PLEASE NOTE: The information contained in this electronic mail message is intended only for the use of the designated recipient(s) named above. If the reader of this message is not the intended recipient, you are hereby notified that you have received this message in error and that any review, dissemination, distribution, or copying of this message is strictly prohibited. If you have received this communication in error, please notify the sender by telephone or e-mail (as shown above) immediately and destroy any and all copies of this message in your possession (whether hard copies or electronically stored copies). -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug 111441] New: iscsi fails to attach to targets
On Fri, Jan 29, 2016 at 05:32:54PM -0600, Mike Christie wrote:
> Hey Christoph and Hannes,
>
> The dh/alua changes that added this:
>
> error = scsi_dh_add_device(sdev);
> if (error) {
> sdev_printk(KERN_INFO, sdev,
> "failed to add device handler: %d\n",
> error);
> return error;
> }
>
> to scsi_sysfs_add_sdev are adding a regression.
>
> 1. If that fails, then we forget to do device_del before doing the
> return. My patch in this thread added that back, so we do not see the
> sysfs oopses anymore. But.
Ok.
> 2. It looks like in older kernels, we would allow misconfigured targets
> like this one to still setup devices. Do we want that old behavior back?
> Should we just ignore the return value from scsi_dh_add_device above?
> Note that in this case, it is LIO so it can be easily fixed on the
> target side by just setting it up properly. I do not think other targets
> would hit this type of issue.
Be liberal in what you accept.. I guess we need to continue allowing
to connect to these broken targets, but a warning would be useful.
Can you send a patch?
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 111441] iscsi fails to attach to targets
https://bugzilla.kernel.org/show_bug.cgi?id=111441
--- Comment #10 from Serguei Bezverkhi ---
Hello,
Any chance we could move forward with this investigation? I still cannot attach
to any remove iscsi targets with either 4.4.0 or 4.4.1 kernels.
Thank you
Serguei
-Original Message-
From: Mike Christie [mailto:micha...@cs.wisc.edu]
Sent: Monday, February 01, 2016 11:55 AM
To: Nicholas A. Bellinger
Cc: Serguei Bezverkhi (sbezverk) ;
bugzilla-dae...@bugzilla.kernel.org; linux-scsi@vger.kernel.org; Christoph
Hellwig ; Hannes Reinecke
Subject: Re: [Bug 111441] New: iscsi fails to attach to targets
On 01/30/2016 01:38 AM, Nicholas A. Bellinger wrote:
> On Fri, 2016-01-29 at 17:32 -0600, Mike Christie wrote:
>> On 01/29/2016 04:21 PM, Serguei Bezverkhi (sbezverk) wrote:
>>> HI Mike,
>>>
>>> I tried your patch and it is has eliminated first traceback but I still do
>>> not see my remote targets.
>>>
>>
>> That is sort of expected. Your target is not setup for ALUA properly.
>> It says it supports ALUA, but when scsi_dh_alua asks about the ports
>> it is reporting there are none. Ccing the people that made the patch
>> that added the issue and own the code.
>>
>> Hey Christoph and Hannes,
>>
>> The dh/alua changes that added this:
>>
>> error = scsi_dh_add_device(sdev);
>> if (error) {
>> sdev_printk(KERN_INFO, sdev,
>> "failed to add device handler: %d\n",
>> error);
>> return error;
>> }
>>
>> to scsi_sysfs_add_sdev are adding a regression.
>>
>> 1. If that fails, then we forget to do device_del before doing the
>> return. My patch in this thread added that back, so we do not see the
>> sysfs oopses anymore. But.
>>
>> 2. It looks like in older kernels, we would allow misconfigured
>> targets like this one to still setup devices. Do we want that old behavior
>> back?
>> Should we just ignore the return value from scsi_dh_add_device above?
>> Note that in this case, it is LIO so it can be easily fixed on the
>> target side by just setting it up properly. I do not think other
>> targets would hit this type of issue.
>>
>
> Btw, what does misconfigured mean here wrt target ALUA..?
[ 25.833195] sd 6:0:0:4: alua: supports implicit and explicit TPGS
[ 25.833360] sd 6:0:0:4: alua: No target port descriptors found
[ 25.833363] sd 6:0:0:4: alua: Attach failed (-22)
[ 25.833365] sd 6:0:0:4: failed to add device handler: -22
He has LIO configured to report it supports implicit/explicit ALUA, but the
ports do not seem to be configured.
For the LIO config side, are his LUNs just not in a the default_lu_gp or any
other group?
--
You are receiving this mail because:
You are the assignee for the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [Bug 111441] New: iscsi fails to attach to targets
Hello,
Any chance we could move forward with this investigation? I still cannot attach
to any remove iscsi targets with either 4.4.0 or 4.4.1 kernels.
Thank you
Serguei
-Original Message-
From: Mike Christie [mailto:micha...@cs.wisc.edu]
Sent: Monday, February 01, 2016 11:55 AM
To: Nicholas A. Bellinger
Cc: Serguei Bezverkhi (sbezverk) ;
bugzilla-dae...@bugzilla.kernel.org; linux-scsi@vger.kernel.org; Christoph
Hellwig ; Hannes Reinecke
Subject: Re: [Bug 111441] New: iscsi fails to attach to targets
On 01/30/2016 01:38 AM, Nicholas A. Bellinger wrote:
> On Fri, 2016-01-29 at 17:32 -0600, Mike Christie wrote:
>> On 01/29/2016 04:21 PM, Serguei Bezverkhi (sbezverk) wrote:
>>> HI Mike,
>>>
>>> I tried your patch and it is has eliminated first traceback but I still do
>>> not see my remote targets.
>>>
>>
>> That is sort of expected. Your target is not setup for ALUA properly.
>> It says it supports ALUA, but when scsi_dh_alua asks about the ports
>> it is reporting there are none. Ccing the people that made the patch
>> that added the issue and own the code.
>>
>> Hey Christoph and Hannes,
>>
>> The dh/alua changes that added this:
>>
>> error = scsi_dh_add_device(sdev);
>> if (error) {
>> sdev_printk(KERN_INFO, sdev,
>> "failed to add device handler: %d\n",
>> error);
>> return error;
>> }
>>
>> to scsi_sysfs_add_sdev are adding a regression.
>>
>> 1. If that fails, then we forget to do device_del before doing the
>> return. My patch in this thread added that back, so we do not see the
>> sysfs oopses anymore. But.
>>
>> 2. It looks like in older kernels, we would allow misconfigured
>> targets like this one to still setup devices. Do we want that old behavior
>> back?
>> Should we just ignore the return value from scsi_dh_add_device above?
>> Note that in this case, it is LIO so it can be easily fixed on the
>> target side by just setting it up properly. I do not think other
>> targets would hit this type of issue.
>>
>
> Btw, what does misconfigured mean here wrt target ALUA..?
[ 25.833195] sd 6:0:0:4: alua: supports implicit and explicit TPGS
[ 25.833360] sd 6:0:0:4: alua: No target port descriptors found
[ 25.833363] sd 6:0:0:4: alua: Attach failed (-22)
[ 25.833365] sd 6:0:0:4: failed to add device handler: -22
He has LIO configured to report it supports implicit/explicit ALUA, but the
ports do not seem to be configured.
For the LIO config side, are his LUNs just not in a the default_lu_gp or any
other group?
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[LSF/MM TOPIC] multiqueue and interrupt assignment
Hi all, here's another topic which I've hit during my performance tests: How should interrupt affinity be handled with blk-multiqueue? The problem is that the blk-multiqueue assumes a certain CPU-to-queue mapping, _and_ the 'queue' in blk-mq syntax is actually a submission/completion queue pair. To achieve optimal performance one should set the interrupt affinity for a given (hardware) queue to the matchine (blk-mq) queue. But typically the interrupt affinity has to be set during HBA setup ie way before any queues are allocated. Which means we have three choices: - outguess the blk-mq algorithm in the driver and set the interrupt affinity during HBA setup - Add some callbacks to coordinate interrupt affinity between driver and blk-mq - Defer it to manual assignment, but inferring the risk of a suboptimal performance. At LSF/MM I would like to have a discussion on how the interrupt affinity should be handled for blk-mq, and whether a generic method is possible or desirable. Also there is the issue of certain drivers (eg lpfc) which normally do interrupt affinity themselves, but disable it for multiqueue. Which results in abysmal performance when comparing single queue against multiqueue :-( As a side note, what does blk-mq do if the interrupt affinity is _deliberately_ set wrong? IE if the completions for one command arrive on completely the wrong queue? Discard the completion? Move it to the correct queue? Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v3 12/14] xen-scsiback: Convert to TARGET_SCF_ACK_KREF I/O krefs
On 30/01/16 08:05, Nicholas A. Bellinger wrote: > From: Nicholas Bellinger > > Cc: Juergen Gross > Cc: Hannes Reinecke > Cc: David Vrabel > Signed-off-by: Nicholas Bellinger Sorry, with your patches applied xen-scsiback isn't working any more. I've tried multiple times with and without your patches. Without the patches everything is fine, while with the patches applied I get the warnings shown in the attached log. I just passed through a DVD drive and did "eject" in the domain. Juergen [10984.266570] [ cut here ] [10984.266597] WARNING: CPU: 0 PID: 0 at drivers/target/target_core_transport.c:717 target_complete_cmd+0x1cb/0x200 [target_core_mod]() [10984.266601] Modules linked in: xt_physdev br_netfilter iptable_filter ip_tables x_tables loop target_core_pscsi target_core_file target_core_iblock iscsi_target_mod tcm_loop xen_scsiback bridge stp llc iscsi_ibft iscsi_boot_sysfs tun arc4 iwldvm mac80211 joydev iwlwifi uvcvideo videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_core snd_hda_codec_realtek cfg80211 snd_hda_codec_hdmi snd_hda_codec_generic videodev intel_rapl x86_pkg_temp_thermal e1000e intel_powerclamp snd_hda_intel media snd_hda_codec coretemp crct10dif_pclmul iTCO_wdt snd_hda_core crc32_pclmul iTCO_vendor_support dell_laptop crc32c_intel sdhci_pci rfkill ghash_clmulni_intel ptp snd_hwdep hmac i2c_hid drbg snd_pcm ansi_cprng hid dell_wmi ppdev snd_timer sparse_keymap dcdbas dell_smm_hwmon parport_pc parport snd i2c_designware_platform [10984.266670] i2c_designware_core thermal tpm_tis xhci_pci tpm evdev mei_me xhci_hcd aesni_intel mei aes_x86_64 psmouse shpchp lrw lpc_ich gf128mul i2c_i801 soundcore mfd_core pps_core ac glue_helper ablk_helper battery serio_raw pcspkr cryptd wmi target_core_mod xenfs xen_privcmd configfs dm_mod ext4 crc16 mbcache jbd2 sd_mod sr_mod cdrom i915 ehci_pci ehci_hcd i2c_algo_bit drm_kms_helper ahci libahci libata usbcore usb_common drm sdhci_acpi video sdhci mmc_core button xen_acpi_processor xen_pciback xen_netback xen_blkback xen_gntalloc xen_gntdev xen_evtchn sg scsi_mod autofs4 [10984.266735] CPU: 0 PID: 0 Comm: swapper/0 Tainted: GW 4.5.0-rc1-pv+ #1 [10984.266739] Hardware name: Dell Inc. Latitude E6440/0159N7, BIOS A07 06/26/2014 [10984.266742] a0422408 812e8bb4 8106e95c [10984.266748] 8800d2e007d0 8800d2e008e0 0001 a08c3150 [10984.266753] a04091ab 8800da13ea20 88000294f7c0 [10984.266758] Call Trace: [10984.266761][] ? dump_stack+0x40/0x5c [10984.266776] [] ? warn_slowpath_common+0x7c/0xb0 [10984.266784] [] ? pscsi_bi_endio+0x10/0x10 [target_core_pscsi] [10984.266794] [] ? target_complete_cmd+0x1cb/0x200 [target_core_mod] [10984.266799] [] ? pscsi_req_done+0x85/0xd0 [target_core_pscsi] [10984.266811] [] ? scsi_end_request+0xf7/0x1a0 [scsi_mod] [10984.266820] [] ? scsi_io_completion+0xfa/0x5f0 [scsi_mod] [10984.266830] [] ? blk_done_softirq+0x73/0x90 [10984.266836] [] ? __do_softirq+0xcc/0x240 [10984.266842] [] ? irq_exit+0x86/0x90 [10984.266852] [] ? xen_evtchn_do_upcall+0x2c/0x40 [10984.266862] [] ? xen_do_hypervisor_callback+0x1e/0x40 [10984.266864][] ? xen_hypercall_sched_op+0xa/0x20 [10984.266874] [] ? xen_hypercall_sched_op+0xa/0x20 [10984.266882] [] ? xen_safe_halt+0xc/0x20 [10984.266891] [] ? default_idle+0x13/0x90 [10984.266898] [] ? cpu_startup_entry+0x25d/0x2f0 [10984.266903] [] ? start_kernel+0x471/0x47c [10984.266907] [] ? set_init_arg+0x50/0x50 [10984.266912] [] ? xen_start_kernel+0x522/0x52c [10984.266916] ---[ end trace 07ad307d0cb62aa4 ]--- [10984.266940] [ cut here ] [10984.266953] WARNING: CPU: 0 PID: 2448 at drivers/target/target_core_transport.c:2120 target_complete_ok_work+0x291/0x2e0 [target_core_mod]() [10984.266955] Modules linked in: xt_physdev br_netfilter iptable_filter ip_tables x_tables loop target_core_pscsi target_core_file target_core_iblock iscsi_target_mod tcm_loop xen_scsiback bridge stp llc iscsi_ibft iscsi_boot_sysfs tun arc4 iwldvm mac80211 joydev iwlwifi uvcvideo videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_core snd_hda_codec_realtek cfg80211 snd_hda_codec_hdmi snd_hda_codec_generic videodev intel_rapl x86_pkg_temp_thermal e1000e intel_powerclamp snd_hda_intel media snd_hda_codec coretemp crct10dif_pclmul iTCO_wdt snd_hda_core crc32_pclmul iTCO_vendor_support dell_laptop crc32c_intel sdhci_pci rfkill ghash_clmulni_intel ptp snd_hwdep hmac i2c_hid drbg snd_pcm ansi_cprng hid dell_wmi ppdev snd_timer sparse_keymap dcdbas dell_smm_hwmon parport_pc parport snd i2c_designware_platform [10984.267013] i2c_designware_core thermal tpm_tis xhci_pci tpm evdev mei_me xhci_hcd aesni_intel mei aes_x86_64 psmouse shpchp lrw lpc_ich gf128mul i2c_i801 soundcore mfd_core pps_core ac glue_helper ablk_helper battery serio_raw pcspkr cryptd wmi target_core_mod xenfs xen_privc
Re: [PATCH] add support for DWC UFS Host Controller
Hi, In order to make a ufs-dwc-pci glue driver I will need to create a "pci driver lib" like we have already for platform (ufshcd-pltfm.c). Should I call the samsung glue driver ufs-samsung-pci.c which will use common pci functions from a ufshcd-pci.c? Agree? On 2/2/2016 11:49 AM, Julian Calaby wrote: > Hi Joao, > > On Tue, Feb 2, 2016 at 10:47 PM, Joao Pinto wrote: >> >> Hi Julian, >> I am already changing the architecture and I will send a v2 soon. >> Thanks for the review. > > Awesome, I look forward to it. > > Thanks, > Thanks. Joao -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add support for DWC UFS Host Controller
Hi Joao, On Tue, Feb 2, 2016 at 10:47 PM, Joao Pinto wrote: > > Hi Julian, > I am already changing the architecture and I will send a v2 soon. > Thanks for the review. Awesome, I look forward to it. Thanks, -- Julian Calaby Email: julian.cal...@gmail.com Profile: http://www.google.com/profiles/julian.calaby/ -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add support for DWC UFS Host Controller
Hi Julian,
I am already changing the architecture and I will send a v2 soon.
Thanks for the review.
Joao
On 2/2/2016 11:44 AM, Julian Calaby wrote:
> Hi Joao,
>
> On Tue, Feb 2, 2016 at 9:22 PM, Joao Pinto wrote:
>> Hi Julian,
>>
>> Thanks for the review. My comments are below.
>>
>> On 2/2/2016 1:00 AM, Julian Calaby wrote:
>>> Hi Joao,
>>>
>>> On Mon, Feb 1, 2016 at 11:47 PM, Joao Pinto wrote:
diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index d15eaa4..0ee6c62 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -167,6 +167,8 @@ static const struct dev_pm_ops ufshcd_pci_pm_ops = {
static const struct pci_device_id ufshcd_pci_tbl[] = {
{ PCI_VENDOR_ID_SAMSUNG, 0xC00C, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
+ { PCI_VENDOR_ID_SYNOPSYS, 0xB101, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0
},
+ { PCI_VENDOR_ID_SYNOPSYS, 0xB102, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0
},
>>>
>>> Listing these here implies that the devices these lines match are
>>> "normal" PCI UFSHCD devices that don't require any special handling
>>> whatsoever. Is that correct?
>>
>> Yes they are normal PCI UFSHCD devices.
>>
>>>
>>> If they do require special handling, then you need to put them in a
>>> separate driver, e.g. ufs-dwc-pci.c
>>
>> Both ufs-dwc and pci driver must execute the same init sequence to correctly
>> kick-off the hardware. That's why the common code is in the ufshcd.c.
>> Maybe create a ufshcd-dwc-quirks.c with the dwc specififc code would be
>> better.
>> This way it could be used by ufs-dwc platform driver and pci.
>> Since dwc hardware uses a specific init routine would it be better to have a
>> ufs-dwc-pci and ufs-dwc calling the dwc specific init routine?
>
> What you suggested below, i.e. having a ufshcd-dwc.c file containing
> the common stuff then having separate platform and PCI drivers sounds
> like the best option.
>
{ } /* terminate list */
};
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 85cd256..05d309d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5521,6 +5541,790 @@ static struct devfreq_dev_profile
ufs_devfreq_profile = {
.get_dev_status = ufshcd_devfreq_get_dev_status,
};
+#ifdef CONFIG_SCSI_UFS_DWC_HOOKS
>>>
>>> This doesn't look right.
>>>
>>> The driver should be structured like this:
>>>
>>> - ufs-dwc: contains everything that is specific to your hardware.
>>> - ufshcd: contains everything that is common to multiple types of
>>> ufshcd from different vendors
>>>
>>> Your "hooks" here look like they're doing stuff that is specific to
>>> the Designware hardware. They should not be in this file as it's for
>>> hardware type independent code.
>>>
>>> If you need to do something special at some point in the common code,
>>> then this should be exposed as an op in struct ufs_hba_variant_ops
>>> which is then implemented in your device-specific code.
>>
>> Yes I agree that maybe the ufs core drive is not the perfect spot for
>> specific
>> vendor code. But DWC HW can be using pci or platform and so it has to share
>> common code and that's why I put it in the ufshcd.
>>
>> I think creating a ufshcd-dwc.c would be better to share code between ufs-dwc
>> and ufs-dwc-pci. Agree?
>
> Agreed.
>
+/**
+ * ufshcd_dwc_program_clk_div()
+ * This function programs the clk divider value. This value is needed to
+ * provide 1 microsecond tick to unipro layer.
+ * @hba: Private Structure pointer
+ * @divider_val: clock divider value to be programmed
+ *
+ */
+void ufshcd_dwc_program_clk_div(struct ufs_hba *hba, u32 divider_val)
+{
+ ufshcd_writel(hba, divider_val, DWC_UFS_REG_HCLKDIV);
+}
+
+/**
+ * ufshcd_dwc_link_is_up()
+ * Check if link is up
+ * @hba: private structure poitner
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+int ufshcd_dwc_link_is_up(struct ufs_hba *hba)
+{
+ int dme_result = 0;
+
+ ufshcd_dme_get(hba, UIC_ARG_MIB(VS_POWERSTATE), &dme_result);
+
+ if (dme_result == UFSHCD_LINK_IS_UP) {
+ ufshcd_set_link_active(hba);
+ return 0;
+ }
+
+ return 1;
+}
+
+/**
+ * ufshcd_dwc_connection_setup()
+ * This function configures both the local side (host) and the peer side
+ * (device) unipro attributes to establish the connection to application/
+ * cport.
+ * This function is not required if the hardware is properly configured to
+ * have this connection setup on reset. But invoking this function does no
+ * harm and should be fine even working with any ufs device.
+ *
+ * @hba: pointer to drivers private data
+ *
+ * Returns 0 on suc
Re: [PATCH] Separate target visibility from reaped state information
On Mon, 2016-02-01 at 19:43 -0800, Bart Van Assche wrote:
> On 01/19/16 17:03, James Bottomley wrote:
> > On Tue, 2016-01-19 at 19:30 -0500, Martin K. Petersen wrote:
> > > > > > > > "Bart" == Bart Van Assche
> > > > > > > > writes:
> > >
> > > Bart> Instead of representing the states "visible in sysfs" and
> > > "has
> > > Bart> been removed from the target list" by a single state
> > > variable,
> > > use
> > > Bart> two variables to represent this information.
> > >
> > > James: Are you happy with the latest iteration of this? Should I
> > > queue
> > > it?
> >
> > Well, I'm OK with the patch: it's a simple transformation of the
> > enumerated state to a two bit state. What I can't see is how it
> > fixes
> > any soft lockup.
> >
> > The only change from the current workflow is that the DEL
> > transition
> > (now the reaped flag) is done before the spin lock is dropped which
> > would fix a tiny window for two threads both trying to remove the
> > same
> > target, but there's nothing that could possibly fix an iterative
> > soft
> > lockup caused by restarting the loop, which is what the changelog
> > says.
>
> Hello James,
>
> scsi_remove_target() doesn't lock the scan_mutex which means that
> concurrent SCSI scanning activity is not prohibited. Such scanning
> activity can postpone the transition of the state of a SCSI target
> into STARGET_DEL. I think if the scheduler decides to run the thread
> that executes scsi_remove_target() on the same CPU as the scanning
> code after the scanning code has obtained a reap ref and before the
> scanning code has released the reap ref again that the soft lockup
> can be triggered that has been reported by Sebastian Herbszt.
OK, I finally understand the scenario; I'm not sure I understand how
we're getting concurrent scanning and removal from a simple rmmod ... I
take it this is insmod rmmod in a tight loop?
So this patch now actually introduces a problem the other way: we can
do a scan with a dying target, which will lead to problems down the
road. The original design of the code was to allow the target to be
resurrected even while being removed, because the target doesn't exist
independently of the devices ... when the last device is removed the
target is reaped. So a test case this would need to pass is adding and
removing a single device on a target in a tight loop. The problem
you'll see is that eventually the add will fail nastily with your code
because the target can't be resurrected even though we have a reference
and we find a device to attach because once we set your reaped flag,
the destruction is irrevocable.
All we really need to break the soft lockup is to not keep looping over
a device that we've called remove on but which hasn't gone into DEL
state. So how about this. It will retain a simplistic memory of the
last target and not keep looping over it. I think it will fix the soft
lockup and preserve the resurrection of the target for the device
add/remove case.
James
---
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 4f18a85..00bc721 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1272,16 +1272,18 @@ static void __scsi_remove_target(struct scsi_target
*starget)
void scsi_remove_target(struct device *dev)
{
struct Scsi_Host *shost = dev_to_shost(dev->parent);
- struct scsi_target *starget;
+ struct scsi_target *starget, *last_target = NULL;
unsigned long flags;
restart:
spin_lock_irqsave(shost->host_lock, flags);
list_for_each_entry(starget, &shost->__targets, siblings) {
- if (starget->state == STARGET_DEL)
+ if (starget->state == STARGET_DEL ||
+ starget == last_target)
continue;
if (starget->dev.parent == dev || &starget->dev == dev) {
kref_get(&starget->reap_ref);
+ last_target = starget;
spin_unlock_irqrestore(shost->host_lock, flags);
__scsi_remove_target(starget);
scsi_target_reap(starget);
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add support for DWC UFS Host Controller
Hi Joao,
On Tue, Feb 2, 2016 at 9:22 PM, Joao Pinto wrote:
> Hi Julian,
>
> Thanks for the review. My comments are below.
>
> On 2/2/2016 1:00 AM, Julian Calaby wrote:
>> Hi Joao,
>>
>> On Mon, Feb 1, 2016 at 11:47 PM, Joao Pinto wrote:
>>> diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
>>> index d15eaa4..0ee6c62 100644
>>> --- a/drivers/scsi/ufs/ufshcd-pci.c
>>> +++ b/drivers/scsi/ufs/ufshcd-pci.c
>>> @@ -167,6 +167,8 @@ static const struct dev_pm_ops ufshcd_pci_pm_ops = {
>>>
>>> static const struct pci_device_id ufshcd_pci_tbl[] = {
>>> { PCI_VENDOR_ID_SAMSUNG, 0xC00C, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
>>> + { PCI_VENDOR_ID_SYNOPSYS, 0xB101, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
>>> + { PCI_VENDOR_ID_SYNOPSYS, 0xB102, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
>>
>> Listing these here implies that the devices these lines match are
>> "normal" PCI UFSHCD devices that don't require any special handling
>> whatsoever. Is that correct?
>
> Yes they are normal PCI UFSHCD devices.
>
>>
>> If they do require special handling, then you need to put them in a
>> separate driver, e.g. ufs-dwc-pci.c
>
> Both ufs-dwc and pci driver must execute the same init sequence to correctly
> kick-off the hardware. That's why the common code is in the ufshcd.c.
> Maybe create a ufshcd-dwc-quirks.c with the dwc specififc code would be
> better.
> This way it could be used by ufs-dwc platform driver and pci.
> Since dwc hardware uses a specific init routine would it be better to have a
> ufs-dwc-pci and ufs-dwc calling the dwc specific init routine?
What you suggested below, i.e. having a ufshcd-dwc.c file containing
the common stuff then having separate platform and PCI drivers sounds
like the best option.
>>> { } /* terminate list */
>>> };
>>>
>>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>>> index 85cd256..05d309d 100644
>>> --- a/drivers/scsi/ufs/ufshcd.c
>>> +++ b/drivers/scsi/ufs/ufshcd.c
>>> @@ -5521,6 +5541,790 @@ static struct devfreq_dev_profile
>>> ufs_devfreq_profile = {
>>> .get_dev_status = ufshcd_devfreq_get_dev_status,
>>> };
>>>
>>> +#ifdef CONFIG_SCSI_UFS_DWC_HOOKS
>>
>> This doesn't look right.
>>
>> The driver should be structured like this:
>>
>> - ufs-dwc: contains everything that is specific to your hardware.
>> - ufshcd: contains everything that is common to multiple types of
>> ufshcd from different vendors
>>
>> Your "hooks" here look like they're doing stuff that is specific to
>> the Designware hardware. They should not be in this file as it's for
>> hardware type independent code.
>>
>> If you need to do something special at some point in the common code,
>> then this should be exposed as an op in struct ufs_hba_variant_ops
>> which is then implemented in your device-specific code.
>
> Yes I agree that maybe the ufs core drive is not the perfect spot for specific
> vendor code. But DWC HW can be using pci or platform and so it has to share
> common code and that's why I put it in the ufshcd.
>
> I think creating a ufshcd-dwc.c would be better to share code between ufs-dwc
> and ufs-dwc-pci. Agree?
Agreed.
>>> +/**
>>> + * ufshcd_dwc_program_clk_div()
>>> + * This function programs the clk divider value. This value is needed to
>>> + * provide 1 microsecond tick to unipro layer.
>>> + * @hba: Private Structure pointer
>>> + * @divider_val: clock divider value to be programmed
>>> + *
>>> + */
>>> +void ufshcd_dwc_program_clk_div(struct ufs_hba *hba, u32 divider_val)
>>> +{
>>> + ufshcd_writel(hba, divider_val, DWC_UFS_REG_HCLKDIV);
>>> +}
>>> +
>>> +/**
>>> + * ufshcd_dwc_link_is_up()
>>> + * Check if link is up
>>> + * @hba: private structure poitner
>>> + *
>>> + * Returns 0 on success, non-zero value on failure
>>> + */
>>> +int ufshcd_dwc_link_is_up(struct ufs_hba *hba)
>>> +{
>>> + int dme_result = 0;
>>> +
>>> + ufshcd_dme_get(hba, UIC_ARG_MIB(VS_POWERSTATE), &dme_result);
>>> +
>>> + if (dme_result == UFSHCD_LINK_IS_UP) {
>>> + ufshcd_set_link_active(hba);
>>> + return 0;
>>> + }
>>> +
>>> + return 1;
>>> +}
>>> +
>>> +/**
>>> + * ufshcd_dwc_connection_setup()
>>> + * This function configures both the local side (host) and the peer side
>>> + * (device) unipro attributes to establish the connection to application/
>>> + * cport.
>>> + * This function is not required if the hardware is properly configured to
>>> + * have this connection setup on reset. But invoking this function does no
>>> + * harm and should be fine even working with any ufs device.
>>> + *
>>> + * @hba: pointer to drivers private data
>>> + *
>>> + * Returns 0 on success non-zero value on failure
>>> + */
>>> +int ufshcd_dwc_connection_setup(struct ufs_hba *hba)
>>> +{
>>> + int ret = 0;
>>> +
>>> + /* Local side Configuration */
>>> + ret = ufshcd_dme_set(hba, UIC_ARG_MIB(T_CONNECTIONSTATE), 0);
>>> + if (ret)
>>> + goto out;
>>
Re: [PATCH-v3 4/5] target: Fix remote-port TMR ABORT + se_cmd fabric stop
> + bool aborted = false, tas = false;
>
> /*
>* Allocate an struct iscsi_conn_recovery for this connection.
> @@ -398,7 +399,7 @@ int iscsit_prepare_cmds_for_realligance(struct iscsi_conn
> *conn)
>
> iscsit_free_all_datain_reqs(cmd);
>
> - transport_wait_for_tasks(&cmd->se_cmd);
> + transport_wait_for_tasks(&cmd->se_cmd, false, &aborted, &tas);
please keep the existing transport_wait_for_tasks prototype and factor
out a new helper for the transport_generic_free_cmd special case to avoid
all this boiler plate.
> + ret = kref_get_unless_zero(&se_cmd->cmd_kref);
> + if (ret)
> + se_cmd->cmd_wait_set = 1;
I don't like the dual use of cmd_wait_set and the combination
with CMD_T_FABRIC_STOP.
How about the following alternative:
pull in my prep patch to rewrite target_sess_cmd_list_set_waiting so
that it doesn't need to iterate over all commands and set cmd_wait_set:
http://www.spinics.net/lists/target-devel/msg11446.html
This gives us a free hand use a different completion for per-command
waiting. Now your new usage of cmd_wait_set can be simplified as we
don't need a CMD_T_FABRIC_STOP to undo it, it can simply be cleared.
While we're at it I'd suggest using a CMD_T_ flag instead of a bitfield
for it.
> if (!(cmd->se_cmd_flags & SCF_SE_LUN_CMD)) {
> if (wait_for_tasks && (cmd->se_cmd_flags & SCF_SCSI_TMR_CDB))
> - transport_wait_for_tasks(cmd);
> + transport_wait_for_tasks(cmd, true, &aborted, &tas);
>
> - ret = transport_put_cmd(cmd);
> + if (!aborted || tas)
> + ret = transport_put_cmd(cmd);
> } else {
> if (wait_for_tasks)
> - transport_wait_for_tasks(cmd);
> + transport_wait_for_tasks(cmd, true, &aborted, &tas);
> /*
>* Handle WRITE failure case where transport_generic_new_cmd()
>* has already added se_cmd to state_list, but fabric has
> @@ -2454,9 +2456,21 @@ int transport_generic_free_cmd(struct se_cmd *cmd, int
> wait_for_tasks)
> if (cmd->se_lun)
> transport_lun_remove_cmd(cmd);
>
> - ret = transport_put_cmd(cmd);
> + if (!aborted || tas)
> + ret = transport_put_cmd(cmd);
> }
FYI, this can be simplified a bit more.
Call transport_wait_for_tasks
if (wait_for_tasks &&
(cmd->se_cmd_flags & (SCF_SE_LUN_CMD | SCF_SCSI_TMR_CDB))) {
and then the
if (!aborted || tas)
et = transport_put_cmd(cmd);
can be moved behind the conditional for LUN_CMDs as well.
> - return ret;
> + /*
> + * If the task has been internally aborted due to TMR ABORT_TASK
> + * or LUN_RESET, target_core_tmr.c is responsible for performing
> + * the remaining calls to target_put_sess_cmd(), and not the
> + * callers of this function.
> + */
> + if (aborted) {
> + pr_debug("Detected CMD_T_ABORTED for ITT: %llu\n", cmd->tag);
> + wait_for_completion(&cmd->cmd_wait_comp);
> + cmd->se_tfo->release_cmd(cmd);
> + }
> + return (aborted) ? 1 : ret;
This could be simplified to:
if (aborted) {
...
ret = 1;
}
return ret;
> + if (cmd->transport_state & CMD_T_ABORTED)
> + *aborted = true;
> + else
> + *aborted = false;
> +
> + if (cmd->transport_state & CMD_T_TAS)
> + *tas = true;
> + else
> + *tas = false;
All callers already initialize these two to false, so the else
branches seem superflous.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add support for DWC UFS Host Controller
Hi Julian,
Thanks for the review. My comments are below.
On 2/2/2016 1:00 AM, Julian Calaby wrote:
> Hi Joao,
>
> On Mon, Feb 1, 2016 at 11:47 PM, Joao Pinto wrote:
>> This patch includes:
>> - quirks in the ufs core driver to support Synopsys MPHY Test Chip config
>> - quirks in the ufs core driver to support DWC configuration sequence
>> - New Unipro attributes were added
>> - ufs core driver was tweaked to support UFS 2.0
>> - support for Synopsys PCI ID in the pci glue driver
>> - new platform glue driver for Synopsys devices
>>
>> Signed-off-by: Joao Pinto
>> ---
>> Documentation/devicetree/bindings/ufs/ufs-dwc.txt | 16 +
>> drivers/scsi/ufs/Kconfig | 54 ++
>> drivers/scsi/ufs/Makefile | 1 +
>> drivers/scsi/ufs/ufs-dwc.c| 115 +++
>> drivers/scsi/ufs/ufshcd-pci.c | 2 +
>> drivers/scsi/ufs/ufshcd-pltfrm.c | 2 +-
>> drivers/scsi/ufs/ufshcd.c | 822
>> +-
>> drivers/scsi/ufs/ufshcd.h | 15 +
>> drivers/scsi/ufs/ufshci.h | 29 +
>> drivers/scsi/ufs/unipro.h | 39 +
>> 10 files changed, 1089 insertions(+), 6 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/ufs/ufs-dwc.txt
>> create mode 100644 drivers/scsi/ufs/ufs-dwc.c
>
> You should separate out your changes into separate patches, e.g.
Ok, I will do that in a 2nd version.
>
> 1. Fix the spelling mistake
> 2. Update the base code for UFSHCI_VERSION_20 compatiblity.
> 3. Implement any common code as a separate module
> 4. Add the OF and PCI drivers
>
>> diff --git a/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
>> b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
>> new file mode 100644
>> index 000..fa361f2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
>> @@ -0,0 +1,16 @@
>> +* Universal Flash Storage (UFS) DesignWare Host Controller
>> +
>> +DWC_UFSHC nodes are defined to describe on-chip UFS host controllers.
>> +Each UFS controller instance should have its own node.
>> +
>> +Required properties:
>> +- compatible: compatible list, contains "snps,ufshcd"
>> +- reg :
>> +- interrupts:
>> +
>> +Example:
>> + dwc_ufshcd@0xD000 {
>> + compatible = "snps,ufshcd";
>> + reg = < 0xD000 0x1 >;
>> + interrupts = < 24 >;
>> + };
>
> If I recall correctly, when adding device tree bindings, you need to
> cc the devicetree list, devicet...@vger.kernel.org.
I forgot to include them! :)
>
>> diff --git a/drivers/scsi/ufs/ufs-dwc.c b/drivers/scsi/ufs/ufs-dwc.c
>> new file mode 100644
>> index 000..e4d70b7
>> --- /dev/null
>> +++ b/drivers/scsi/ufs/ufs-dwc.c
>> @@ -0,0 +1,115 @@
>> +/*
>> ==
>> + * The Synopsys DWC UFS Software Driver and documentation (hereinafter
>> + * "Software") is an unsupported proprietary work of Synopsys, Inc. unless
>> + * otherwise expressly agreed to in writing between Synopsys and you.
>> + *
>> + * The Software IS NOT an item of Licensed Software or Licensed Product
>> under
>> + * any End User Software License Agreement or Agreement for Licensed Product
>> + * with Synopsys or any supplement thereto. Permission is hereby granted,
>> + * free of charge, to any person obtaining a copy of this software annotated
>> + * with this license and the Software, to deal in the Software without
>> + * restriction, including without limitation the rights to use, copy,
>> modify,
>> + * merge, publish, distribute, sublicense, and/or sell copies of the
>> Software,
>> + * and to permit persons to whom the Software is furnished to do so, subject
>> + * to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included
>> in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THIS SOFTWARE IS BEING DISTRIBUTED BY SYNOPSYS SOLELY ON AN "AS IS" BASIS
>> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>> PURPOSE
>> + * ARE HEREBY DISCLAIMED. IN NO EVENT SHALL SYNOPSYS BE LIABLE FOR ANY
>> DIRECT,
>> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
>> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
>> + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
>> HOWEVER
>> + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
>> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
>> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>> SUCH
>> + * DAMAGE.
>> + *
>> ==
>
> Is this GPLv2 compatible?
I s
Re: [PATCH 0/2] scsi: Fix endless loop of ATA hard resets due to VPD reads
On Wed, Jan 20, 2016 at 10:35:15PM -0800, Alexander Duyck wrote:
> Ultimately neither of these bugs were my root cause. It turns out the
> Marvel Console SCSI device in my system needed to have a flag set to
> disable VPD access in order to keep things from looping through the error
> repeatedly. In order to resolve it I had to add the kernel parameter
> "scsi_mod.dev_flags=Marvell:Console:0x400". This allowed my system to
> boot without any errors, however the first two issues described above are
> still relevent so I thought I would provide the patches since I had already
> written them up.
I have the same problem.
Shouldn't we put quirk for that?
>From d5ad5e1ee4128c454f39d7f3ccaa0b202e0e8534 Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov"
Date: Tue, 2 Feb 2016 12:44:04 +0300
Subject: [PATCH] scsi: add Marvell Console to the ignore VPD pages blacklist
With current upstream, I see these messages in loop.
ata14: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
ata14.00: configured for UDMA/66
ata14: EH complete
ata14.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6
ata14.00: irq_stat 0x4001
ata14.00: cmd a0/01:00:00:00:01/00:00:00:00:00/a0 tag 22 dma 16640 in
Inquiry 12 01 00 00 ff 00res 00/00:00:00:00:00/00:00:00:00:00/00 Emask
0x3 (HSM violation)
ata14: hard resetting link
Looks like we should blacklist the device to stop it.
Signed-off-by: Kirill A. Shutemov
---
drivers/scsi/scsi_devinfo.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index 47b9d13f97b8..da2e068ee47d 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -205,6 +205,7 @@ static struct {
{"Intel", "Multi-Flex", NULL, BLIST_NO_RSOC},
{"iRiver", "iFP Mass Driver", NULL, BLIST_NOT_LOCKABLE |
BLIST_INQUIRY_36},
{"LASOUND", "CDX7405", "3.10", BLIST_MAX5LUN | BLIST_SINGLELUN},
+ {"Marvell", "Console", NULL, BLIST_SKIP_VPD_PAGES},
{"MATSHITA", "PD-1", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
{"MATSHITA", "DMC-LC5", NULL, BLIST_NOT_LOCKABLE | BLIST_INQUIRY_36},
{"MATSHITA", "DMC-LC40", NULL, BLIST_NOT_LOCKABLE | BLIST_INQUIRY_36},
--
Kirill A. Shutemov
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v7 02/14] scsi: ufs: clear fields UTRD, UPIU req and rsp before new transfers
> Some of the data structures (like response UPIU) and/or its elements > (unused fields) should be cleared before sending out the respective > command to UFS device. > > This change clears the UPIU response data structure for query commands > and NOP command before sending out the command. We also initialize the > PRDT table length to zero which should take care of commands which doesn't > have any data associated with it. We are also clearing the unused fields in > request UPIU for NOP command. > > Reviewed-by: Gilad Broner > Reviewed-by: Dolev Raviv > Signed-off-by: Subhash Jadavani > Signed-off-by: Yaniv Gardi You can add my ack and/or tested by (we have similar fix running in our code base for a long time :)) Thanks Tomas -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 111731] New: mpt2sas doesn't get built
https://bugzilla.kernel.org/show_bug.cgi?id=111731 Bug ID: 111731 Summary: mpt2sas doesn't get built Product: SCSI Drivers Version: 2.5 Kernel Version: 4.4.0 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Other Assignee: scsi_drivers-ot...@kernel-bugs.osdl.org Reporter: n...@li.nux.ro Regression: No Hello, It looks like 4.4.0 is not building mpt2sas modules any more. We got bit by this while testing it (via ElRepo) on EL7. Even though specified in the config, only mpt3sas is built. While mpt3sas can indeed support the hardware in question (DELL/LSI H200 in my case), dracut/mkinitrd fails to include it in the initrd, because it's looking for mpt2sas which is missing from /lib/modules altogether. `dracut --add-drivers` solves the problem, but this is not a graceful kernel upgrade and ends up with unbootable OS. More information about this issue here http://elrepo.org/bugs/view.php?id=628 -- You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v3 5/5] target: Drop legacy se_cmd->task_stop_comp + REQUEST_STOP usage
Looks fine, Reviewed-by: Christoph Hellwig -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v3 3/5] target: Fix TAS handling for multi-session se_node_acls
Looks fine, Reviewed-by: Christoph Hellwig -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v3 2/5] target: Fix LUN_RESET active TMR descriptor handling
> + if (WARN_ON_ONCE(!sess))
> + continue;
> +
> + spin_lock(&sess->sess_cmd_lock);
> spin_lock(&cmd->t_state_lock);
> if (!(cmd->transport_state & CMD_T_ACTIVE)) {
> spin_unlock(&cmd->t_state_lock);
> + spin_unlock(&sess->sess_cmd_lock);
> continue;
> }
> if (cmd->t_state == TRANSPORT_ISTATE_PROCESSING) {
> spin_unlock(&cmd->t_state_lock);
> + spin_unlock(&sess->sess_cmd_lock);
> continue;
> }
> + cmd->transport_state |= CMD_T_ABORTED;
> spin_unlock(&cmd->t_state_lock);
>
> + rc = kref_get_unless_zero(&cmd->cmd_kref);
> + spin_unlock(&sess->sess_cmd_lock);
Same here - what is the point of sess_cmd_lock?
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v3 1/5] target: Fix LUN_RESET active I/O handling for ACK_KREF
> @@ -282,6 +305,16 @@ static void core_tmr_drain_state_list( > if (prout_cmd == cmd) > continue; > > + sess = cmd->se_sess; > + if (WARN_ON_ONCE(!sess)) > + continue; > + > + spin_lock(&sess->sess_cmd_lock); > + rc = __target_check_io_state(cmd); > + spin_unlock(&sess->sess_cmd_lock); > + if (!rc) > + continue; > + I still don't understand why we care about the session or sess_cmd_lock here. There isn't anything in __target_check_io_state that relies on them (except for the assert checking the lock). -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4 15/23] scsi_dh_alua: Add new blacklist flag 'BLIST_SYNC_ALUA'
Looks fine, Reviewed-by: Christoph Hellwig -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4 13/23] scsi_dh_alua: Use workqueue for RTPG
> static void release_port_group(struct kref *kref)
> {
> struct alua_port_group *pg;
>
> + synchronize_rcu();
> pg = container_of(kref, struct alua_port_group, kref);
> + if (pg->rtpg_sdev)
> + flush_delayed_work(&pg->rtpg_work);
> spin_lock(&port_group_lock);
> list_del(&pg->node);
> spin_unlock(&port_group_lock);
I don't think this is correct - we need a grace period after the
list_del to prevent new lookups. I suspect the right thing to do here
is to simply use kfree_rcu for the pg. This also avoids waiting for
whole grace periods for each deleted port_group, which might be rather
expensive.
> + /* Check for existing port group references */
> + spin_lock(&h->pg_lock);
> + if (h->pg) {
> + old_pg = pg;
> + if (h->pg != pg) {
> + /* port group has changed. Update to new port group */
> + old_pg = h->pg;
> + rcu_assign_pointer(h->pg, pg);
> + }
> + } else {
> + rcu_assign_pointer(h->pg, pg);
> + }
This looks confusing - pg is the structure we just allocated, so why
do we assign it to old_pg? It seems like the above could be written
similar and more clear as:
olg_pg = h->pg;
if (pg != old_pg)
rcu_assign_pointer(h->pg, pg);
> + if (!sdev) {
> + WARN_ON(pg->flags & ALUA_PG_RUN_RTPG ||
> + pg->flags & ALUA_PG_RUN_STPG);
Please use two separate WARN_ONs here to know which one triggered from
the line in the kernel log.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Separate target visibility from reaped state information
On Mon, Feb 01, 2016 at 08:11:29PM -0500, Martin K. Petersen wrote: > > "Sebastian" == Sebastian Herbszt writes: > > >> The only change from the current workflow is that the DEL transition > >> (now the reaped flag) is done before the spin lock is dropped which > >> would fix a tiny window for two threads both trying to remove the > >> same target, but there's nothing that could possibly fix an iterative > >> soft lockup caused by restarting the loop, which is what the > >> changelog says. > > Sebastian> James, Martin, what's the status of this patch? I still hit > Sebastian> the reported soft lockup on 4.5-rc1. > > And you have verified that Bart's patch applied on top of 4.5-rc1 still > fixes the lockup? (I know you tested a previous version) > I had an off list discussion/problem report from Dick Kennedy, pointed him to this very patch and he verified it solved his problem (a lockup like reported by Sebastian). I'm not sure if he tested v4.4.X or v4.5-rcX though. > I am concerned about queuing something as a stable fix if it is just > masking a fundamental underlying problem. James' comment suggests that > there is something else going on. > > -- > Martin K. PetersenOracle Linux Engineering -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 98171] [Regression] Marvell SE91xx SATA 3 controllers not recognized correctly
https://bugzilla.kernel.org/show_bug.cgi?id=98171 jrick...@myamigos.us changed: What|Removed |Added CC||jrick...@myamigos.us --- Comment #6 from jrick...@myamigos.us --- I see almost the exact same error across multiple (qty 4, all the same HW configs) Supermicro X10SBA motherboards that I have. Jan 30 06:52:52 tank2 kernel: [1.689428] ata12.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 Jan 30 06:52:52 tank2 kernel: [1.689430] ata12.00: irq_stat 0x4001 Jan 30 06:52:52 tank2 kernel: [1.689440] ata12.00: cmd a0/01:00:00:00:01/00:00:00:00:00/a0 tag 2 dma 16640 in Jan 30 06:52:52 tank2 kernel: [1.689440] opcode=0x12 12 01 00 00 ff 00res 00/00:00:00:00:00/00:00:00:00:00/00 Emask 0x3 (HSM violation) Jan 30 06:52:52 tank2 kernel: [1.689446] ata12: hard resetting link These motherboards use the Marvell 9230 chip to add 4 SATA ports to the 2 SATA ports provided by the BayTrail SoC. I can sometimes recreate this error when placing the filesystem under heavy load. In this case it is a multi-terabyte ZFS "tank" and "zpool" is doing a "scrub". Gentoo Linux running gentoo-sources-4.1.15-r1 I do not remember when I first started to see this error appear. -- You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
