[patch RESEND] atp870u: 64 bit bug in atp885_init()
On 64 bit CPUs there is a memory corruption bug on probe(). It should
be a u32 pointer instead of an unsigned long pointer or we write past
the end of the setupdata[] array.
Signed-off-by: Dan Carpenter
Reviewed-by: Hannes Reinecke
---
Resending because we have shuffled the code around so the patch needed
to be refreshed against linux-next. Although I do wonder why we are
still working on this code since it has never worked on 64 bit systems
so probably all the users gave up a decade ago.
diff --git a/drivers/scsi/atp870u.c b/drivers/scsi/atp870u.c
index 8b52a9d..b46997c 100644
--- a/drivers/scsi/atp870u.c
+++ b/drivers/scsi/atp870u.c
@@ -1413,11 +1413,11 @@ static void atp885_init(struct Scsi_Host *shpnt)
atpdev->global_map[m] = 0;
for (k = 0; k < 4; k++) {
atp_writew_base(atpdev, 0x3c, n++);
- ((unsigned long *)&setupdata[m][0])[k] =
atp_readl_base(atpdev, 0x38);
+ ((u32 *)&setupdata[m][0])[k] =
atp_readl_base(atpdev, 0x38);
}
for (k = 0; k < 4; k++) {
atp_writew_base(atpdev, 0x3c, n++);
- ((unsigned long *)&atpdev->sp[m][0])[k] =
atp_readl_base(atpdev, 0x38);
+ ((u32 *)&atpdev->sp[m][0])[k] =
atp_readl_base(atpdev, 0x38);
}
n += 8;
}
--
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] hisi_sas: fix error codes in hisi_sas_task_prep()
There were a couple cases where the error codes weren't set and also I
changed the success return to "return 0;" which is the same as
"return rc;" but more explicit.
Fixes: 42e7a69368a5 ('hisi_sas: Add ssp command functio')
Signed-off-by: Dan Carpenter
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 2929018..99b1950 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -208,15 +208,19 @@ static int hisi_sas_task_prep(struct sas_task *task,
struct hisi_hba *hisi_hba,
slot->status_buffer = dma_pool_alloc(hisi_hba->status_buffer_pool,
GFP_ATOMIC,
&slot->status_buffer_dma);
- if (!slot->status_buffer)
+ if (!slot->status_buffer) {
+ rc = -ENOMEM;
goto err_out_slot_buf;
+ }
memset(slot->status_buffer, 0, HISI_SAS_STATUS_BUF_SZ);
slot->command_table = dma_pool_alloc(hisi_hba->command_table_pool,
GFP_ATOMIC,
&slot->command_table_dma);
- if (!slot->command_table)
+ if (!slot->command_table) {
+ rc = -ENOMEM;
goto err_out_status_buf;
+ }
memset(slot->command_table, 0, HISI_SAS_COMMAND_TABLE_SZ);
memset(slot->cmd_hdr, 0, sizeof(struct hisi_sas_cmd_hdr));
@@ -254,7 +258,7 @@ static int hisi_sas_task_prep(struct sas_task *task, struct
hisi_hba *hisi_hba,
sas_dev->running_req++;
++(*pass);
- return rc;
+ return 0;
err_out_sge:
dma_pool_free(hisi_hba->sge_page_pool, slot->sge_page,
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 78/71] ncr5380: Add support for HP 53C400A-based cards (C2502)
On Tuesday 08 December 2015 12:40:18 Finn Thain wrote:
>
> On Tue, 8 Dec 2015, Ondrej Zary wrote:
>
> > HP C2502 cards (based on 53C400A chips) use different magic numbers for
> > software-based I/O address configuration than other cards. The
> > configuration is also extended to allow setting the IRQ.
> >
> > Move the configuration to a new function magic_configure() and move
> > magic the magic numbers into an array. Add new magic numbers for these
> > HP cards and hp_53c400a module parameter to use them.
> >
> > Tested with HP C2502 and DTCT-436P.
> >
> > Signed-off-by: Ondrej Zary
> > ---
> > drivers/scsi/g_NCR5380.c | 76
> > --
> > drivers/scsi/g_NCR5380.h |1 +
> > 2 files changed, 61 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
> > index 42fdeaf..121d143 100644
> > --- a/drivers/scsi/g_NCR5380.c
> > +++ b/drivers/scsi/g_NCR5380.c
> > @@ -80,6 +80,7 @@ static int ncr_5380;
> > static int ncr_53c400;
> > static int ncr_53c400a;
> > static int dtc_3181e;
> > +static int hp_53c400a;
>
> From the photos that I've seen, the HP cards have either NCR or SYMBIOS
> 53C400A devices.
>
> You've added a new setup option and a new board type, but the name you've
> chosen is neither the board type nor the device type. It is a combination.
Because it's a HP board with NCR/SYMBIOS 53C400A chip. Windows driver calls
it "Symbios Logic 53C400A (HP Version)", suggesting that there might be more
compatible cards.
But search did not found any on the web, only C2502. So I'll rename it to
hp_c2502.
> >
> > static struct override {
> > NCR5380_map_type NCR5380_map_name;
> > @@ -225,6 +226,32 @@ static int __init do_DTC3181E_setup(char *str)
> >
> > #endif
> >
> > +#ifndef SCSI_G_NCR5380_MEM
> > +/*
> > + * Configure I/O address of 53C400A or DTC 3181 by writing magic numbers
> > + * to ports 0x779 and 0x379. Two magic number sequences are known:
> > + * 1. for generic NCR 53C400A-based cards and DTC436 chips
> > + * 2. for HP C2502 card (also based on 53C400A but different decode logic)
>
> Forgive my ignorance of ISA card design, but that suggests that these
> magic numbers are only relevant to HP C2502 boards and not SYM 53C400A
> devices in general (?) ...
Looks like the original magic numbers (they came from the outb() calls, now
in ncr_53c400a_magic[]) are hardcoded in the 53C400A chip (and also DTC
chips).
HP C2502 card has some PALCE chips between the ISA bus and 53C400A. They
probably react to the HP-specific magic numbers (and don't pass the
"original" magic numbers to the 53C400A chip).
> > + */
> > +static void magic_configure(int idx, u8 irq, u8 magic[])
> > +{
> > + u8 cfg = 0;
> > +
> > + outb(magic[0], 0x779);
> > + outb(magic[1], 0x379);
> > + outb(magic[2], 0x379);
> > + outb(magic[3], 0x379);
> > + outb(magic[4], 0x379);
> > +
> > + /* allowed IRQs for HP 53C400A */
> > + if (irq != 2 && irq != 3 && irq != 4 && irq != 5 && irq != 7)
> > + irq = 0;
> > + if (idx >= 0 && idx <= 7)
> > + cfg = 0x80 | idx | (irq << 4);
> > + outb(cfg, 0x379);
> > +}
> > +#endif
> > +
> > /**
> > * generic_NCR5380_detect - look for NCR5380 controllers
> > * @tpnt: the scsi template
> > @@ -241,8 +268,9 @@ static int __init generic_NCR5380_detect(struct
> > scsi_host_template *tpnt)
> > static int current_override;
> > int count;
> > unsigned int *ports;
> > + u8 *magic;
> > #ifndef SCSI_G_NCR5380_MEM
> > - int i;
> > + int i, port_idx;
> > unsigned long region_size = 16;
> > #endif
> > static unsigned int __initdata ncr_53c400a_ports[] = {
> > @@ -251,6 +279,12 @@ static int __init generic_NCR5380_detect(struct
> > scsi_host_template *tpnt)
> > static unsigned int __initdata dtc_3181e_ports[] = {
> > 0x220, 0x240, 0x280, 0x2a0, 0x2c0, 0x300, 0x320, 0x340, 0
> > };
> > + static u8 ncr_53c400a_magic[] __initdata = {/* 53C400A & DTC 3181 */
> > + 0x59, 0xb9, 0xc5, 0xae, 0xa6
> > + };
> > + static u8 hp_53c400a_magic[] __initdata = { /* HP C2502 */
> > + 0x0f, 0x22, 0xf0, 0x20, 0x80
> > + };
>
> ... so maybe that should be called 'hp_c2502_magic'?
Yes.
> > int flags;
> > struct Scsi_Host *instance;
> > struct NCR5380_hostdata *hostdata;
> > @@ -273,6 +307,8 @@ static int __init generic_NCR5380_detect(struct
> > scsi_host_template *tpnt)
> > overrides[0].board = BOARD_NCR53C400A;
> > else if (dtc_3181e)
> > overrides[0].board = BOARD_DTC3181E;
> > + else if (hp_53c400a)
> > + overrides[0].board = BOARD_HP53C400A;
> > #ifndef SCSI_G_NCR5380_MEM
> > if (!current_override && isapnp_present()) {
> > struct pnp_dev *dev = NULL;
> > @@ -326,10 +362,17 @@ static int __init generic_NCR5380_detect(struct
> > scsi_host_template *tpnt)
> > case BOARD_NCR53C400A:
> >
Re: [PATCH] scsi: sim710: fix build warning
On Tue, Sep 15, 2015 at 01:30:00PM +0530, Sudip Mukherjee wrote: > On Fri, Sep 04, 2015 at 07:40:29PM +0530, Sudip Mukherjee wrote: > > We were getting build warning about: > > "Section mismatch in reference from the variable sim710_eisa_driver to > > the function .init.text:sim710_eisa_probe() > > The variable sim710_eisa_driver references the function __init > > sim710_eisa_probe()" > > > > sim710_eisa_probe() was having __init but that was being referenced from > > sim710_eisa_driver. > > > > Signed-off-by: Sudip Mukherjee > > --- > A gentle ping. > I still get the build warning while building for i386 allmodconfig with > next-20150915. > Build is at https://travis-ci.org/sudipm-mukherjee/parport/jobs/80365417 Another gentle ping. Build warning with i386 allmodconfig is still there with next-20151209. Build log is at: https://travis-ci.org/sudipm-mukherjee/parport/jobs/95746184 regards sudip -- 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 RESEND] atp870u: 64 bit bug in atp885_init()
On Wednesday 09 December 2015 12:53:39 One Thousand Gnomes wrote: > On Wed, 9 Dec 2015 13:24:53 +0300 > Dan Carpenter wrote: > > > On 64 bit CPUs there is a memory corruption bug on probe(). It should > > be a u32 pointer instead of an unsigned long pointer or we write past > > the end of the setupdata[] array. > > > > Signed-off-by: Dan Carpenter > > Reviewed-by: Hannes Reinecke > > --- > > Resending because we have shuffled the code around so the patch needed > > to be refreshed against linux-next. Although I do wonder why we are > > still working on this code since it has never worked on 64 bit systems > > so probably all the users gave up a decade ago. > > So this is untested ? If so please make it very clear in the commit > message because the kernel is IMHO getting too full of polished, neat, > recently modified, never tested, never used code. > > I agree it would be better if the driver was simply deleted. I've not > even seen an ATP870 bug report in years. Maybe because it worked. Although the code was horrible. I've done some big changes to this driver recently (tested, of course). I can't test this patch as I don't have ATP885 card, only ATP870. -- Ondrej Zary -- 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 v3 78/71] ncr5380: Add support for HP 53C400A-based cards (C2502)
HP C2502 cards (based on 53C400A chips) use different magic numbers for
software-based I/O address configuration than other cards.
The configuration is also extended to allow setting the IRQ.
Move the configuration to a new function magic_configure() and move
magic the magic numbers into an array. Add new magic numbers for these
HP cards and hp_c2502 module parameter to use them, e.g.:
modprobe g_NCR5380 ncr_irq=7 ncr_addr=0x280 ncr_53c400a=1 hp_c2502=1
Tested with HP C2502 and DTCT-436P.
Signed-off-by: Ondrej Zary
---
drivers/scsi/g_NCR5380.c | 74 --
1 file changed, 58 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
index 42fdeaf..dd32c68 100644
--- a/drivers/scsi/g_NCR5380.c
+++ b/drivers/scsi/g_NCR5380.c
@@ -80,12 +80,14 @@ static int ncr_5380;
static int ncr_53c400;
static int ncr_53c400a;
static int dtc_3181e;
+static int hp_c2502;
static struct override {
NCR5380_map_type NCR5380_map_name;
int irq;
int dma;
int board; /* Use NCR53c400, Ricoh, etc. extensions ? */
+ bool hp_c2502;
} overrides
#ifdef GENERIC_NCR5380_OVERRIDE
[] __initdata = GENERIC_NCR5380_OVERRIDE;
@@ -225,6 +227,32 @@ static int __init do_DTC3181E_setup(char *str)
#endif
+#ifndef SCSI_G_NCR5380_MEM
+/*
+ * Configure I/O address of 53C400A or DTC436 by writing magic numbers
+ * to ports 0x779 and 0x379. Two magic number sequences are known:
+ * 1. for generic NCR 53C400A-based cards and DTC436 chips
+ * 2. for HP C2502 card (also based on 53C400A but different decode logic)
+ */
+static void magic_configure(int idx, u8 irq, u8 magic[])
+{
+ u8 cfg = 0;
+
+ outb(magic[0], 0x779);
+ outb(magic[1], 0x379);
+ outb(magic[2], 0x379);
+ outb(magic[3], 0x379);
+ outb(magic[4], 0x379);
+
+ /* allowed IRQs for HP 53C400A */
+ if (irq != 2 && irq != 3 && irq != 4 && irq != 5 && irq != 7)
+ irq = 0;
+ if (idx >= 0 && idx <= 7)
+ cfg = 0x80 | idx | (irq << 4);
+ outb(cfg, 0x379);
+}
+#endif
+
/**
* generic_NCR5380_detect - look for NCR5380 controllers
* @tpnt: the scsi template
@@ -241,8 +269,9 @@ static int __init generic_NCR5380_detect(struct
scsi_host_template *tpnt)
static int current_override;
int count;
unsigned int *ports;
+ u8 *magic;
#ifndef SCSI_G_NCR5380_MEM
- int i;
+ int i, port_idx;
unsigned long region_size = 16;
#endif
static unsigned int __initdata ncr_53c400a_ports[] = {
@@ -251,6 +280,12 @@ static int __init generic_NCR5380_detect(struct
scsi_host_template *tpnt)
static unsigned int __initdata dtc_3181e_ports[] = {
0x220, 0x240, 0x280, 0x2a0, 0x2c0, 0x300, 0x320, 0x340, 0
};
+ static u8 ncr_53c400a_magic[] __initdata = {/* 53C400A & DTC436 */
+ 0x59, 0xb9, 0xc5, 0xae, 0xa6
+ };
+ static u8 hp_c2502_magic[] __initdata = { /* HP C2502 */
+ 0x0f, 0x22, 0xf0, 0x20, 0x80
+ };
int flags;
struct Scsi_Host *instance;
struct NCR5380_hostdata *hostdata;
@@ -273,6 +308,8 @@ static int __init generic_NCR5380_detect(struct
scsi_host_template *tpnt)
overrides[0].board = BOARD_NCR53C400A;
else if (dtc_3181e)
overrides[0].board = BOARD_DTC3181E;
+ if (hp_c2502)
+ overrides[0].hp_c2502 = true;
#ifndef SCSI_G_NCR5380_MEM
if (!current_override && isapnp_present()) {
struct pnp_dev *dev = NULL;
@@ -326,24 +363,25 @@ static int __init generic_NCR5380_detect(struct
scsi_host_template *tpnt)
case BOARD_NCR53C400A:
flags = FLAG_NO_DMA_FIXUP;
ports = ncr_53c400a_ports;
+ magic = ncr_53c400a_magic;
break;
case BOARD_DTC3181E:
flags = FLAG_NO_DMA_FIXUP;
ports = dtc_3181e_ports;
+ magic = ncr_53c400a_magic;
break;
}
+ if (overrides[current_override].hp_c2502)
+ magic = hp_c2502_magic;
+ else
+ magic = ncr_53c400a_magic;
#ifndef SCSI_G_NCR5380_MEM
if (ports) {
/* wakeup sequence for the NCR53C400A and DTC3181E */
/* Disable the adapter and look for a free io port */
- outb(0x59, 0x779);
- outb(0xb9, 0x379);
- outb(0xc5, 0x379);
- outb(0xae, 0x379);
- outb(0xa6, 0x379);
- outb(0x00, 0x379);
+ magic_configure(-1, 0, magic);
if (overrides[current_override].NCR5380_map_name !=
PORT
Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
On Wed, 9 Dec 2015 13:24:53 +0300 Dan Carpenter wrote: > On 64 bit CPUs there is a memory corruption bug on probe(). It should > be a u32 pointer instead of an unsigned long pointer or we write past > the end of the setupdata[] array. > > Signed-off-by: Dan Carpenter > Reviewed-by: Hannes Reinecke > --- > Resending because we have shuffled the code around so the patch needed > to be refreshed against linux-next. Although I do wonder why we are > still working on this code since it has never worked on 64 bit systems > so probably all the users gave up a decade ago. So this is untested ? If so please make it very clear in the commit message because the kernel is IMHO getting too full of polished, neat, recently modified, never tested, never used code. I agree it would be better if the driver was simply deleted. I've not even seen an ATP870 bug report in years. Alan -- 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 108771] scsi: ses: kasan: ses_enclosure_data_process use after free on boot SAS2X28
https://bugzilla.kernel.org/show_bug.cgi?id=108771 --- Comment #2 from Pavel Tikhomirov --- On 12/08/2015 07:16 PM, James Bottomley wrote: > On Mon, 2015-12-07 at 14:01 +, bugzilla-dae...@bugzilla.kernel.org > wrote: >> https://bugzilla.kernel.org/show_bug.cgi?id=108771 >> >> --- Comment #1 from Pavel Tikhomirov --- >> Aditional info about enclosue(from that node, but older 3.10 based kernel): >> >> [root@p9 crash]# modprobe sg >> [root@p9 crash]# sg_map -i >> /dev/sg0 LSI SAS2X28 0e12 >> /dev/sg1 /dev/sda LSI MR9260-4i 2.13 >> [root@p9 crash]# lsscsi -gs >> [1:0:16:0] enclosu LSI SAS2X28 0e12 - /dev/sg0 >> - >> [1:2:0:0]diskLSI MR9260-4i2.13 /dev/sda /dev/sg1 >> 3.99TB >> [root@p9 crash]# sg_ses /dev/sg0 >>LSI SAS2X28 0e12 >> Supported diagnostic pages: >>Supported Diagnostic Pages [sdp] [0x0] >>Configuration (SES) [cf] [0x1] >>Enclosure Status/Control (SES) [ec,es] [0x2] >>Element Descriptor (SES) [ed] [0x7] >>Additional Element Status (SES-2) [aes] [0xa] >>Download Microcode (SES-2) [dm] [0xe] >> [root@p9 crash]# sg_ses /dev/sg1 >>LSI MR9260-4i 2.13 >> disk device (not an enclosure) >> Supported diagnostic pages: > > OK, can you give us the contents of pages 1, 2 and 10 with > > sg_ses --page=1 --hex /dev/sg0 > sg_ses --page=2 --hex /dev/sg0 > sg_ses --page=10 --hex /dev/sg0 > > The version of the kernel you do this on doesn't really matter. Here are these pages: [root@p9 ~]# sg_ses --page=1 --hex /dev/sg0 LSI SAS2X28 0e12 Response in hex from diagnostic page: Configuration (SES) 00 01 00 00 c9 00 00 00 00 11 00 09 2c 50 03 04 80 ...,P... 10 00 a7 1e bf 4c 53 49 20 20 20 20 20 53 41 53 32LSI SAS2 20 58 32 38 20 20 20 20 20 20 20 20 20 30 65 31 32X28 0e12 30 11 22 33 44 55 00 00 00 17 0c 00 0b 04 01 00 13 ."3DU... 40 03 03 00 04 12 02 00 0f 02 02 00 0e 0e 01 00 09 50 18 01 00 0d 19 0e 00 0e 11 02 00 0e 44 72 69 76 Driv 60 65 20 53 6c 6f 74 73 54 65 6d 70 65 72 61 74 75e SlotsTemperatu 70 72 65 20 53 65 6e 73 6f 72 73 46 61 6e 73 56 6fre SensorsFansVo 80 6c 74 61 67 65 20 53 65 6e 73 6f 72 73 50 6f 77ltage SensorsPow 90 65 72 20 53 75 70 70 6c 69 65 73 45 6e 63 6c 6fer SuppliesEnclo a0 73 75 72 65 53 41 53 20 45 78 70 61 6e 64 65 72sureSAS Expander b0 73 53 41 53 20 43 6f 6e 6e 65 63 74 6f 72 73 45sSAS ConnectorsE c0 74 68 65 72 6e 65 74 20 70 6f 72 74 73 thernet ports [root@p9 ~]# sg_ses --page=2 --hex /dev/sg0 LSI SAS2X28 0e12 Response in hex from diagnostic page: Enclosure Status (SES) 00 02 00 00 c0 00 00 00 00 00 00 00 00 05 00 00 00 10 05 00 00 00 01 00 00 00 05 00 00 00 05 00 00 00 20 01 00 00 00 05 00 00 00 05 00 00 00 01 00 00 00 30 05 00 00 00 05 00 00 00 01 00 00 00 00 00 00 00 40 01 00 2c 00 00 00 00 00 05 00 00 50 05 00 00 50 ..,P...P 50 05 00 00 50 00 00 00 00 01 00 01 f9 01 00 04 b3 ...P 60 00 00 00 00 47 80 00 20 47 80 00 20 00 00 00 00G.. G.. 70 01 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 80 01 11 ff 00 01 11 ff 00 01 20 00 00 01 20 00 00. ... .. 90 01 20 00 00 01 20 00 00 01 20 00 00 01 20 00 00. ... ... ... .. a0 01 20 00 00 01 20 00 00 01 20 00 00 01 20 00 00. ... ... ... .. b0 01 20 00 00 01 20 00 00 00 00 00 00 00 00 00 00. ... .. c0 00 00 00 00 [root@p9 ~]# sg_ses --page=10 --hex /dev/sg0 LSI SAS2X28 0e12 Response in hex from diagnostic page: Additional Element Status (SES-2) 00 0a 00 01 fc 00 00 00 00 16 22 00 00 01 00 00 00 .".. 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 20 00 00 00 00 00 00 00 00 00 00 00 00 16 22 00 01 .".. 30 01 00 00 01 00 00 00 00 00 00 00 00 00 00 00 00 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 50 16 22 00 02 01 00 00 02 00 00 00 01 50 03 04 80 ."..P... 60 00 a7 1e bf 50 03 04 80 00 a7 1e ae 00 00 00 00 P... 70 00 00 00 00 16 22 00 03 01 00 00 03 00 00 00 00 .".. 80 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 90 00 00 00 00 00 00 00 00 16 22 00 04 01 00 00 04 .".. a0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 b0 00 00 00 00 00 00 00 00 00 00 00 00 16 22 00 05 .".. c0 01 00 00 05 00 00 00 01 50 03 04 80 00 a7 1e bf P... d0 50 03 04 80 00 a7 1e b1 00 00 00 00 00 00 00 0
Re: [Bug 108771] scsi: ses: kasan: ses_enclosure_data_process use after free on boot SAS2X28
On 12/08/2015 07:16 PM, James Bottomley wrote: On Mon, 2015-12-07 at 14:01 +, bugzilla-dae...@bugzilla.kernel.org wrote: https://bugzilla.kernel.org/show_bug.cgi?id=108771 --- Comment #1 from Pavel Tikhomirov --- Aditional info about enclosue(from that node, but older 3.10 based kernel): [root@p9 crash]# modprobe sg [root@p9 crash]# sg_map -i /dev/sg0 LSI SAS2X28 0e12 /dev/sg1 /dev/sda LSI MR9260-4i 2.13 [root@p9 crash]# lsscsi -gs [1:0:16:0] enclosu LSI SAS2X28 0e12 - /dev/sg0 - [1:2:0:0]diskLSI MR9260-4i2.13 /dev/sda /dev/sg1 3.99TB [root@p9 crash]# sg_ses /dev/sg0 LSI SAS2X28 0e12 Supported diagnostic pages: Supported Diagnostic Pages [sdp] [0x0] Configuration (SES) [cf] [0x1] Enclosure Status/Control (SES) [ec,es] [0x2] Element Descriptor (SES) [ed] [0x7] Additional Element Status (SES-2) [aes] [0xa] Download Microcode (SES-2) [dm] [0xe] [root@p9 crash]# sg_ses /dev/sg1 LSI MR9260-4i 2.13 disk device (not an enclosure) Supported diagnostic pages: OK, can you give us the contents of pages 1, 2 and 10 with sg_ses --page=1 --hex /dev/sg0 sg_ses --page=2 --hex /dev/sg0 sg_ses --page=10 --hex /dev/sg0 The version of the kernel you do this on doesn't really matter. Here are these pages: [root@p9 ~]# sg_ses --page=1 --hex /dev/sg0 LSI SAS2X28 0e12 Response in hex from diagnostic page: Configuration (SES) 00 01 00 00 c9 00 00 00 00 11 00 09 2c 50 03 04 80 ...,P... 10 00 a7 1e bf 4c 53 49 20 20 20 20 20 53 41 53 32LSI SAS2 20 58 32 38 20 20 20 20 20 20 20 20 20 30 65 31 32X28 0e12 30 11 22 33 44 55 00 00 00 17 0c 00 0b 04 01 00 13 ."3DU... 40 03 03 00 04 12 02 00 0f 02 02 00 0e 0e 01 00 09 50 18 01 00 0d 19 0e 00 0e 11 02 00 0e 44 72 69 76 Driv 60 65 20 53 6c 6f 74 73 54 65 6d 70 65 72 61 74 75e SlotsTemperatu 70 72 65 20 53 65 6e 73 6f 72 73 46 61 6e 73 56 6fre SensorsFansVo 80 6c 74 61 67 65 20 53 65 6e 73 6f 72 73 50 6f 77ltage SensorsPow 90 65 72 20 53 75 70 70 6c 69 65 73 45 6e 63 6c 6fer SuppliesEnclo a0 73 75 72 65 53 41 53 20 45 78 70 61 6e 64 65 72sureSAS Expander b0 73 53 41 53 20 43 6f 6e 6e 65 63 74 6f 72 73 45sSAS ConnectorsE c0 74 68 65 72 6e 65 74 20 70 6f 72 74 73 thernet ports [root@p9 ~]# sg_ses --page=2 --hex /dev/sg0 LSI SAS2X28 0e12 Response in hex from diagnostic page: Enclosure Status (SES) 00 02 00 00 c0 00 00 00 00 00 00 00 00 05 00 00 00 10 05 00 00 00 01 00 00 00 05 00 00 00 05 00 00 00 20 01 00 00 00 05 00 00 00 05 00 00 00 01 00 00 00 30 05 00 00 00 05 00 00 00 01 00 00 00 00 00 00 00 40 01 00 2c 00 00 00 00 00 05 00 00 50 05 00 00 50 ..,P...P 50 05 00 00 50 00 00 00 00 01 00 01 f9 01 00 04 b3 ...P 60 00 00 00 00 47 80 00 20 47 80 00 20 00 00 00 00G.. G.. 70 01 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 80 01 11 ff 00 01 11 ff 00 01 20 00 00 01 20 00 00. ... .. 90 01 20 00 00 01 20 00 00 01 20 00 00 01 20 00 00. ... ... ... .. a0 01 20 00 00 01 20 00 00 01 20 00 00 01 20 00 00. ... ... ... .. b0 01 20 00 00 01 20 00 00 00 00 00 00 00 00 00 00. ... .. c0 00 00 00 00 [root@p9 ~]# sg_ses --page=10 --hex /dev/sg0 LSI SAS2X28 0e12 Response in hex from diagnostic page: Additional Element Status (SES-2) 00 0a 00 01 fc 00 00 00 00 16 22 00 00 01 00 00 00 .".. 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 20 00 00 00 00 00 00 00 00 00 00 00 00 16 22 00 01 .".. 30 01 00 00 01 00 00 00 00 00 00 00 00 00 00 00 00 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 50 16 22 00 02 01 00 00 02 00 00 00 01 50 03 04 80 ."..P... 60 00 a7 1e bf 50 03 04 80 00 a7 1e ae 00 00 00 00 P... 70 00 00 00 00 16 22 00 03 01 00 00 03 00 00 00 00 .".. 80 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 90 00 00 00 00 00 00 00 00 16 22 00 04 01 00 00 04 .".. a0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 b0 00 00 00 00 00 00 00 00 00 00 00 00 16 22 00 05 .".. c0 01 00 00 05 00 00 00 01 50 03 04 80 00 a7 1e bf P... d0 50 03 04 80 00 a7 1e b1 00 00 00 00 00 00 00 00 P... e0 16 22 00 06 01 00 00 06 00 00 00 00 00 00 00 00 .".. f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 10000 00 00 00 16 22 00 07 01 00 00 07 00 00 00 00
Re: [patch] hisi_sas: fix error codes in hisi_sas_task_prep()
On 09/12/2015 10:48, Dan Carpenter wrote:
There were a couple cases where the error codes weren't set and also I
changed the success return to "return 0;" which is the same as
"return rc;" but more explicit.
Fixes: 42e7a69368a5 ('hisi_sas: Add ssp command functio')
Signed-off-by: Dan Carpenter
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 2929018..99b1950 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -208,15 +208,19 @@ static int hisi_sas_task_prep(struct sas_task *task,
struct hisi_hba *hisi_hba,
slot->status_buffer = dma_pool_alloc(hisi_hba->status_buffer_pool,
GFP_ATOMIC,
&slot->status_buffer_dma);
- if (!slot->status_buffer)
+ if (!slot->status_buffer) {
+ rc = -ENOMEM;
goto err_out_slot_buf;
+ }
memset(slot->status_buffer, 0, HISI_SAS_STATUS_BUF_SZ);
slot->command_table = dma_pool_alloc(hisi_hba->command_table_pool,
GFP_ATOMIC,
&slot->command_table_dma);
- if (!slot->command_table)
+ if (!slot->command_table) {
+ rc = -ENOMEM;
goto err_out_status_buf;
+ }
memset(slot->command_table, 0, HISI_SAS_COMMAND_TABLE_SZ);
memset(slot->cmd_hdr, 0, sizeof(struct hisi_sas_cmd_hdr));
@@ -254,7 +258,7 @@ static int hisi_sas_task_prep(struct sas_task *task, struct
hisi_hba *hisi_hba,
sas_dev->running_req++;
++(*pass);
- return rc;
+ return 0;
err_out_sge:
dma_pool_free(hisi_hba->sge_page_pool, slot->sge_page,
--
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
Looks ok, Thanks.
Tested-by: John Garry
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 74/71] ncr5380: Enable PDMA for NCR53C400A
On Tuesday 08 December 2015 03:05:11 Finn Thain wrote:
>
> On Sun, 6 Dec 2015, Ondrej Zary wrote:
>
> > Add I/O register mapping for NCR53C400A and enable PDMA mode to
> > improve performance and fix non-working IRQ.
> >
> > Tested with HP C2502 (and user-space enabler).
> >
> > Signed-off-by: Ondrej Zary
> > ---
> > drivers/scsi/g_NCR5380.c | 14 +++---
> > 1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
> > index 86740fd..099fdac 100644
> > --- a/drivers/scsi/g_NCR5380.c
> > +++ b/drivers/scsi/g_NCR5380.c
> > @@ -324,7 +324,7 @@ static int __init generic_NCR5380_detect(struct
> > scsi_host_template *tpnt)
> > #endif
> > break;
> > case BOARD_NCR53C400A:
> > - flags = FLAG_NO_PSEUDO_DMA;
> > + flags = FLAG_NO_DMA_FIXUP;
> > ports = ncr_53c400a_ports;
> > break;
> > case BOARD_DTC3181E:
> > @@ -406,11 +406,18 @@ static int __init generic_NCR5380_detect(struct
> > scsi_host_template *tpnt)
> > * On NCR53C400 boards, NCR5380 registers are mapped 8 past
> > * the base address.
> > */
> > - if (overrides[current_override].board == BOARD_NCR53C400) {
> > + switch (overrides[current_override].board) {
> > + case BOARD_NCR53C400:
> > instance->io_port += 8;
> > hostdata->c400_ctl_status = 0;
> > hostdata->c400_blk_cnt = 1;
> > hostdata->c400_host_buf = 4;
> > + break;
> > + case BOARD_NCR53C400A:
> > + hostdata->c400_ctl_status = 9;
> > + hostdata->c400_blk_cnt = 10;
> > + hostdata->c400_host_buf = 8;
> > + break;
> > }
> > #else
> > instance->base = overrides[current_override].NCR5380_map_name;
>
>
> For SCSI_G_NCR5380_MEM and BOARD_NCR53C400A (or BOARD_DTC3181E), you have
> not assigned c400_ctl_status, c400_blk_cnt and c400_host_buf. Perhaps we
> should throw an error, something like this?
>
> hostdata->iomem = iomem;
> - if (overrides[current_override].board == BOARD_NCR53C400) {
> + switch (overrides[current_override].board) {
> + case BOARD_NCR53C400:
> hostdata->c400_ctl_status = 0x100;
> hostdata->c400_blk_cnt = 0x101;
> hostdata->c400_host_buf = 0x104;
> + break;
> + case BOARD_NCR53C400A:
> + pr_err(DRV_MODULE_NAME ": unknown register offsets\n");
> + goto out_unregister;
> }
> #endif
We don't need to fail, just disable PDMA by setting:
flags = FLAG_NO_PSEUDO_DMA;
And BTW. this:
if (overrides[current_override].board == BOARD_NCR53C400 ||
overrides[current_override].board == BOARD_NCR53C400A)
NCR5380_write(hostdata->c400_ctl_status, CSR_BASE);
should then be, solving the problem of growing if condition:
if (!(flags & FLAG_NO_PSEUDO_DMA))
NCR5380_write(hostdata->c400_ctl_status, CSR_BASE);
--
Ondrej Zary
--
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 RESEND] atp870u: 64 bit bug in atp885_init()
Everyone knows I didn't test it but it's an obvious one line fix for memory corruption. If no one uses the code, at least this is harmless and silences a static checker warning. In olden times we used to say, "Oh this bounds checking is crap but it's root only so let's leave it alone." But these days we just fix it. It's easier to just fix everything instead of trying to decide which bugs are critical. regards, dan carpenter -- 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 RESEND] atp870u: 64 bit bug in atp885_init()
On Wed, 9 Dec 2015 16:45:12 +0300 Dan Carpenter wrote: > Everyone knows I didn't test it but it's an obvious one line fix for > memory corruption. If no one uses the code, at least this is harmless > and silences a static checker warning. > > In olden times we used to say, "Oh this bounds checking is crap but it's > root only so let's leave it alone." But these days we just fix it. > It's easier to just fix everything instead of trying to decide which > bugs are critical. Unfortunately it's all too easy to look down 50 commit messages to an apaprently active file all "fixing small bugs" or "correcting indenting" without realising that every single one of them should have been tagged "[UNTESTED]: " so that anyone looking at the code can see immediately its historical hazardous waste. Alan -- 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] ses: Fix problems with simple enclosures
On 8.12.2015 18:00, James Bottomley wrote:
> Simple enclosure implementations (mostly USB) are allowed to return only
> page 8 to every diagnostic query. That really confuses our
> implementation because we assume the return is the page we asked for and
> end up doing incorrect offsets based on bogus information leading to
> accesses outside of allocated ranges. Fix that by checking the page
> code of the return and giving an error if it isn't the one we asked for.
> This should fix reported bugs with USB storage by simply refusing to
> attach to enclosures that behave like this. It's also good defensive
> practise now that we're starting to see more USB enclosures.
Ideally this patch also fixes all callers so they evaluate the return value
from ses_recv_diag. That is missed in ses_enclosure_data_process
and ses_get_page2_descriptor.
-tms
>
> Reported-by: Andrea Gelmini
> Cc: sta...@vger.kernel.org
> Signed-off-by: James Bottomley
>
> ---
>
> diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
> index dcb0d76..7d9cec5 100644
> --- a/drivers/scsi/ses.c
> +++ b/drivers/scsi/ses.c
> @@ -84,6 +84,7 @@ static void init_device_slot_control(unsigned char
> *dest_desc,
> static int ses_recv_diag(struct scsi_device *sdev, int page_code,
>void *buf, int bufflen)
> {
> + int ret;
> unsigned char cmd[] = {
> RECEIVE_DIAGNOSTIC,
> 1, /* Set PCV bit */
> @@ -92,9 +93,26 @@ static int ses_recv_diag(struct scsi_device *sdev, int
> page_code,
> bufflen & 0xff,
> 0
> };
> + unsigned char recv_page_code;
>
> - return scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buf, bufflen,
> + ret = scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buf, bufflen,
> NULL, SES_TIMEOUT, SES_RETRIES, NULL);
> + if (unlikely(!ret))
> + return ret;
> +
> + recv_page_code = ((unsigned char *)buf)[0];
> +
> + if (likely(recv_page_code == page_code))
> + return ret;
> +
> + /* successful diagnostic but wrong page code. This happens to some
> + * USB devices, just print a message and pretend there was an error */
> +
> + sdev_printk(KERN_ERR, sdev,
> + "Wrong diagnostic page; asked for %d got %u\n",
> + page_code, recv_page_code);
> +
> + return -EINVAL;
> }
>
> static int ses_send_diag(struct scsi_device *sdev, int page_code,
>
>
> --
> 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
--
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 RESEND] atp870u: 64 bit bug in atp885_init()
We should add a tag to indicate that we are sending a patch for a crappy driver. IMHO-this-driver-is-garbage: Your Name If it got 10 votes of no confidence it would be moved to staging and then deleted. Anyway, realistically, let's just apply this fix. It's tempting to think we could delete all atp885 related code, but maybe people are still using it with 32 bit kernels. Or someone could delete it, but I'm not brave enough to be the one to do it. regards, dan carpenter -- 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 RESEND] atp870u: 64 bit bug in atp885_init()
On Wed, 9 Dec 2015, Dan Carpenter wrote: > We should add a tag to indicate that we are sending a patch for a crappy > driver. > > IMHO-this-driver-is-garbage: Your Name > > If it got 10 votes of no confidence it would be moved to staging and > then deleted. Forgive my ignorance, but what is the exact procedure? For example, the following file: drivers/pcmcia/vrc4173_cardu.c contains the following code: INIT_WORK(&socket->tq_work, cardu_bh, socket);. The last time INIT_WORK took three arguments was Linux 2.6.19, so I think no one has been compiling this code recently. There would be the .c file and the associated .h file to move to staging, but it's less clear to me eg what to do with the Kconfig entry and the Makefile entry. And is there anything else to take into account? thanks, julia -- 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] ses: Fix problems with simple enclosures
On Wed, 2015-12-09 at 18:07 +0100, Tomas Henzl wrote:
> On 8.12.2015 18:00, James Bottomley wrote:
> > Simple enclosure implementations (mostly USB) are allowed to return only
> > page 8 to every diagnostic query. That really confuses our
> > implementation because we assume the return is the page we asked for and
> > end up doing incorrect offsets based on bogus information leading to
> > accesses outside of allocated ranges. Fix that by checking the page
> > code of the return and giving an error if it isn't the one we asked for.
> > This should fix reported bugs with USB storage by simply refusing to
> > attach to enclosures that behave like this. It's also good defensive
> > practise now that we're starting to see more USB enclosures.
>
> Ideally this patch also fixes all callers so they evaluate the return value
> from ses_recv_diag. That is missed in ses_enclosure_data_process
> and ses_get_page2_descriptor.
Well, it wouldn't be a bug fix and it's strictly not necessary. in
ses_intf_add() we won't attach if the initial retrieve of page 2 fails.
That means we already have an old copy. So if there's a failure in
ses_get_page2_descriptor() then we're just working from old data.
Essentially there's nothing else we could do (except perhaps log the
problem).
James
> -tms
>
> >
> > Reported-by: Andrea Gelmini
> > Cc: sta...@vger.kernel.org
> > Signed-off-by: James Bottomley
> >
> > ---
> >
> > diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
> > index dcb0d76..7d9cec5 100644
> > --- a/drivers/scsi/ses.c
> > +++ b/drivers/scsi/ses.c
> > @@ -84,6 +84,7 @@ static void init_device_slot_control(unsigned char
> > *dest_desc,
> > static int ses_recv_diag(struct scsi_device *sdev, int page_code,
> > void *buf, int bufflen)
> > {
> > + int ret;
> > unsigned char cmd[] = {
> > RECEIVE_DIAGNOSTIC,
> > 1, /* Set PCV bit */
> > @@ -92,9 +93,26 @@ static int ses_recv_diag(struct scsi_device *sdev, int
> > page_code,
> > bufflen & 0xff,
> > 0
> > };
> > + unsigned char recv_page_code;
> >
> > - return scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buf, bufflen,
> > + ret = scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buf, bufflen,
> > NULL, SES_TIMEOUT, SES_RETRIES, NULL);
> > + if (unlikely(!ret))
> > + return ret;
> > +
> > + recv_page_code = ((unsigned char *)buf)[0];
> > +
> > + if (likely(recv_page_code == page_code))
> > + return ret;
> > +
> > + /* successful diagnostic but wrong page code. This happens to some
> > +* USB devices, just print a message and pretend there was an error */
> > +
> > + sdev_printk(KERN_ERR, sdev,
> > + "Wrong diagnostic page; asked for %d got %u\n",
> > + page_code, recv_page_code);
> > +
> > + return -EINVAL;
> > }
> >
> > static int ses_send_diag(struct scsi_device *sdev, int page_code,
> >
> >
> > --
> > 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
>
> --
> 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
>
--
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 RESEND] atp870u: 64 bit bug in atp885_init()
On Wed, Dec 09, 2015 at 07:11:15PM +0100, Julia Lawall wrote: > Forgive my ignorance, but what is the exact procedure? For example, the > following file: drivers/pcmcia/vrc4173_cardu.c contains the following > code: INIT_WORK(&socket->tq_work, cardu_bh, socket);. The last time > INIT_WORK took three arguments was Linux 2.6.19, so I think no one has > been compiling this code recently. There would be the .c file and the > associated .h file to move to staging, but it's less clear to me eg what > to do with the Kconfig entry and the Makefile entry. And is there > anything else to take into account? You should just delete that code along with the Kconfig and Makefile entries. Don't bother moving it to staging. The move to staging is supposed to give people one last chance to yell if they absolutely need the code. No one has compiled this code for years so no one will miss it. regards, dan carpenter -- 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 0/3] Fix the problem of SATA devices within SAS enclosures
A big use of SAS enclosures is actually to hold SATA devices. Right at the moment, the ses enclosure system doesn't recognize SATA devices. The reason for this is that SAS actually makes up an endpoint address for a SATA device since SATA doesn't have an addressing scheme. The problem this causes is that the made up SAS address doesn't match the SATA NAA identifier in VPD page 0x83, so the address for the device the enclosure reports never matches. We can fix this by using the SAS transport class to give us the actual end point address instead of using the identity VPD page. This ensures SATA devices are always correctly matched at the expense of pulling in all the SAS transport code. Instead of making ses depend on the SAS transport class, I've introduced an is_sas_attached() function that will allow us to compile out the code if the kernel isn't built with SAS. If anyone ever gets around to doing FC enclosures, they should probably be done in the same way. James --- James Bottomley (3): scsi_transport_sas: add is_sas_attached() function scsi_transport_sas: add function to get SAS endpoint address ses: fix discovery of SATA devices in SAS enclosures drivers/scsi/scsi_transport_sas.c | 30 ++ drivers/scsi/ses.c| 22 -- include/scsi/scsi_transport_sas.h | 10 ++ 3 files changed, 44 insertions(+), 18 deletions(-) -- 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 1/3] scsi_transport_sas: add is_sas_attached() function
Adds a function designed to be callable any time (regardless of
whether the transport attributes are configured or not) which returns
true if the device is attached over a SAS transport. The design of
this function is that transport specific functions can be embedded
within a
if (is_sas_attached(sdev)) {
...
}
which would be compiled out (and thus eliminate the symbols) if SAS is
not configured.
Signed-off-by: James Bottomley
---
drivers/scsi/scsi_transport_sas.c | 16
include/scsi/scsi_transport_sas.h |9 +
2 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/scsi_transport_sas.c
b/drivers/scsi/scsi_transport_sas.c
index 30d26e3..b17f763 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -341,6 +341,22 @@ static int do_sas_phy_delete(struct device *dev, void
*data)
}
/**
+ * is_sas_attached - check if device is SAS attached
+ * @sdev: scsi device to check
+ *
+ * returns true if the device is SAS attached
+ */
+int is_sas_attached(struct scsi_device *sdev)
+{
+ struct Scsi_Host *shost = sdev->host;
+
+ return shost->transportt->host_attrs.ac.class ==
+ &sas_host_class.class;
+}
+EXPORT_SYMBOL(is_sas_attached);
+
+
+/**
* sas_remove_children - tear down a devices SAS data structures
* @dev: device belonging to the sas object
*
diff --git a/include/scsi/scsi_transport_sas.h
b/include/scsi/scsi_transport_sas.h
index 0bd71e2..a8fdd10 100644
--- a/include/scsi/scsi_transport_sas.h
+++ b/include/scsi/scsi_transport_sas.h
@@ -10,6 +10,15 @@ struct scsi_transport_template;
struct sas_rphy;
struct request;
+#if !IS_ENABLED(CONFIG_SCSI_SAS_ATTRS)
+static inline int is_sas_attached(struct scsi_device *sdev)
+{
+ return 0;
+}
+#else
+extern int is_sas_attached(struct scsi_device *sdev);
+#endif
+
static inline int sas_protocol_ata(enum sas_protocol proto)
{
return ((proto & SAS_PROTOCOL_SATA) ||
--
1.7.4.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 2/3] scsi_transport_sas: add function to get SAS endpoint address
For a device known to be SAS connected, this will return the endpoint
address. This is useful for getting the SAS address of SATA devices.
Signed-off-by: James Bottomley
---
drivers/scsi/scsi_transport_sas.c | 14 ++
include/scsi/scsi_transport_sas.h |1 +
2 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/scsi_transport_sas.c
b/drivers/scsi/scsi_transport_sas.c
index b17f763..80520e2 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -383,6 +383,20 @@ void sas_remove_host(struct Scsi_Host *shost)
EXPORT_SYMBOL(sas_remove_host);
/**
+ * sas_get_address - return the SAS address of the device
+ * @sdev: scsi device
+ *
+ * Returns the SAS address of the scsi device
+ */
+u64 sas_get_address(struct scsi_device *sdev)
+{
+ struct sas_end_device *rdev = sas_sdev_to_rdev(sdev);
+
+ return rdev->rphy.identify.sas_address;
+}
+EXPORT_SYMBOL(sas_get_address);
+
+/**
* sas_tlr_supported - checking TLR bit in vpd 0x90
* @sdev: scsi device struct
*
diff --git a/include/scsi/scsi_transport_sas.h
b/include/scsi/scsi_transport_sas.h
index a8fdd10..13c0b2b 100644
--- a/include/scsi/scsi_transport_sas.h
+++ b/include/scsi/scsi_transport_sas.h
@@ -189,6 +189,7 @@ extern int sas_phy_add(struct sas_phy *);
extern void sas_phy_delete(struct sas_phy *);
extern int scsi_is_sas_phy(const struct device *);
+u64 sas_get_address(struct scsi_device *);
unsigned int sas_tlr_supported(struct scsi_device *);
unsigned int sas_is_tlr_enabled(struct scsi_device *);
void sas_disable_tlr(struct scsi_device *);
--
1.7.4.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 3/3] ses: fix discovery of SATA devices in SAS enclosures
The current discovery routines use the VPD 0x83 inquiry page to find
the device SAS address and match it to the end point in the enclosure.
This doesn't work for SATA devices because expanders (or hosts) simply
make up an endpoint address for STP and thus the address returned by
the VPD page never matches. Instead of doing this, for SAS attached
devices, match by the direct endpoint address instead.
Signed-off-by: James Bottomley
---
drivers/scsi/ses.c | 22 --
1 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index 7d9cec5..1736935 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -34,6 +34,8 @@
#include
#include
+#include
+
struct ses_device {
unsigned char *page1;
unsigned char *page1_types;
@@ -571,31 +573,15 @@ static void ses_enclosure_data_process(struct
enclosure_device *edev,
static void ses_match_to_enclosure(struct enclosure_device *edev,
struct scsi_device *sdev)
{
- unsigned char *desc;
struct efd efd = {
.addr = 0,
};
ses_enclosure_data_process(edev, to_scsi_device(edev->edev.parent), 0);
- if (!sdev->vpd_pg83_len)
- return;
-
- desc = sdev->vpd_pg83 + 4;
- while (desc < sdev->vpd_pg83 + sdev->vpd_pg83_len) {
- enum scsi_protocol proto = desc[0] >> 4;
- u8 code_set = desc[0] & 0x0f;
- u8 piv = desc[1] & 0x80;
- u8 assoc = (desc[1] & 0x30) >> 4;
- u8 type = desc[1] & 0x0f;
- u8 len = desc[3];
-
- if (piv && code_set == 1 && assoc == 1
- && proto == SCSI_PROTOCOL_SAS && type == 3 && len == 8)
- efd.addr = get_unaligned_be64(&desc[4]);
+ if (is_sas_attached(sdev))
+ efd.addr = sas_get_address(sdev);
- desc += len + 4;
- }
if (efd.addr) {
efd.dev = &sdev->sdev_gendev;
--
1.7.4.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 RESEND] atp870u: 64 bit bug in atp885_init()
> You should just delete that code along with the Kconfig and Makefile > entries. Don't bother moving it to staging. The move to staging is > supposed to give people one last chance to yell if they absolutely need > the code. No one has compiled this code for years so no one will miss > it. And for stuff which might be worth saving (eg something that looks rather broken but has possibly got users) the driver goes into staging in its own directory and the Makefile and Kconfig entry for it move into the staging directory with the hope that someone screams and maintains it. Alan -- 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 12/20] target/tmr: LUN reset cause cmd premature free.
Christoph, Understood. I was not sure which way the community is swinging. For what it¹s worth, this fix was required to stabilize one of our customer environment in the older kernel. Regards, Quinn Tran On 12/7/15, 6:48 PM, "target-devel-ow...@vger.kernel.org on behalf of Christoph Hellwig" wrote: >On Mon, Dec 07, 2015 at 07:48:59PM -0500, Himanshu Madhani wrote: >> From: Quinn Tran >> >> During LUN/Target reset, the TMR code attempt to intercept >> cmds and try to aborted them. Current code assume cmds are >> always intercepted at the back end device. The cleanup code >> would issue a "queue_status() & check_stop_free()" to terminate >> the command. However, when a cmd is intercepted at the front >> end/Fabric layer, current code introduce premature free or >> cause Fabric to double free. >> >> When command is intercepted at Fabric layer, it means a >> check_stop_free(cmd_kref--) has been called. The extra >> check_stop_free in the Lun Reset cleanup code causes early >> free. When a cmd in the Fabric layer is completed, the normal >> free code adds another another free which introduce a double free. >> >> To fix the issue: >> - add a new flag/CMD_T_SENT_STATUS to track command that have >> made it down to fabric layer after back end good/bad completion. >> - if cmd reach Fabric Layer at Lun Reset time, add an extra >> cmd_kref count to prevent premature free. > >This seems lke something solved by Bart's abort rewrite in a much nicer >way. All this special casing based on magic flags isn't maintainable >in the long run. >-- >To unsubscribe from this list: send the line "unsubscribe target-devel" in >the body of a message to majord...@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html <>
Re: [PATCH 11/20] qla2xxx: Add TAS detection for kernel 3.15 n newer
On 12/7/15, 6:48 PM, "target-devel-ow...@vger.kernel.org on behalf of
Christoph Hellwig" wrote:
>> diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
>>b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
>> index 842fcca..2e9c194 100644
>> --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
>> +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
>> @@ -617,6 +617,26 @@ static int tcm_qla2xxx_queue_status(struct se_cmd
>>*se_cmd)
>> struct qla_tgt_cmd, se_cmd);
>> int xmit_type = QLA_TGT_XMIT_STATUS;
>>
>> +if (se_cmd->transport_state & CMD_T_ABORTED) {
>> +/* For TCM TAS support n kernel >= 3.15:
>> + * This cmd is attempting to respond with "Task Aborted Status".
>> + */
>> +if (cmd->aborted) {
>> +return 0;
>> +} else if ((cmd->state == QLA_TGT_STATE_NEED_DATA) &&
>> +cmd->cmd_sent_to_fw) {
>> +qlt_abort_cmd(cmd);
>> +return 0;
>> +} else if (cmd->state == QLA_TGT_STATE_PROCESSED) {
>> +if (cmd->cmd_sent_to_fw) {
>> +qlt_abort_cmd(cmd);
>> +return 0;
>> +} else {/* about to be free */
>> +return 0;
>> +}
>> +}
>> +}
>> +
>
>This is really something that should be explicitly communicated
>from the core instead of having to second guess it.
QT> The extra protection of the code here is to reduce erroneous error
message and interaction error with our firmware. I think communicating
back to the core at this stage might add undue complication. It¹s best to
allow the initiator to re-drive the command at this point.
<>
Re: [PATCH 3/3] ses: fix discovery of SATA devices in SAS enclosures
Hi James, [auto build test ERROR on v4.4-rc4] [cannot apply to scsi/for-next next-20151209] url: https://github.com/0day-ci/linux/commits/James-Bottomley/Fix-the-problem-of-SATA-devices-within-SAS-enclosures/20151210-031802 config: i386-randconfig-b0-12100330 (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/built-in.o: In function `ses_match_to_enclosure': >> ses.c:(.text+0x1c04ad): undefined reference to `is_sas_attached' >> ses.c:(.text+0x1c0519): undefined reference to `sas_get_address' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
Re: [PATCH 3/3] ses: fix discovery of SATA devices in SAS enclosures
On Thu, 2015-12-10 at 04:38 +0800, kbuild test robot wrote: > Hi James, > > [auto build test ERROR on v4.4-rc4] > [cannot apply to scsi/for-next next-20151209] > > url: > https://github.com/0day-ci/linux/commits/James-Bottomley/Fix-the-problem-of-SATA-devices-within-SAS-enclosures/20151210-031802 > config: i386-randconfig-b0-12100330 (attached as .config) > reproduce: > # save the attached .config to linux build tree > make ARCH=i386 > > All errors (new ones prefixed by >>): > >drivers/built-in.o: In function `ses_match_to_enclosure': > >> ses.c:(.text+0x1c04ad): undefined reference to `is_sas_attached' > >> ses.c:(.text+0x1c0519): undefined reference to `sas_get_address' Oh, that's the perennial file built in but dependencies are modules problem. The fix is to add this line to the Kconfig for SCSI_ENCLOSURE depends on m || SCSI_SAS_ATTRS != m I'll post a v2 with that added. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] ses: fix discovery of SATA devices in SAS enclosures
The current discovery routines use the VPD 0x83 inquiry page to find
the device SAS address and match it to the end point in the enclosure.
This doesn't work for SATA devices because expanders (or hosts) simply
make up an endpoint address for STP and thus the address returned by
the VPD page never matches. Instead of doing this, for SAS attached
devices, match by the direct endpoint address instead.
Signed-off-by: James Bottomley
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 5f692ae..2a1d20e 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -194,6 +194,7 @@ config CHR_DEV_SCH
config SCSI_ENCLOSURE
tristate "SCSI Enclosure Support"
depends on SCSI && ENCLOSURE_SERVICES
+ depends on m || SCSI_SAS_ATTRS != m
help
Enclosures are devices sitting on or in SCSI backplanes that
manage devices. If you have a disk cage, the chances are that
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index 7d9cec5..1736935 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -34,6 +34,8 @@
#include
#include
+#include
+
struct ses_device {
unsigned char *page1;
unsigned char *page1_types;
@@ -571,31 +573,15 @@ static void ses_enclosure_data_process(struct
enclosure_device *edev,
static void ses_match_to_enclosure(struct enclosure_device *edev,
struct scsi_device *sdev)
{
- unsigned char *desc;
struct efd efd = {
.addr = 0,
};
ses_enclosure_data_process(edev, to_scsi_device(edev->edev.parent), 0);
- if (!sdev->vpd_pg83_len)
- return;
-
- desc = sdev->vpd_pg83 + 4;
- while (desc < sdev->vpd_pg83 + sdev->vpd_pg83_len) {
- enum scsi_protocol proto = desc[0] >> 4;
- u8 code_set = desc[0] & 0x0f;
- u8 piv = desc[1] & 0x80;
- u8 assoc = (desc[1] & 0x30) >> 4;
- u8 type = desc[1] & 0x0f;
- u8 len = desc[3];
-
- if (piv && code_set == 1 && assoc == 1
- && proto == SCSI_PROTOCOL_SAS && type == 3 && len == 8)
- efd.addr = get_unaligned_be64(&desc[4]);
+ if (is_sas_attached(sdev))
+ efd.addr = sas_get_address(sdev);
- desc += len + 4;
- }
if (efd.addr) {
efd.dev = &sdev->sdev_gendev;
--
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 3/3] hpsa: add box and bay information for enclosure devices
Reviewed-by: Justin Lindley
Reviewed-by: Kevin Barnett
Reviewed-by: Scott Teel
Signed-off-by: Don Brace
---
drivers/scsi/hpsa.c | 108 ---
drivers/scsi/hpsa_cmd.h | 13 ++
2 files changed, 114 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index f8370b8..6f84ec7 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -750,7 +750,6 @@ static ssize_t host_show_hp_ssd_smart_path_enabled(struct
device *dev,
}
#define MAX_PATHS 8
-
static ssize_t path_info_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -792,9 +791,7 @@ static ssize_t path_info_show(struct device *dev,
hdev->bus, hdev->target, hdev->lun,
scsi_device_type(hdev->devtype));
- if (hdev->external ||
- hdev->devtype == TYPE_RAID ||
- is_logical_device(hdev)) {
+ if (hdev->devtype == TYPE_RAID || is_logical_device(hdev)) {
output_len += scnprintf(buf + output_len,
PAGE_SIZE - output_len,
"%s\n", active);
@@ -808,8 +805,7 @@ static ssize_t path_info_show(struct device *dev,
phys_connector[0] = '0';
if (phys_connector[1] < '0')
phys_connector[1] = '0';
- if (hdev->phys_connector[i] > 0)
- output_len += scnprintf(buf + output_len,
+ output_len += scnprintf(buf + output_len,
PAGE_SIZE - output_len,
"PORT: %.2s ",
phys_connector);
@@ -3191,6 +3187,90 @@ out:
return rc;
}
+/*
+ * get enclosure information
+ * struct ReportExtendedLUNdata *rlep - Used for BMIC drive number
+ * struct hpsa_scsi_dev_t *encl_dev - device entry for enclosure
+ * Uses id_physical_device to determine the box_index.
+ */
+static int hpsa_get_enclosure_info(struct ctlr_info *h,
+ unsigned char *scsi3addr,
+ struct ReportExtendedLUNdata *rlep, int rle_index,
+ struct hpsa_scsi_dev_t *encl_dev)
+{
+ int rc = -1;
+ struct CommandList *c = NULL;
+ struct ErrorInfo *ei = NULL;
+ struct bmic_sense_storage_box_params *bssbp = NULL;
+ struct bmic_identify_physical_device *id_phys = NULL;
+ struct ext_report_lun_entry *rle = &rlep->LUN[rle_index];
+ u16 bmic_device_index = 0;
+
+
+ bssbp = kzalloc(sizeof(*bssbp), GFP_KERNEL);
+ if (!bssbp)
+ goto out;
+
+ id_phys = kzalloc(sizeof(*id_phys), GFP_KERNEL);
+ if (!id_phys)
+ goto out;
+
+ bmic_device_index = GET_BMIC_DRIVE_NUMBER(&rle->lunid[0]);
+
+ if (bmic_device_index == 0xFF00)
+ goto out;
+
+ memset(id_phys, 0, sizeof(*id_phys));
+ rc = hpsa_bmic_id_physical_device(h, scsi3addr, bmic_device_index,
+ id_phys, sizeof(*id_phys));
+ if (rc) {
+ dev_warn(&h->pdev->dev, "%s: id_phys failed %d bdi[0x%x]\n",
+ __func__, encl_dev->external, bmic_device_index);
+ goto out;
+ }
+
+ c = cmd_alloc(h);
+
+ rc = fill_cmd(c, BMIC_SENSE_STORAGE_BOX_PARAMS, h, bssbp,
+ sizeof(*bssbp), 0, RAID_CTLR_LUNID, TYPE_CMD);
+
+ if (rc)
+ goto out;
+
+ if (id_phys->phys_connector[1] == 'E')
+ c->Request.CDB[5] = id_phys->box_index;
+ else
+ c->Request.CDB[5] = 0;
+
+ rc = hpsa_scsi_do_simple_cmd_with_retry(h, c, PCI_DMA_FROMDEVICE,
+ NO_TIMEOUT);
+ if (rc)
+ goto out;
+
+ ei = c->err_info;
+ if (ei->CommandStatus != 0 && ei->CommandStatus != CMD_DATA_UNDERRUN) {
+ rc = -1;
+ goto out;
+ }
+
+ encl_dev->box[id_phys->active_path_number] = bssbp->phys_box_on_port;
+ memcpy(&encl_dev->phys_connector[id_phys->active_path_number],
+ bssbp->phys_connector, sizeof(bssbp->phys_connector));
+
+ rc = IO_OK;
+out:
+ kfree(bssbp);
+ kfree(id_phys);
+
+ if (c)
+ cmd_free(h, c);
+
+ if (rc != IO_OK)
+ hpsa_show_dev_msg(KERN_INFO, h, encl_dev,
+ "Error, could not get enclosure information\n");
+ return rc;
+}
+
static u64 hpsa_get_sas_address_from_report_physical(struct ctlr_info *h,
unsigned char *scsi3addr)
{
@@ -4032,7 +4112,8 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h)
/* skip masked non-disk devices */
if (MASKED_DEVICE(lunaddrbytes) && physical
[PATCH 1/3] hpsa: fix path_info_show
left off some changes from Rasmus Villemoes where he changed
snprintf to scnprintf
Suggested-by: Rasmus Villemoes
Reviewed-by: Justin Lindley
Reviewed-by: Kevin Barnett
Reviewed-by: Scott Teel
Signed-off-by: Don Brace
---
drivers/scsi/hpsa.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index a386036..f8370b8 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -795,7 +795,7 @@ static ssize_t path_info_show(struct device *dev,
if (hdev->external ||
hdev->devtype == TYPE_RAID ||
is_logical_device(hdev)) {
- output_len += snprintf(buf + output_len,
+ output_len += scnprintf(buf + output_len,
PAGE_SIZE - output_len,
"%s\n", active);
continue;
@@ -809,28 +809,28 @@ static ssize_t path_info_show(struct device *dev,
if (phys_connector[1] < '0')
phys_connector[1] = '0';
if (hdev->phys_connector[i] > 0)
- output_len += snprintf(buf + output_len,
+ output_len += scnprintf(buf + output_len,
PAGE_SIZE - output_len,
"PORT: %.2s ",
phys_connector);
if (hdev->devtype == TYPE_DISK && hdev->expose_device) {
if (box == 0 || box == 0xFF) {
- output_len += snprintf(buf + output_len,
+ output_len += scnprintf(buf + output_len,
PAGE_SIZE - output_len,
"BAY: %hhu %s\n",
bay, active);
} else {
- output_len += snprintf(buf + output_len,
+ output_len += scnprintf(buf + output_len,
PAGE_SIZE - output_len,
"BOX: %hhu BAY: %hhu %s\n",
box, bay, active);
}
} else if (box != 0 && box != 0xFF) {
- output_len += snprintf(buf + output_len,
+ output_len += scnprintf(buf + output_len,
PAGE_SIZE - output_len, "BOX: %hhu %s\n",
box, active);
} else
- output_len += snprintf(buf + output_len,
+ output_len += scnprintf(buf + output_len,
PAGE_SIZE - output_len, "%s\n", active);
}
--
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 0/3] hpsa update
These patches are based on Linus's tree The changes are: - correct missing changes from snprintf to scnprintf in path_info_show by Rasmus Villemoes - fix reported bus for SAS transport devices - add in enclosure information --- Don Brace (3): hpsa: fix path_info_show hpsa: change SAS transport devices to bus 0. hpsa: add box and bay information for enclosure devices drivers/scsi/hpsa.c | 118 ++- drivers/scsi/hpsa.h |2 - drivers/scsi/hpsa_cmd.h | 13 + 3 files changed, 120 insertions(+), 13 deletions(-) -- Signature -- 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 2/3] hpsa: change SAS transport devices to bus 0.
sas transport places devices on bus 0 but driver was setting
the bus to 3.
Reviewed-by: Justin Lindley
Reviewed-by: Kevin Barnett
Reviewed-by: Scott Teel
Signed-off-by: Don Brace
---
drivers/scsi/hpsa.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
index ae5beda..fdd39fc 100644
--- a/drivers/scsi/hpsa.h
+++ b/drivers/scsi/hpsa.h
@@ -400,7 +400,7 @@ struct offline_device_entry {
#define HPSA_PHYSICAL_DEVICE_BUS 0
#define HPSA_RAID_VOLUME_BUS 1
#define HPSA_EXTERNAL_RAID_VOLUME_BUS 2
-#define HPSA_HBA_BUS 3
+#define HPSA_HBA_BUS 0
/*
Send the command to the hardware
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/20] qla2xxx: Fix interaction issue between qla2xxx and Target Core Module
On 12/7/15, 6:37 PM, "target-devel-ow...@vger.kernel.org on behalf of
Christoph Hellwig" wrote:
>> -void qlt_abort_cmd(struct qla_tgt_cmd *cmd)
>> +int qlt_abort_cmd(struct qla_tgt_cmd *cmd)
>> {
>> struct qla_tgt *tgt = cmd->tgt;
>> struct scsi_qla_host *vha = tgt->vha;
>> struct se_cmd *se_cmd = &cmd->se_cmd;
>> +unsigned long flags,refcount;
>>
>> ql_dbg(ql_dbg_tgt_mgt, vha, 0xf014,
>> "qla_target(%d): terminating exchange for aborted cmd=%p "
>> "(se_cmd=%p, tag=%llu)", vha->vp_idx, cmd, &cmd->se_cmd,
>> se_cmd->tag);
>>
>> +spin_lock_irqsave(&cmd->cmd_lock, flags);
>> +if (cmd->aborted) {
>> +spin_unlock_irqrestore(&cmd->cmd_lock, flags);
>> +
>> +/* It's normal to see 2 calls in this path:
>> + * 1) XFER Rdy completion + CMD_T_ABORT
>> + * 2) TCM TMR - drain_state_list
>> + */
>> +refcount = atomic_read(&cmd->se_cmd.cmd_kref.refcount);
>> +ql_dbg(ql_dbg_tgt_mgt, vha, 0x,
>> + "multiple abort. %p refcount %lx"
>> + "transport_state %x, t_state %x, se_cmd_flags %x \n",
>> + cmd, refcount,cmd->se_cmd.transport_state,
>> + cmd->se_cmd.t_state,cmd->se_cmd.se_cmd_flags);
>> +
>> +return EIO;
>> +}
>
>Err, no. Looking into the refcount inside a kref is never the
>right thing to do.
QT> even for debug purpose??
>
>> +typedef enum {
>> +/*
>> + * BIT_0 - Atio Arrival / schedule to work
>> + * BIT_1 - qlt_do_work
>> + * BIT_2 - qlt_do work failed
>> + * BIT_3 - xfer rdy/tcm_qla2xxx_write_pending
>> + * BIT_4 - read respond/tcm_qla2xx_queue_data_in
>> + * BIT_5 - status respond / tcm_qla2xx_queue_status
>> + * BIT_6 - tcm request to abort/Term exchange.
>> + * pre_xmit_response->qlt_send_term_exchange
>> + * BIT_7 - SRR received (qlt_handle_srr->qlt_xmit_response)
>> + * BIT_8 - SRR received (qlt_handle_srr->qlt_rdy_to_xfer)
>> + * BIT_9 - SRR received (qla_handle_srr->qlt_send_term_exchange)
>> + * BIT_10 - Data in - hanlde_data->tcm_qla2xxx_handle_data
>> +
>> + * BIT_12 - good completion - qlt_ctio_do_completion -->free_cmd
>> + * BIT_13 - Bad completion -
>> + * qlt_ctio_do_completion --> qlt_term_ctio_exchange
>> + * BIT_14 - Back end data received/sent.
>> + * BIT_15 - SRR prepare ctio
>> + * BIT_16 - complete free
>> + * BIT_17 - flush - qlt_abort_cmd_on_host_reset
>> + * BIT_18 - completion w/abort status
>> + * BIT_19 - completion w/unknown status
>> + * BIT_20 - tcm_qla2xxx_free_cmd
>
>Please use descriptive names for these flags in the source code!
QT> ACK. We¹ll change the bits to more descriptive name in a ³follow on²
patch.
>
>> +BUG_ON(cmd->cmd_flags & BIT_20);
>> +cmd->cmd_flags |= BIT_20;
>> +
>
>And no crazieness like this. While we're at it: what synchronizes
>access to ->cmd_flags?
QT> These bits provide indication as to where the command has traversed in
the QLA code. Each bit is set one time. Due to the async nature of the
TMR code, it triggers QLA driver to repeat this specific free path in the
double free case. This BUG_ON allows us trap it early on.
In one of the corner case (below), I need to overloaded it + lock for the
cleanup process.
>
>> @@ -466,13 +484,25 @@ static int tcm_qla2xxx_handle_cmd(scsi_qla_host_t
>>*vha, struct qla_tgt_cmd *cmd,
>> static void tcm_qla2xxx_handle_data_work(struct work_struct *work)
>> {
>> struct qla_tgt_cmd *cmd = container_of(work, struct qla_tgt_cmd,
>>work);
>> +unsigned long flags;
>>
>> /*
>> * Ensure that the complete FCP WRITE payload has been received.
>> * Otherwise return an exception via CHECK_CONDITION status.
>> */
>> cmd->cmd_in_wq = 0;
>> -cmd->cmd_flags |= BIT_11;
>> +
>> +spin_lock_irqsave(&cmd->cmd_lock, flags);
>> +cmd->cmd_flags |= CMD_FLAG_DATA_WORK;
>> +if (cmd->aborted) {
>> +cmd->cmd_flags |= CMD_FLAG_DATA_WORK_FREE;
>> +spin_unlock_irqrestore(&cmd->cmd_lock, flags);
>> +
>> +tcm_qla2xxx_free_cmd(cmd);
>> +return;
>> +}
>> +spin_unlock_irqrestore(&cmd->cmd_lock, flags);
>
>All these abort flag hacks look very suspicios. Can you explain the
>exact theory of operation behind them?
QT> The cmd->aborted flag is used to track the CMD_T_ABORT flag at TCM
level. If the command have been requested to be aborted by TCM or already
aborted, we advance it to the ³free" state because our hardware have
already started freeing up resources associated to this command/exchange.
In this specific case(above), a XFER RDY was aborted by the TMR.
Returning the cmd to TCM to generate SCSI Status would generate erroneous
HW error due to freed resource.
>
>--
>To unsubscribe from this list: send the line "unsubscribe target-devel" in
>the body of a message to majord...@vger.kernel.org
>More majordomo info at http://vger.ker
Re: [PATCH 10/20] qla2xxx: Fix interaction issue between qla2xxx and Target Core Module
Hannes, ACK. We¹ll move the flags to bitops in the "follow on" patch to clean it up. Those flags was introduced from a different patch. Will move the few overloaded flag to bit field. However, getting rid of the spin lock would prove tricky because the code is trying to serialize the cleanup. With out the lock, we kept hitting multiple free problem. Regards, Quinn Tran On 12/8/15, 11:01 PM, "target-devel-ow...@vger.kernel.org on behalf of Hannes Reinecke" wrote: >>+ >> } >> >> static void tcm_qla2xxx_clear_sess_lookup(struct tcm_qla2xxx_lport *, >Have you considered moving to bit ops when modifying cmd_flags? >I guess you can also move the ->aborted bit into the bit field, and >could get rid of some of the spinlocks ... > >Cheers, > >Hannes >-- >Dr. Hannes Reinecke zSeries & Storage >h...@suse.de +49 911 74053 688 >SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg >GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) >-- <>
Re: [PATCH 1/3] hpsa: fix path_info_show
Reviewed-by: Matthew R. Ochs -- 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 2/3] hpsa: change SAS transport devices to bus 0.
> On Dec 9, 2015, at 3:18 PM, Don Brace wrote:
>
> sas transport places devices on bus 0 but driver was setting
> the bus to 3.
>
> Reviewed-by: Justin Lindley
> Reviewed-by: Kevin Barnett
> Reviewed-by: Scott Teel
> Signed-off-by: Don Brace
> ---
> drivers/scsi/hpsa.h |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
> index ae5beda..fdd39fc 100644
> --- a/drivers/scsi/hpsa.h
> +++ b/drivers/scsi/hpsa.h
> @@ -400,7 +400,7 @@ struct offline_device_entry {
> #define HPSA_PHYSICAL_DEVICE_BUS 0
> #define HPSA_RAID_VOLUME_BUS 1
> #define HPSA_EXTERNAL_RAID_VOLUME_BUS 2
> -#define HPSA_HBA_BUS 3
> +#define HPSA_HBA_BUS 0
Is this not the same as using HPSA_PHYSICAL_DEVICE_BUS?
--
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 3/3] hpsa: add box and bay information for enclosure devices
> On Dec 9, 2015, at 3:18 PM, Don Brace wrote:
No commit message?
>
> Reviewed-by: Justin Lindley
> Reviewed-by: Kevin Barnett
> Reviewed-by: Scott Teel
> Signed-off-by: Don Brace
> ---
> drivers/scsi/hpsa.c | 108 ---
> drivers/scsi/hpsa_cmd.h | 13 ++
> 2 files changed, 114 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index f8370b8..6f84ec7 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -750,7 +750,6 @@ static ssize_t host_show_hp_ssd_smart_path_enabled(struct
> device *dev,
> }
>
> #define MAX_PATHS 8
> -
> static ssize_t path_info_show(struct device *dev,
>struct device_attribute *attr, char *buf)
> {
> @@ -792,9 +791,7 @@ static ssize_t path_info_show(struct device *dev,
> hdev->bus, hdev->target, hdev->lun,
> scsi_device_type(hdev->devtype));
>
> - if (hdev->external ||
> - hdev->devtype == TYPE_RAID ||
> - is_logical_device(hdev)) {
> + if (hdev->devtype == TYPE_RAID || is_logical_device(hdev)) {
How does removing the check for external here relate to the rest of this commit?
> output_len += scnprintf(buf + output_len,
> PAGE_SIZE - output_len,
> "%s\n", active);
> @@ -808,8 +805,7 @@ static ssize_t path_info_show(struct device *dev,
> phys_connector[0] = '0';
> if (phys_connector[1] < '0')
> phys_connector[1] = '0';
> - if (hdev->phys_connector[i] > 0)
> - output_len += scnprintf(buf + output_len,
> + output_len += scnprintf(buf + output_len,
Same question regarding the removal of the > 0 check.
> PAGE_SIZE - output_len,
> "PORT: %.2s ",
> phys_connector);
> @@ -3191,6 +3187,90 @@ out:
> return rc;
> }
>
> +/*
> + * get enclosure information
> + * struct ReportExtendedLUNdata *rlep - Used for BMIC drive number
> + * struct hpsa_scsi_dev_t *encl_dev - device entry for enclosure
> + * Uses id_physical_device to determine the box_index.
> + */
> +static int hpsa_get_enclosure_info(struct ctlr_info *h,
> + unsigned char *scsi3addr,
> + struct ReportExtendedLUNdata *rlep, int rle_index,
> + struct hpsa_scsi_dev_t *encl_dev)
> +{
> + int rc = -1;
> + struct CommandList *c = NULL;
> + struct ErrorInfo *ei = NULL;
> + struct bmic_sense_storage_box_params *bssbp = NULL;
> + struct bmic_identify_physical_device *id_phys = NULL;
> + struct ext_report_lun_entry *rle = &rlep->LUN[rle_index];
> + u16 bmic_device_index = 0;
> +
> +
> + bssbp = kzalloc(sizeof(*bssbp), GFP_KERNEL);
> + if (!bssbp)
> + goto out;
> +
> + id_phys = kzalloc(sizeof(*id_phys), GFP_KERNEL);
> + if (!id_phys)
> + goto out;
> +
> + bmic_device_index = GET_BMIC_DRIVE_NUMBER(&rle->lunid[0]);
> +
> + if (bmic_device_index == 0xFF00)
> + goto out;
Why not put this check before the memory allocations so you can
avoid them if this condition is not met?
> +
> + memset(id_phys, 0, sizeof(*id_phys));
Didn't you just obtain zeroed memory from kzalloc()?
> + rc = hpsa_bmic_id_physical_device(h, scsi3addr, bmic_device_index,
> + id_phys, sizeof(*id_phys));
> + if (rc) {
> + dev_warn(&h->pdev->dev, "%s: id_phys failed %d bdi[0x%x]\n",
> + __func__, encl_dev->external, bmic_device_index);
> + goto out;
> + }
> +
> + c = cmd_alloc(h);
> +
> + rc = fill_cmd(c, BMIC_SENSE_STORAGE_BOX_PARAMS, h, bssbp,
> + sizeof(*bssbp), 0, RAID_CTLR_LUNID, TYPE_CMD);
> +
> + if (rc)
> + goto out;
> +
> + if (id_phys->phys_connector[1] == 'E')
> + c->Request.CDB[5] = id_phys->box_index;
> + else
> + c->Request.CDB[5] = 0;
> +
> + rc = hpsa_scsi_do_simple_cmd_with_retry(h, c, PCI_DMA_FROMDEVICE,
> + NO_TIMEOUT);
> + if (rc)
> + goto out;
> +
> + ei = c->err_info;
> + if (ei->CommandStatus != 0 && ei->CommandStatus != CMD_DATA_UNDERRUN) {
> + rc = -1;
> + goto out;
> + }
> +
> + encl_dev->box[id_phys->active_path_number] = bssbp->phys_box_on_port;
> + memcpy(&encl_dev->phys_connector[id_phys->active_path_number],
> + bssbp->phys_connector, sizeof(bssbp->phys_connector));
> +
> + rc = IO_OK;
> +out:
> + kfree(bssbp);
> + kfree(id_phys);
> +
> + if (c)
> + cmd_free(h, c);
> +
> + if (rc != IO_OK)
> + hpsa_show_dev_msg
Re: [Bug 108771] scsi: ses: kasan: ses_enclosure_data_process use after free on boot SAS2X28
On Wed, 2015-12-09 at 15:35 +0300, Pavel Tikhomirov wrote: > > On 12/08/2015 07:16 PM, James Bottomley wrote: > > On Mon, 2015-12-07 at 14:01 +, bugzilla-dae...@bugzilla.kernel.org > > wrote: > >> https://bugzilla.kernel.org/show_bug.cgi?id=108771 > >> > >> --- Comment #1 from Pavel Tikhomirov --- > >> Aditional info about enclosue(from that node, but older 3.10 based kernel): > >> > >> [root@p9 crash]# modprobe sg > >> [root@p9 crash]# sg_map -i > >> /dev/sg0 LSI SAS2X28 0e12 > >> /dev/sg1 /dev/sda LSI MR9260-4i 2.13 > >> [root@p9 crash]# lsscsi -gs > >> [1:0:16:0] enclosu LSI SAS2X28 0e12 - /dev/sg0 > >> - > >> [1:2:0:0]diskLSI MR9260-4i2.13 /dev/sda /dev/sg1 > >> 3.99TB > >> [root@p9 crash]# sg_ses /dev/sg0 > >>LSI SAS2X28 0e12 > >> Supported diagnostic pages: > >>Supported Diagnostic Pages [sdp] [0x0] > >>Configuration (SES) [cf] [0x1] > >>Enclosure Status/Control (SES) [ec,es] [0x2] > >>Element Descriptor (SES) [ed] [0x7] > >>Additional Element Status (SES-2) [aes] [0xa] > >>Download Microcode (SES-2) [dm] [0xe] > >> [root@p9 crash]# sg_ses /dev/sg1 > >>LSI MR9260-4i 2.13 > >> disk device (not an enclosure) > >> Supported diagnostic pages: > > > > OK, can you give us the contents of pages 1, 2 and 10 with > > > > sg_ses --page=1 --hex /dev/sg0 > > sg_ses --page=2 --hex /dev/sg0 > > sg_ses --page=10 --hex /dev/sg0 > > > > The version of the kernel you do this on doesn't really matter. > > Here are these pages: > > [root@p9 ~]# sg_ses --page=1 --hex /dev/sg0 >LSI SAS2X28 0e12 > Response in hex from diagnostic page: Configuration (SES) > 00 01 00 00 c9 00 00 00 00 11 00 09 2c 50 03 04 80 > ...,P... > 10 00 a7 1e bf 4c 53 49 20 20 20 20 20 53 41 53 32LSI > SAS2 > 20 58 32 38 20 20 20 20 20 20 20 20 20 30 65 31 32X28 > 0e12 > 30 11 22 33 44 55 00 00 00 17 0c 00 0b 04 01 00 13 > ."3DU... > 40 03 03 00 04 12 02 00 0f 02 02 00 0e 0e 01 00 09 > > 50 18 01 00 0d 19 0e 00 0e 11 02 00 0e 44 72 69 76 > Driv > 60 65 20 53 6c 6f 74 73 54 65 6d 70 65 72 61 74 75e > SlotsTemperatu > 70 72 65 20 53 65 6e 73 6f 72 73 46 61 6e 73 56 6fre > SensorsFansVo > 80 6c 74 61 67 65 20 53 65 6e 73 6f 72 73 50 6f 77ltage > SensorsPow > 90 65 72 20 53 75 70 70 6c 69 65 73 45 6e 63 6c 6fer > SuppliesEnclo > a0 73 75 72 65 53 41 53 20 45 78 70 61 6e 64 65 72sureSAS > Expander > b0 73 53 41 53 20 43 6f 6e 6e 65 63 74 6f 72 73 45sSAS > ConnectorsE > c0 74 68 65 72 6e 65 74 20 70 6f 72 74 73 thernet ports Wow, that's some crazy enclosure. The description says it's a single primary subenclosure with 9 different element types comprising 12 Device slots, 1 temperature sensor, 3 fans, 2 voltage sensors, 2 power supplies, 1 Enclosure, 1 SAS Expander, 14 SAS connectors, 2 Communications ports. For 38 total element descriptors > [root@p9 ~]# sg_ses --page=2 --hex /dev/sg0 >LSI SAS2X28 0e12 > Response in hex from diagnostic page: Enclosure Status (SES) > 00 02 00 00 c0 00 00 00 00 00 00 00 00 05 00 00 00 > > 10 05 00 00 00 01 00 00 00 05 00 00 00 05 00 00 00 > > 20 01 00 00 00 05 00 00 00 05 00 00 00 01 00 00 00 > > 30 05 00 00 00 05 00 00 00 01 00 00 00 00 00 00 00 > > 40 01 00 2c 00 00 00 00 00 05 00 00 50 05 00 00 50 > ..,P...P > 50 05 00 00 50 00 00 00 00 01 00 01 f9 01 00 04 b3 > ...P > 60 00 00 00 00 47 80 00 20 47 80 00 20 00 00 00 00G.. G.. > > 70 01 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 > > 80 01 11 ff 00 01 11 ff 00 01 20 00 00 01 20 00 00. > ... .. > 90 01 20 00 00 01 20 00 00 01 20 00 00 01 20 00 00. ... ... > ... .. > a0 01 20 00 00 01 20 00 00 01 20 00 00 01 20 00 00. ... ... > ... .. > b0 01 20 00 00 01 20 00 00 00 00 00 00 00 00 00 00. ... > .. > c0 00 00 00 00 Given each type has one overall descriptor followed by the individual ones, we have 38 + 9 = 47 total descriptors, which is what we see here. > [root@p9 ~]# sg_ses --page=10 --hex /dev/sg0 >LSI SAS2X28 0e12 > Response in hex from diagnostic page: Additional Element Status (SES-2) > 00 0a 00 01 fc 00 00 00 00 16 22 00 00 01 00 00 00 > .".. > 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > 20 00 00 00 00 00 00 00 00 00 00 00 00 16 22 00 01 > .".. > 30 01 00 00 01 00 00 00 00 00 00 00 00 00 00 00 00 > > 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > 50 16 22 00 02 01 00 00 02 00 00 00 01 50
Re: [PATCH 09/20] qla2xxx: Change check_stop_free to always return 1
On 12/8/15, 10:56 PM, "target-devel-ow...@vger.kernel.org on behalf of
Hannes Reinecke" wrote:
>On 12/08/2015 01:48 AM, Himanshu Madhani wrote:
>> From: Quinn Tran
>>
>> change tcm_qla2xxx_check_stop_free to always return 1
>> to prevent transport_cmd_finish_abort from accidently
>> taking extra kref_put.
>>
>> Signed-off-by: Quinn Tran
>> Signed-off-by: Himanshu Madhani
>> ---
>> drivers/scsi/qla2xxx/tcm_qla2xxx.c |3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
>>b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
>> index 74c6e9b..366142a 100644
>> --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
>> +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
>> @@ -314,7 +314,8 @@ static int tcm_qla2xxx_check_stop_free(struct
>>se_cmd *se_cmd)
>> cmd->cmd_flags |= BIT_14;
>> }
>>
>> -return target_put_sess_cmd(se_cmd);
>> +target_put_sess_cmd(se_cmd);
>> +return 1;
>> }
>>
>> /* tcm_qla2xxx_release_cmd - Callback from TCM Core to release
>>underlying
>>
>Odd. I would have expected target_put_sess_cmd() to never fail.
QT> It does not failed under normal usage.
>But if it does, doesn't it mean that the command is hosed, and we should
>have accessed it in the first place?
>Very suspicious.
QT> here are the cases I see. Note: The patch series is in reference of
pre-Bart¹s fix.
- cmd_kref 2 -> 1, target_put_sess_cmd return true. This is Normal case.
Backend finished with the command and send it to the Front end.
- cmd_kref 1 -> 0, target_put_sess_cmd return true.
Case 1: Early free by TCM/TMR thread. TMR thread call 2nd check_stop.
core_tmr_drain_tmr_list->transport_cmd_finish_abort->
transport_cmd_check_stop_to_fabric
- cmd_kref 0 -> -1, target_put_sess_cmd return false. QLA does not call
³check_stop² in either cases below.
Case 1: QLA cause double free the command as the command is coming out of
HW queue. In other word QLA access stale pointer during cleanup/free.
Case 2: QLA driver beat TMR thread by a few nano second by freeing the
command 1st(cmd_kref=0). TMR thread call the extra check_stop(cmd_kref =
-1). transport_cmd_finish_abort do additional kref_put (cmd_kref = -2).
void transport_cmd_finish_abort(struct se_cmd *cmd, int remove)
{
if (transport_cmd_check_stop_to_fabric(cmd))
return;
if (remove)
transport_put_cmd(cmd);
}
>
>Cheers,
>
>Hannes
>--
>Dr. Hannes Reinecke zSeries & Storage
>h...@suse.de +49 911 74053 688
>SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
>GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
>--
>To unsubscribe from this list: send the line "unsubscribe target-devel" in
>the body of a message to majord...@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
<>
[BISECTED] WARNING: CPU: 2 PID: 142 at block/genhd.c:626 add_disk+0x480/0x4e0()
Hi,
We received a report (https://bugzilla.redhat.com/show_bug.cgi?id=1288687) that
live images with the rawhide kernel were failing to boot on USB sticks.
Similar issues were reported when just inserting a USB stick into a boot from a
CD instead of USB ("I see /dev/sdb, but no /dev/sdb1 etc." per the report)
I reduced the test scenario to:
1) insert scsi_dh_alua module
2) insert Live USB drive
which gives
[ 125.107185] sd 6:0:0:0: alua: supports implicit and explicit TPGS
[ 125.107778] sd 6:0:0:0: [sdb] 15634432 512-byte logical blocks: (8.00 GB/7.46
GiB)
[ 125.107973] sd 6:0:0:0: alua: No target port descriptors found
[ 125.107975] sd 6:0:0:0: alua: Attach failed (-22)
[ 125.107978] sd 6:0:0:0: failed to add device handler: -22
[ 125.108462] sd 6:0:0:0: [sdb] Write Protect is off
[ 125.108465] sd 6:0:0:0: [sdb] Mode Sense: 43 00 00 00
[ 125.108468] sd 6:0:0:0: [sdb] Asking for cache data failed
[ 125.108469] sd 6:0:0:0: [sdb] Assuming drive cache: write through
[ 125.109122] [ cut here ]
[ 125.109127] WARNING: CPU: 2 PID: 142 at block/genhd.c:626
add_disk+0x480/0x4e0()
[ 125.109128] Modules linked in: uas usb_storage scsi_dh_alua fuse xt_CHECKSUM
ipt_MASQUERADE nf_nat_masquerade_ipv4 ccm tun nf_conntrack_netbios_ns
nf_conntrack_broadcast ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack
ebtable_filter ebtable_nat ebtable_broute bridge stp llc ebtables ip6table_raw
ip6table_security ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6
ip6table_mangle ip6table_filter ip6_tables iptable_raw iptable_security
iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack
iptable_mangle bnep snd_hda_codec_hdmi arc4 iwlmvm mac80211 i915 intel_rapl
iosf_mbi x86_pkg_temp_thermal coretemp iwlwifi kvm_intel kvm
snd_hda_codec_realtek uvcvideo snd_hda_codec_generic btusb snd_hda_intel btrtl
videobuf2_vmalloc cfg80211 snd_hda_codec btbcm iTCO_wdt videobuf2_v4l2
[ 125.109164] btintel iTCO_vendor_support videobuf2_core irqbypass
videobuf2_memops bluetooth v4l2_common snd_hda_core ghash_clmulni_intel
videodev snd_hwdep snd_seq media pcspkr joydev snd_seq_device rtsx_pci_ms
snd_pcm memstick thinkpad_acpi snd_timer mei_me snd i2c_algo_bit mei
drm_kms_helper ie31200_edac rfkill tpm_tis edac_core shpchp soundcore tpm
i2c_i801 lpc_ich wmi nfsd auth_rpcgss nfs_acl lockd grace sunrpc binfmt_misc
dm_crypt hid_microsoft rtsx_pci_sdmmc mmc_core crct10dif_pclmul crc32_pclmul
crc32c_intel serio_raw drm e1000e ptp rtsx_pci pps_core fjes video
[ 125.109197] CPU: 2 PID: 142 Comm: kworker/u16:6 Tainted: G W
4.4.0-rc4-usbbadness-next-20151209+ #3
[ 125.109198] Hardware name: LENOVO 20BFS0EC00/20BFS0EC00, BIOS GMET62WW (2.10
) 03/19/2014
[ 125.109202] Workqueue: events_unbound async_run_entry_fn
[ 125.109204] 202f2ede 880402ccfc38
81434509
[ 125.109206] 880402ccfc70 810ad9c2
880407a1e000
[ 125.109208] 880407a1e0b0 880407a1e00c 880401e48ef0
8800c90d0600
[ 125.109211] Call Trace:
[ 125.109214] [] dump_stack+0x4b/0x72
[ 125.109218] [] warn_slowpath_common+0x82/0xc0
[ 125.109220] [] warn_slowpath_null+0x1a/0x20
[ 125.109222] [] add_disk+0x480/0x4e0
[ 125.109225] [] sd_probe_async+0x115/0x1d0
[ 125.109227] [] async_run_entry_fn+0x4a/0x140
[ 125.109231] [] process_one_work+0x239/0x6b0
[ 125.109233] [] ? process_one_work+0x1a2/0x6b0
[ 125.109235] [] worker_thread+0x4e/0x490
[ 125.109237] [] ? process_one_work+0x6b0/0x6b0
[ 125.109238] [] kthread+0x101/0x120
[ 125.109242] [] ? trace_hardirqs_on_caller+0x129/0x1b0
[ 125.109243] [] ? kthread_create_on_node+0x250/0x250
[ 125.109247] [] ret_from_fork+0x3f/0x70
[ 125.109248] [] ? kthread_create_on_node+0x250/0x250
[ 125.109250] ---[ end trace d54b73ed8d1295d5 ]---
[ 125.109272] sd 6:0:0:0: [sdb] Attached SCSI removable disk
and no partitions so the drive can't be mounted. Note the alua -EINVAL
error is there even when the drive can be mounted so the warning and
lack of partitions is the real indication of the problem.
I did a bisect and came up with this as the first bad commit:
commit 086b91d052ebe4ead5d28021afe3bdfd70af15bf
Author: Christoph Hellwig
Date: Thu Aug 27 14:16:57 2015 +0200
scsi_dh: integrate into the core SCSI code
Stop building scsi_dh as a separate module and integrate it fully into the
core SCSI code with explicit callouts at bus scan time. For now the
callouts are placed at the same point as the old bus notifiers were called,
but in the future we will be able to look at ALUA INQUIRY data earlier on.
Note that this also means that the device handler modules need to be loaded
by the time we scan the bus. The next patches will add support for
autoloading device handlers at bus scan time to make sure they are always
loaded if they are enabled in the kernel config.
Signed-off-by: Christoph Hellwig
Reviewed-by: Martin K. Petersen
Reviewed-by: Hannes Reinecke
Acked-by: Mike Snitzer
Signed-off-by: James Bottomley
This
Re: [BISECTED] WARNING: CPU: 2 PID: 142 at block/genhd.c:626 add_disk+0x480/0x4e0()
On 12/10/2015 05:00 AM, Laura Abbott wrote:
Hi,
We received a report
(https://bugzilla.redhat.com/show_bug.cgi?id=1288687) that
live images with the rawhide kernel were failing to boot on USB sticks.
Similar issues were reported when just inserting a USB stick into a
boot from a
CD instead of USB ("I see /dev/sdb, but no /dev/sdb1 etc." per the
report)
I reduced the test scenario to:
1) insert scsi_dh_alua module
2) insert Live USB drive
which gives
[ 125.107185] sd 6:0:0:0: alua: supports implicit and explicit TPGS
[ 125.107778] sd 6:0:0:0: [sdb] 15634432 512-byte logical blocks:
(8.00 GB/7.46 GiB)
[ 125.107973] sd 6:0:0:0: alua: No target port descriptors found
[ 125.107975] sd 6:0:0:0: alua: Attach failed (-22)
[ 125.107978] sd 6:0:0:0: failed to add device handler: -22
[ 125.108462] sd 6:0:0:0: [sdb] Write Protect is off
[ 125.108465] sd 6:0:0:0: [sdb] Mode Sense: 43 00 00 00
[ 125.108468] sd 6:0:0:0: [sdb] Asking for cache data failed
[ 125.108469] sd 6:0:0:0: [sdb] Assuming drive cache: write through
[ 125.109122] [ cut here ]
[ 125.109127] WARNING: CPU: 2 PID: 142 at block/genhd.c:626
add_disk+0x480/0x4e0()
[ 125.109128] Modules linked in: uas usb_storage scsi_dh_alua fuse
xt_CHECKSUM
ipt_MASQUERADE nf_nat_masquerade_ipv4 ccm tun nf_conntrack_netbios_ns
nf_conntrack_broadcast ip6t_rpfilter ip6t_REJECT nf_reject_ipv6
xt_conntrack
ebtable_filter ebtable_nat ebtable_broute bridge stp llc ebtables
ip6table_raw
ip6table_security ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6
nf_nat_ipv6
ip6table_mangle ip6table_filter ip6_tables iptable_raw iptable_security
iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat
nf_conntrack
iptable_mangle bnep snd_hda_codec_hdmi arc4 iwlmvm mac80211 i915
intel_rapl
iosf_mbi x86_pkg_temp_thermal coretemp iwlwifi kvm_intel kvm
snd_hda_codec_realtek uvcvideo snd_hda_codec_generic btusb
snd_hda_intel btrtl
videobuf2_vmalloc cfg80211 snd_hda_codec btbcm iTCO_wdt videobuf2_v4l2
[ 125.109164] btintel iTCO_vendor_support videobuf2_core irqbypass
videobuf2_memops bluetooth v4l2_common snd_hda_core ghash_clmulni_intel
videodev snd_hwdep snd_seq media pcspkr joydev snd_seq_device
rtsx_pci_ms
snd_pcm memstick thinkpad_acpi snd_timer mei_me snd i2c_algo_bit mei
drm_kms_helper ie31200_edac rfkill tpm_tis edac_core shpchp
soundcore tpm
i2c_i801 lpc_ich wmi nfsd auth_rpcgss nfs_acl lockd grace sunrpc
binfmt_misc
dm_crypt hid_microsoft rtsx_pci_sdmmc mmc_core crct10dif_pclmul
crc32_pclmul
crc32c_intel serio_raw drm e1000e ptp rtsx_pci pps_core fjes video
[ 125.109197] CPU: 2 PID: 142 Comm: kworker/u16:6 Tainted: G W
4.4.0-rc4-usbbadness-next-20151209+ #3
[ 125.109198] Hardware name: LENOVO 20BFS0EC00/20BFS0EC00, BIOS
GMET62WW (2.10 ) 03/19/2014
[ 125.109202] Workqueue: events_unbound async_run_entry_fn
[ 125.109204] 202f2ede 880402ccfc38
81434509
[ 125.109206] 880402ccfc70 810ad9c2
880407a1e000
[ 125.109208] 880407a1e0b0 880407a1e00c 880401e48ef0
8800c90d0600
[ 125.109211] Call Trace:
[ 125.109214] [] dump_stack+0x4b/0x72
[ 125.109218] [] warn_slowpath_common+0x82/0xc0
[ 125.109220] [] warn_slowpath_null+0x1a/0x20
[ 125.109222] [] add_disk+0x480/0x4e0
[ 125.109225] [] sd_probe_async+0x115/0x1d0
[ 125.109227] [] async_run_entry_fn+0x4a/0x140
[ 125.109231] [] process_one_work+0x239/0x6b0
[ 125.109233] [] ? process_one_work+0x1a2/0x6b0
[ 125.109235] [] worker_thread+0x4e/0x490
[ 125.109237] [] ? process_one_work+0x6b0/0x6b0
[ 125.109238] [] kthread+0x101/0x120
[ 125.109242] [] ?
trace_hardirqs_on_caller+0x129/0x1b0
[ 125.109243] [] ? kthread_create_on_node+0x250/0x250
[ 125.109247] [] ret_from_fork+0x3f/0x70
[ 125.109248] [] ? kthread_create_on_node+0x250/0x250
[ 125.109250] ---[ end trace d54b73ed8d1295d5 ]---
[ 125.109272] sd 6:0:0:0: [sdb] Attached SCSI removable disk
and no partitions so the drive can't be mounted. Note the alua -EINVAL
error is there even when the drive can be mounted so the warning and
lack of partitions is the real indication of the problem.
I did a bisect and came up with this as the first bad commit:
commit 086b91d052ebe4ead5d28021afe3bdfd70af15bf
Author: Christoph Hellwig
Date: Thu Aug 27 14:16:57 2015 +0200
scsi_dh: integrate into the core SCSI code
Stop building scsi_dh as a separate module and integrate it fully
into the
core SCSI code with explicit callouts at bus scan time. For now the
callouts are placed at the same point as the old bus notifiers were
called,
but in the future we will be able to look at ALUA INQUIRY data
earlier on.
Note that this also means that the device handler modules need to be
loaded
by the time we scan the bus. The next patches will add support for
autoloading device handlers at bus scan time to make sure they are
always
loaded if they are enabled in the kernel config.
Signed-off-by: Christoph Hellwig
Reviewed-by: Martin K. Petersen
Reviewed-by: Hannes Reinecke
Acked-by: Mike Snitzer
