[PATCH] megaraid_sas: Fix probing cards without io port

[Yinghai Lu]

Found one megaraid_sas HBA probe fails,

[  187.235190] scsi host2: Avago SAS based MegaRAID driver
[  191.112365] megaraid_sas :89:00.0: BAR 0: can't reserve [io  
0x-0x00ff]
[  191.120548] megaraid_sas :89:00.0: IO memory region busy!

and the card has resource like,
[  125.097714] pci :89:00.0: [1000:005d] type 00 class 0x010400
[  125.104446] pci :89:00.0: reg 0x10: [io  0x-0x00ff]
[  125.110686] pci :89:00.0: reg 0x14: [mem 0xce40-0xce40 64bit]
[  125.118286] pci :89:00.0: reg 0x1c: [mem 0xce30-0xce3f 64bit]
[  125.125891] pci :89:00.0: reg 0x30: [mem 0xce20-0xce2f pref]

that does not io port resource allocated from BIOS, and kernel can not assign
one as io port shortage.

The driver is only looking for MEM, and should not fail.

It turns out megasas_init_fw() etc are using bar index as mask.
index 1 is used as mask 1, so that pci_request_selected_regions()
is trying to request BAR0 instead of BAR1.

Fix all related reference.

Fixes: b6d5d8808b4c ("megaraid_sas: Use lowest memory bar for SR-IOV VF 
support")
Signed-off-by: Yinghai Lu <ying...@kernel.org>

---
 drivers/scsi/megaraid/megaraid_sas_base.c   |6 +++---
 drivers/scsi/megaraid/megaraid_sas_fusion.c |2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

Index: linux-2.6/drivers/scsi/megaraid/megaraid_sas_base.c
===
--- linux-2.6.orig/drivers/scsi/megaraid/megaraid_sas_base.c
+++ linux-2.6/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -5037,7 +5037,7 @@ static int megasas_init_fw(struct megasa
/* Find first memory bar */
bar_list = pci_select_bars(instance->pdev, IORESOURCE_MEM);
instance->bar = find_first_bit(_list, sizeof(unsigned long));
-   if (pci_request_selected_regions(instance->pdev, instance->bar,
+   if (pci_request_selected_regions(instance->pdev, 1<bar,
 "megasas: LSI")) {
dev_printk(KERN_DEBUG, >pdev->dev, "IO memory region 
busy!\n");
return -EBUSY;
@@ -5339,7 +5339,7 @@ fail_ready_state:
iounmap(instance->reg_set);
 
   fail_ioremap:
-   pci_release_selected_regions(instance->pdev, instance->bar);
+   pci_release_selected_regions(instance->pdev, 1<bar);
 
return -EINVAL;
 }
@@ -5360,7 +5360,7 @@ static void megasas_release_mfi(struct m
 
iounmap(instance->reg_set);
 
-   pci_release_selected_regions(instance->pdev, instance->bar);
+   pci_release_selected_regions(instance->pdev, 1<bar);
 }
 
 /**
Index: linux-2.6/drivers/scsi/megaraid/megaraid_sas_fusion.c
===
--- linux-2.6.orig/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ linux-2.6/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -2603,7 +2603,7 @@ megasas_release_fusion(struct megasas_in
 
iounmap(instance->reg_set);
 
-   pci_release_selected_regions(instance->pdev, instance->bar);
+   pci_release_selected_regions(instance->pdev, 1<bar);
 }
 
 /**
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/25] mpt3sas: Mergering mpt2sas & mpt3sas driver code

[Yinghai Lu]

On Wed, Nov 11, 2015 at 5:09 PM, Martin K. Petersen
 wrote:
>> "Sreekanth" == Sreekanth Reddy  writes:
>
> The patches in the single-module portion of the series did not compile
> individually and I gave up untangling them. They were fundamentally too
> intertwined and I squashed them into one commit. Since it's mostly
> boilerplate stuff it should not matter too much from a bisection
> perspective.

on opensuse 13.1 gcc
gcc --version
gcc (SUSE Linux) 4.8.1 20130909 [gcc-4_8-branch revision 202388]

got:

In file included from drivers/scsi/mpt3sas/mpt3sas_scsih.c:59:0:
drivers/scsi/mpt3sas/mpt3sas_scsih.c: In function ‘_scsih_io_done’:
drivers/scsi/mpt3sas/mpt3sas_base.h:1414:1: error: inlining failed in
call to always_inline ‘mpt3sas_scsi_direct_io_get’: function body not
available
 mpt3sas_scsi_direct_io_get(struct MPT3SAS_ADAPTER *ioc, u16 smid);
 ^
drivers/scsi/mpt3sas/mpt3sas_scsih.c:4448:32: error: called from here
  if (mpt3sas_scsi_direct_io_get(ioc, smid) &&
^
In file included from drivers/scsi/mpt3sas/mpt3sas_scsih.c:59:0:
drivers/scsi/mpt3sas/mpt3sas_base.h:1416:1: error: inlining failed in
call to always_inline ‘mpt3sas_scsi_direct_io_set’: function body not
available
 mpt3sas_scsi_direct_io_set(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8
direct_io);
 ^
drivers/scsi/mpt3sas/mpt3sas_scsih.c:4454:29: error: called from here
   mpt3sas_scsi_direct_io_set(ioc, smid, 0);
--
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] [SCSI] mpt2sas, mpt3sas: Abort initialization if no memory I/O resources detected

[Yinghai Lu]

On Wed, Jul 15, 2015 at 6:52 AM, Timothy Pearson
tpear...@raptorengineeringinc.com wrote:
 I have just kept the same description provide by Timothy in his
 initial patch.

 But I observe that their may be chance of getting unable to handle
 kernel NULL pointer dereference kernel panic if no Memory Resource
 available in the PCI subsystem. So agreed to the Timothy proposal of
 aborting the driver initialization if it doesn't detect any Memory
 resource instead of whole system get into panic state.

 On some systems Linux is unable / unwilling to assign a BAR if the BIOS
 does not assign one at startup.  I didn't look into the Linux allocator
 side of things in much detail, but it is quite possible that Linux is
 unaware the device only has partial resources assigned.


Would be great if you can post boot log so we can figure about why those
BARs are not assigned.

Yinghai
--
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] [SCSI] mpt2sas, mpt3sas: Abort initialization if no memory I/O resources detected

[Yinghai Lu]

On Tue, Jul 14, 2015 at 9:49 PM, Sreekanth Reddy
sreekanth.re...@avagotech.com wrote:
 Driver crashes if the BIOS do not set up at least one
 memory I/O resource. This failure can happen if the device is too
 slow to respond during POST and is missed by the BIOS, but Linux
 then detects the device later in the boot process.

But pci subsystem should assign resources to those unassigned BAR.

Do you mean even kernel can not assign resource to them? or it takes so long for
mpt FW to get ready?

Thanks

Yinghai
--
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 v4 1/5] PCI: pcibus address to resource converting take bus instead of dev

[Yinghai Lu]

For allocating resource under bus path, we do not have dev to pass along,
and we only have bus to use instead.

-v2: drop pcibios_bus_addr_to_resource().
-v3: drop __* change requested by Bjorn.

Signed-off-by: Yinghai Lu ying...@kernel.org
Cc: linux-al...@vger.kernel.org
Cc: linuxppc-...@lists.ozlabs.org
Cc: sparcli...@vger.kernel.org
Cc: linux-pcm...@lists.infradead.org
Cc: linux-scsi@vger.kernel.org
Cc: linux-fb...@vger.kernel.org
---
 arch/alpha/kernel/pci-sysfs.c   |  4 ++--
 arch/powerpc/kernel/pci-common.c|  4 ++--
 arch/powerpc/kernel/pci_of_scan.c   |  4 ++--
 arch/sparc/kernel/pci.c |  6 +++---
 drivers/pci/host-bridge.c   | 24 +++-
 drivers/pci/probe.c | 18 +-
 drivers/pci/quirks.c|  2 +-
 drivers/pci/rom.c   |  2 +-
 drivers/pci/setup-bus.c | 16 
 drivers/pci/setup-res.c |  2 +-
 drivers/pcmcia/i82092.c |  2 +-
 drivers/pcmcia/yenta_socket.c   |  6 +++---
 drivers/scsi/sym53c8xx_2/sym_glue.c |  5 +++--
 drivers/video/arkfb.c   |  2 +-
 drivers/video/s3fb.c|  2 +-
 drivers/video/vt8623fb.c|  2 +-
 include/linux/pci.h |  4 ++--
 17 files changed, 52 insertions(+), 53 deletions(-)

diff --git a/arch/alpha/kernel/pci-sysfs.c b/arch/alpha/kernel/pci-sysfs.c
index 2b183b0..99e8d47 100644
--- a/arch/alpha/kernel/pci-sysfs.c
+++ b/arch/alpha/kernel/pci-sysfs.c
@@ -83,7 +83,7 @@ static int pci_mmap_resource(struct kobject *kobj,
if (iomem_is_exclusive(res-start))
return -EINVAL;
 
-   pcibios_resource_to_bus(pdev, bar, res);
+   pcibios_resource_to_bus(pdev-bus, bar, res);
vma-vm_pgoff += bar.start  (PAGE_SHIFT - (sparse ? 5 : 0));
mmap_type = res-flags  IORESOURCE_MEM ? pci_mmap_mem : pci_mmap_io;
 
@@ -139,7 +139,7 @@ static int sparse_mem_mmap_fits(struct pci_dev *pdev, int 
num)
long dense_offset;
unsigned long sparse_size;
 
-   pcibios_resource_to_bus(pdev, bar, pdev-resource[num]);
+   pcibios_resource_to_bus(pdev-bus, bar, pdev-resource[num]);
 
/* All core logic chips have 4G sparse address space, except
   CIA which has 16G (see xxx_SPARSE_MEM and xxx_DENSE_MEM
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index a1e3e40..d9476c1 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -835,7 +835,7 @@ static void pcibios_fixup_resources(struct pci_dev *dev)
 * at 0 as unset as well, except if PCI_PROBE_ONLY is also set
 * since in that case, we don't want to re-assign anything
 */
-   pcibios_resource_to_bus(dev, reg, res);
+   pcibios_resource_to_bus(dev-bus, reg, res);
if (pci_has_flag(PCI_REASSIGN_ALL_RSRC) ||
(reg.start == 0  !pci_has_flag(PCI_PROBE_ONLY))) {
/* Only print message if not re-assigning */
@@ -886,7 +886,7 @@ static int pcibios_uninitialized_bridge_resource(struct 
pci_bus *bus,
 
/* Job is a bit different between memory and IO */
if (res-flags  IORESOURCE_MEM) {
-   pcibios_resource_to_bus(dev, region, res);
+   pcibios_resource_to_bus(dev-bus, region, res);
 
/* If the BAR is non-0 then it's probably been initialized */
if (region.start != 0)
diff --git a/arch/powerpc/kernel/pci_of_scan.c 
b/arch/powerpc/kernel/pci_of_scan.c
index ac0b034..83c26d8 100644
--- a/arch/powerpc/kernel/pci_of_scan.c
+++ b/arch/powerpc/kernel/pci_of_scan.c
@@ -111,7 +111,7 @@ static void of_pci_parse_addrs(struct device_node *node, 
struct pci_dev *dev)
res-name = pci_name(dev);
region.start = base;
region.end = base + size - 1;
-   pcibios_bus_to_resource(dev, res, region);
+   pcibios_bus_to_resource(dev-bus, res, region);
}
 }
 
@@ -280,7 +280,7 @@ void of_scan_pci_bridge(struct pci_dev *dev)
res-flags = flags;
region.start = of_read_number(ranges[1], 2);
region.end = region.start + size - 1;
-   pcibios_bus_to_resource(dev, res, region);
+   pcibios_bus_to_resource(dev-bus, res, region);
}
sprintf(bus-name, PCI Bus %04x:%02x, pci_domain_nr(bus),
bus-number);
diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index cb02145..7de8d1f 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -392,7 +392,7 @@ static void apb_fake_ranges(struct pci_dev *dev,
res-flags = IORESOURCE_IO;
region.start = (first  21);
region.end = (last  21) + ((1  21) - 1);
-   pcibios_bus_to_resource(dev, res, region);
+   pcibios_bus_to_resource(dev-bus, res, region);
 
pci_read_config_byte(dev

Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

[Yinghai Lu]

On Wed, May 15, 2013 at 7:39 AM, Liu Jiang liu...@gmail.com wrote:
 On Wed 15 May 2013 02:52:51 AM CST, Yinghai Lu wrote:

 On Tue, May 14, 2013 at 9:57 AM, Liu Jiang liu...@gmail.com wrote:

 On Tue 14 May 2013 11:10:33 PM CST, Yinghai Lu wrote:


 On Tue, May 14, 2013 at 7:59 AM, Liu Jiang liu...@gmail.com wrote:


 On 05/14/2013 04:26 PM, Gu Zheng wrote:
   I suggest to use pci_release_dev() instead because it also needs
 to
 release OF related resources.
 I will update it in next version.

 diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
 index bc075a3..2ac6338 100644
 --- a/drivers/pci/probe.c
 +++ b/drivers/pci/probe.c
 @@ -1281,7 +1281,7 @@ static struct pci_dev *pci_scan_device(struct
 pci_bus
 *bus
   pci_set_of_node(dev);

   if (pci_setup_device(dev)) {
 -   kfree(dev);
 +   pci_release_dev(dev-dev);
   return NULL;



 no, should move pci_set_of_node calling into pci_setup_device.

 Yinghai



 I'm not sure whether we should call pci_set_of_node() for SR-IOV devices
 too,
 any suggestions here?


 or just move down pci_set_of_node after pci_setup_device?

 anyway that is another bug.

 I'm not familiar with the OF logic and can't make sure whether
 pci_setup_device()
 has dependency on dev-of_node. Feel it's more safe to call
 pci_release_of_node()
 on failing path instead of tuning call-site of pci_set_of_node().

that is another bug, let of guy handle it.

Yinghai
--
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, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

[Yinghai Lu]

On Tue, May 14, 2013 at 7:59 AM, Liu Jiang liu...@gmail.com wrote:
 On 05/14/2013 04:26 PM, Gu Zheng wrote:
 I suggest to use pci_release_dev() instead because it also needs to
 release OF related resources.
 I will update it in next version.

 diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
 index bc075a3..2ac6338 100644
 --- a/drivers/pci/probe.c
 +++ b/drivers/pci/probe.c
 @@ -1281,7 +1281,7 @@ static struct pci_dev *pci_scan_device(struct pci_bus
 *bus
 pci_set_of_node(dev);

 if (pci_setup_device(dev)) {
 -   kfree(dev);
 +   pci_release_dev(dev-dev);
 return NULL;

no, should move pci_set_of_node calling into pci_setup_device.

Yinghai
--
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, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

[Yinghai Lu]

On Tue, May 14, 2013 at 9:57 AM, Liu Jiang liu...@gmail.com wrote:
 On Tue 14 May 2013 11:10:33 PM CST, Yinghai Lu wrote:

 On Tue, May 14, 2013 at 7:59 AM, Liu Jiang liu...@gmail.com wrote:

 On 05/14/2013 04:26 PM, Gu Zheng wrote:
  I suggest to use pci_release_dev() instead because it also needs to
 release OF related resources.
 I will update it in next version.

 diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
 index bc075a3..2ac6338 100644
 --- a/drivers/pci/probe.c
 +++ b/drivers/pci/probe.c
 @@ -1281,7 +1281,7 @@ static struct pci_dev *pci_scan_device(struct
 pci_bus
 *bus
  pci_set_of_node(dev);

  if (pci_setup_device(dev)) {
 -   kfree(dev);
 +   pci_release_dev(dev-dev);
  return NULL;


 no, should move pci_set_of_node calling into pci_setup_device.

 Yinghai


 I'm not sure whether we should call pci_set_of_node() for SR-IOV devices
 too,
 any suggestions here?

or just move down pci_set_of_node after pci_setup_device?

anyway that is another bug.

Yinghai
--
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, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

[Yinghai Lu]

On Mon, May 13, 2013 at 9:08 AM, Jiang Liu liu...@gmail.com wrote:
 From: Gu Zheng guz.f...@cn.fujitsu.com
 diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
 index 4f0bc0a..bc075a3 100644
 --- a/drivers/pci/probe.c
 +++ b/drivers/pci/probe.c
 @@ -1131,6 +1131,7 @@ static void pci_release_dev(struct device *dev)
 struct pci_dev *pci_dev;

 pci_dev = to_pci_dev(dev);
 +   pci_bus_put(pci_dev-bus);
 pci_release_capabilities(pci_dev);
 pci_release_of_node(pci_dev);
 kfree(pci_dev);
 @@ -1269,11 +1270,10 @@ static struct pci_dev *pci_scan_device(struct pci_bus 
 *bus, int devfn)
 if (!pci_bus_read_dev_vendor_id(bus, devfn, l, 60*1000))
 return NULL;

 -   dev = alloc_pci_dev();
 +   dev = pci_alloc_dev(bus);
 if (!dev)
 return NULL;

 -   dev-bus = bus;
 dev-devfn = devfn;
 dev-vendor = l  0x;
 dev-device = (l  16)  0x;

in pci_setup_device() fail path, it release the ref to that bus.

Yinghai
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

scsi_execute_req return 80002 in ses_match_to_enclosure.

[Yinghai Lu]

James,

i tried some sas disks in my system with expander with SES.

seagate and fuji disk can not get the VPD 0x83. and scsi_execute_req
got 0x80002.

but hitachi disk works well.

with lsscsi all disk can get sas address correctly.

so wonder if there is some other way to get the SAS address..

YH

static void ses_match_to_enclosure(struct enclosure_device *edev,
   struct scsi_device *sdev)
{
unsigned char *buf = kmalloc(VPD_INQUIRY_SIZE, GFP_KERNEL);
int result;
unsigned char *desc;
int len;
struct efd efd = {
.addr = 0,
};
unsigned char cmd[] = {
INQUIRY,
1,
0x83,
VPD_INQUIRY_SIZE  8,
VPD_INQUIRY_SIZE  0xff,
0
};

if (!buf)
return;

scsi_test_unit_ready(sdev, SD_TIMEOUT, SD_MAX_RETRIES, NULL);

result = scsi_execute_req(sdev,  cmd, DMA_FROM_DEVICE, buf,
VPD_INQUIRY_SIZE, NULL, SES_TIMEOUT, SES_RETRIES);
if (result) {
sdev_printk(KERN_INFO, sdev, ses_match_to_enclosure:
inquiry VPD page 0x83 failed with %x\n, result);
goto free;
}
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] SCSI: fix data corruption caused by ses v2

[Yinghai Lu]

On Friday 15 February 2008 07:53:06 am James Bottomley wrote:
 On Wed, 2008-02-13 at 16:25 -0800, Yinghai Lu wrote:
  one system: initrd get courrupted:
  
  RAMDISK: Compressed image found at block 0
  RAMDISK: incomplete write (-28 != 2048) 134217728
  crc error
  VFS: Mounted root (ext2 filesystem).
  Freeing unused kernel memory: 388k freed
  init_special_inode: bogus i_mode (17)
  Warning: unable to open an initial console.
  init_special_inode: bogus i_mode (17)
  init_special_inode: bogus i_mode (17)
  Kernel panic - not syncing: No init found.  Try passing init= option to 
  kernel.
  
  bisected to
  commit 9927c68864e9c39cc317b4f559309ba29e642168
  Author: James Bottomley [EMAIL PROTECTED]
  Date:   Sun Feb 3 15:48:56 2008 -0600
  
  [SCSI] ses: add new Enclosure ULD
  
  changes:
  1. change char to unsigned char to avoid type change later.
  2. preserve len for page1
  3. need to move desc_ptr even the entry is not 
  enclosure_component_device/raid.
 so keep desc_ptr on right position
  4. record page7 len, and double check if desc_ptr out of boundary before 
  touch.
  5. fix typo in subenclosure checking: should use hdr_buf instead.
  
  Signed-off-by: Yinghai Lu [EMAIL PROTECTED]
 
 OK, I added this with a fixup to eliminate the spurious goto
 

good
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] SCSI: fix data corruption caused by ses

[Yinghai Lu]

On Wednesday 13 February 2008 03:25:27 pm James Bottomley wrote:
 On Tue, 2008-02-12 at 23:10 -0800, Yinghai Lu wrote:
  if (type_ptr[0] != ENCLOSURE_COMPONENT_DEVICE 
  type_ptr[0] != ENCLOSURE_COMPONENT_ARRAY_DEVICE)
  -   continue;
  +   goto next;
  +
  ecomp = enclosure_component_register(edev,
   components++,
   type_ptr[0],
   name);
  +
  +   if (desc_ptr  !IS_ERR(ecomp)  addl_desc_ptr)
  +   ses_process_descriptor(ecomp,
  +  addl_desc_ptr);
  +   next:
  if (desc_ptr) {
  desc_ptr += len;
  -   if (!IS_ERR(ecomp))
  -   ses_process_descriptor(ecomp,
  -  addl_desc_ptr);
   
  if (addl_desc_ptr)
  addl_desc_ptr += addl_desc_ptr[1] + 2;
 
 Everything looks fine, thanks, except this piece.
 
 That 
 
 if (x)
  goto next;
 ...
 next:
 
 Needs to be
 
 if (!x) {
...
 }
 

find other problems about sub_enclosure...

will send you updated one.

YH
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] SCSI: fix data corruption caused by ses v2

[Yinghai Lu]

one system: initrd get courrupted:

RAMDISK: Compressed image found at block 0
RAMDISK: incomplete write (-28 != 2048) 134217728
crc error
VFS: Mounted root (ext2 filesystem).
Freeing unused kernel memory: 388k freed
init_special_inode: bogus i_mode (17)
Warning: unable to open an initial console.
init_special_inode: bogus i_mode (17)
init_special_inode: bogus i_mode (17)
Kernel panic - not syncing: No init found.  Try passing init= option to kernel.

bisected to
commit 9927c68864e9c39cc317b4f559309ba29e642168
Author: James Bottomley [EMAIL PROTECTED]
Date:   Sun Feb 3 15:48:56 2008 -0600

[SCSI] ses: add new Enclosure ULD

changes:
1. change char to unsigned char to avoid type change later.
2. preserve len for page1
3. need to move desc_ptr even the entry is not enclosure_component_device/raid.
   so keep desc_ptr on right position
4. record page7 len, and double check if desc_ptr out of boundary before touch.
5. fix typo in subenclosure checking: should use hdr_buf instead.

Signed-off-by: Yinghai Lu [EMAIL PROTECTED]

Index: linux-2.6/drivers/scsi/ses.c
===
--- linux-2.6.orig/drivers/scsi/ses.c
+++ linux-2.6/drivers/scsi/ses.c
@@ -33,9 +33,9 @@
 #include scsi/scsi_host.h
 
 struct ses_device {
-   char *page1;
-   char *page2;
-   char *page10;
+   unsigned char *page1;
+   unsigned char *page2;
+   unsigned char *page10;
short page1_len;
short page2_len;
short page10_len;
@@ -67,7 +66,7 @@ static int ses_probe(struct device *dev)
 static int ses_recv_diag(struct scsi_device *sdev, int page_code,
 void *buf, int bufflen)
 {
-   char cmd[] = {
+   unsigned char cmd[] = {
RECEIVE_DIAGNOSTIC,
1,  /* Set PCV bit */
page_code,
@@ -85,7 +84,7 @@ static int ses_send_diag(struct scsi_dev
 {
u32 result;
 
-   char cmd[] = {
+   unsigned char cmd[] = {
SEND_DIAGNOSTIC,
0x10,   /* Set PF bit */
0,
@@ -104,13 +103,13 @@ static int ses_send_diag(struct scsi_dev
 
 static int ses_set_page2_descriptor(struct enclosure_device *edev,
  struct enclosure_component *ecomp,
- char *desc)
+ unsigned char *desc)
 {
int i, j, count = 0, descriptor = ecomp-number;
struct scsi_device *sdev = to_scsi_device(edev-cdev.dev);
struct ses_device *ses_dev = edev-scratch;
-   char *type_ptr = ses_dev-page1 + 12 + ses_dev-page1[11];
-   char *desc_ptr = ses_dev-page2 + 8;
+   unsigned char *type_ptr = ses_dev-page1 + 12 + ses_dev-page1[11];
+   unsigned char *desc_ptr = ses_dev-page2 + 8;
 
/* Clear everything */
memset(desc_ptr, 0, ses_dev-page2_len - 8);
@@ -133,14 +132,14 @@ static int ses_set_page2_descriptor(stru
return ses_send_diag(sdev, 2, ses_dev-page2, ses_dev-page2_len);
 }
 
-static char *ses_get_page2_descriptor(struct enclosure_device *edev,
+static unsigned char *ses_get_page2_descriptor(struct enclosure_device *edev,
  struct enclosure_component *ecomp)
 {
int i, j, count = 0, descriptor = ecomp-number;
struct scsi_device *sdev = to_scsi_device(edev-cdev.dev);
struct ses_device *ses_dev = edev-scratch;
-   char *type_ptr = ses_dev-page1 + 12 + ses_dev-page1[11];
-   char *desc_ptr = ses_dev-page2 + 8;
+   unsigned char *type_ptr = ses_dev-page1 + 12 + ses_dev-page1[11];
+   unsigned char *desc_ptr = ses_dev-page2 + 8;
 
ses_recv_diag(sdev, 2, ses_dev-page2, ses_dev-page2_len);
 
@@ -160,17 +159,18 @@ static char *ses_get_page2_descriptor(st
 static void ses_get_fault(struct enclosure_device *edev,
  struct enclosure_component *ecomp)
 {
-   char *desc;
+   unsigned char *desc;
 
desc = ses_get_page2_descriptor(edev, ecomp);
-   ecomp-fault = (desc[3]  0x60)  4;
+   if (desc)
+   ecomp-fault = (desc[3]  0x60)  4;
 }
 
 static int ses_set_fault(struct enclosure_device *edev,
  struct enclosure_component *ecomp,
 enum enclosure_component_setting val)
 {
-   char desc[4] = {0 };
+   unsigned char desc[4] = {0 };
 
switch (val) {
case ENCLOSURE_SETTING_DISABLED:
@@ -190,26 +190,28 @@ static int ses_set_fault(struct enclosur
 static void ses_get_status(struct enclosure_device *edev,
   struct enclosure_component *ecomp)
 {
-   char *desc;
+   unsigned char *desc;
 
desc = ses_get_page2_descriptor(edev, ecomp);
-   ecomp-status = (desc[0]  0x0f);
+   if (desc)
+   ecomp-status = (desc[0]  0x0f);
 }
 
 static void ses_get_locate(struct enclosure_device *edev,
   struct

[PATCH] SCSI: fix data corruption caused by ses

[Yinghai Lu]

one system: initrd get courrupted:

RAMDISK: Compressed image found at block 0
RAMDISK: incomplete write (-28 != 2048) 134217728
crc error
VFS: Mounted root (ext2 filesystem).
Freeing unused kernel memory: 388k freed
init_special_inode: bogus i_mode (17)
Warning: unable to open an initial console.
init_special_inode: bogus i_mode (17)
init_special_inode: bogus i_mode (17)
Kernel panic - not syncing: No init found.  Try passing init= option to kernel.

bisected to
commit 9927c68864e9c39cc317b4f559309ba29e642168
Author: James Bottomley [EMAIL PROTECTED]
Date:   Sun Feb 3 15:48:56 2008 -0600

[SCSI] ses: add new Enclosure ULD

changes:
1. change char to unsigned char to avoid type change later.
2. preserve len for page1
3. need to move desc_ptr even the entry is not enclosure_component_device/raid.
   so keep desc_ptr on right position
4. also record page7 len, and double check if desc_ptr out of boundary before 
touch.

Signed-off-by: Yinghai Lu [EMAIL PROTECTED]

Index: linux-2.6/drivers/scsi/ses.c
===
--- linux-2.6.orig/drivers/scsi/ses.c
+++ linux-2.6/drivers/scsi/ses.c
@@ -33,9 +33,9 @@
 #include scsi/scsi_host.h
 
 struct ses_device {
-   char *page1;
-   char *page2;
-   char *page10;
+   unsigned char *page1;
+   unsigned char *page2;
+   unsigned char *page10;
short page1_len;
short page2_len;
short page10_len;
@@ -67,7 +67,7 @@ static int ses_probe(struct device *dev)
 static int ses_recv_diag(struct scsi_device *sdev, int page_code,
 void *buf, int bufflen)
 {
-   char cmd[] = {
+   unsigned char cmd[] = {
RECEIVE_DIAGNOSTIC,
1,  /* Set PCV bit */
page_code,
@@ -85,7 +85,7 @@ static int ses_send_diag(struct scsi_dev
 {
u32 result;
 
-   char cmd[] = {
+   unsigned char cmd[] = {
SEND_DIAGNOSTIC,
0x10,   /* Set PF bit */
0,
@@ -104,13 +104,13 @@ static int ses_send_diag(struct scsi_dev
 
 static int ses_set_page2_descriptor(struct enclosure_device *edev,
  struct enclosure_component *ecomp,
- char *desc)
+ unsigned char *desc)
 {
int i, j, count = 0, descriptor = ecomp-number;
struct scsi_device *sdev = to_scsi_device(edev-cdev.dev);
struct ses_device *ses_dev = edev-scratch;
-   char *type_ptr = ses_dev-page1 + 12 + ses_dev-page1[11];
-   char *desc_ptr = ses_dev-page2 + 8;
+   unsigned char *type_ptr = ses_dev-page1 + 12 + ses_dev-page1[11];
+   unsigned char *desc_ptr = ses_dev-page2 + 8;
 
/* Clear everything */
memset(desc_ptr, 0, ses_dev-page2_len - 8);
@@ -133,14 +133,14 @@ static int ses_set_page2_descriptor(stru
return ses_send_diag(sdev, 2, ses_dev-page2, ses_dev-page2_len);
 }
 
-static char *ses_get_page2_descriptor(struct enclosure_device *edev,
+static unsigned char *ses_get_page2_descriptor(struct enclosure_device *edev,
  struct enclosure_component *ecomp)
 {
int i, j, count = 0, descriptor = ecomp-number;
struct scsi_device *sdev = to_scsi_device(edev-cdev.dev);
struct ses_device *ses_dev = edev-scratch;
-   char *type_ptr = ses_dev-page1 + 12 + ses_dev-page1[11];
-   char *desc_ptr = ses_dev-page2 + 8;
+   unsigned char *type_ptr = ses_dev-page1 + 12 + ses_dev-page1[11];
+   unsigned char *desc_ptr = ses_dev-page2 + 8;
 
ses_recv_diag(sdev, 2, ses_dev-page2, ses_dev-page2_len);
 
@@ -160,17 +160,18 @@ static char *ses_get_page2_descriptor(st
 static void ses_get_fault(struct enclosure_device *edev,
  struct enclosure_component *ecomp)
 {
-   char *desc;
+   unsigned char *desc;
 
desc = ses_get_page2_descriptor(edev, ecomp);
-   ecomp-fault = (desc[3]  0x60)  4;
+   if (desc)
+   ecomp-fault = (desc[3]  0x60)  4;
 }
 
 static int ses_set_fault(struct enclosure_device *edev,
  struct enclosure_component *ecomp,
 enum enclosure_component_setting val)
 {
-   char desc[4] = {0 };
+   unsigned char desc[4] = {0 };
 
switch (val) {
case ENCLOSURE_SETTING_DISABLED:
@@ -190,26 +191,28 @@ static int ses_set_fault(struct enclosur
 static void ses_get_status(struct enclosure_device *edev,
   struct enclosure_component *ecomp)
 {
-   char *desc;
+   unsigned char *desc;
 
desc = ses_get_page2_descriptor(edev, ecomp);
-   ecomp-status = (desc[0]  0x0f);
+   if (desc)
+   ecomp-status = (desc[0]  0x0f);
 }
 
 static void ses_get_locate(struct enclosure_device *edev,
   struct enclosure_component *ecomp)
 {
-   char *desc;
+   unsigned

Re: [SCSI] ses: fix memory leaks

[Yinghai Lu]

On Monday 11 February 2008 09:02:06 am James Bottomley wrote:
 
 On Mon, 2008-02-11 at 10:23 -0600, James Bottomley wrote:
  On Sun, 2008-02-10 at 23:25 -0800, Yinghai Lu wrote:
   please check it...
  
  This one looks perfect, thanks!
 
 Well, nearly perfect.  I corrected this typo:
 
 +   if (!buf)
 +   goto simple_polulate;
 
 
 Which a compile check before you submitted the patch would have picked
 up ...

sorry for that.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: ses fix mem leaking when fail to add intf

[Yinghai Lu]

On Sunday 10 February 2008 08:28:38 pm James Bottomley wrote:
 
 On Sat, 2008-02-09 at 15:15 -0800, Yinghai Lu wrote:
  [PATCH] scsi: ses fix mem leaking when fail to add intf
  
  fix leaking with scomp leaking when failing.
  also remove one extra space.
 
 There are still a few extraneous code moves in this one.  This is about
 the correct minimal set, isn't it?


if buf allocation for page 7 get NULL...

if put 
+ if (!buf)
+   goto err_free;

still not right, because still undo 
edev = enclosure_register(cdev-dev, sdev-sdev_gendev.bus_id,
  components, ses_enclosure_callbacks);

all just add 
+ if (!buf)
+   goto simple_populate;

there?

YH
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[SCSI] ses: fix memory leaks

[Yinghai Lu]

please check it...

---

From: Yinghai Lu [EMAIL PROTECTED]

[SCSI] ses: fix memory leaks

fix leaking with scomp leaking when failing. Also free page10 on
driver removal  and remove one extra space.

Signed-off-by: Yinghai Lu [EMAIL PROTECTED]
Signed-off-by: James Bottomley [EMAIL PROTECTED]
---

 drivers/scsi/ses.c |   23 ++-
 1 file changed, 14 insertions(+), 9 deletions(-)

Index: linux-2.6/drivers/scsi/ses.c
===
--- linux-2.6.orig/drivers/scsi/ses.c
+++ linux-2.6/drivers/scsi/ses.c
@@ -416,11 +416,11 @@ static int ses_intf_add(struct class_dev
int i, j, types, len, components = 0;
int err = -ENOMEM;
struct enclosure_device *edev;
-   struct ses_component *scomp;
+   struct ses_component *scomp = NULL;
 
if (!scsi_device_enclosure(sdev)) {
/* not an enclosure, but might be in one */
-   edev =  enclosure_find(sdev-host-shost_gendev);
+   edev = enclosure_find(sdev-host-shost_gendev);
if (edev) {
ses_match_to_enclosure(edev, sdev);
class_device_put(edev-cdev);
@@ -456,9 +456,6 @@ static int ses_intf_add(struct class_dev
if (!buf)
goto err_free;
 
-   ses_dev-page1 = buf;
-   ses_dev-page1_len = len;
-
result = ses_recv_diag(sdev, 1, buf, len);
if (result)
goto recv_failed;
@@ -473,6 +470,9 @@ static int ses_intf_add(struct class_dev
type_ptr[0] == ENCLOSURE_COMPONENT_ARRAY_DEVICE)
components += type_ptr[1];
}
+   ses_dev-page1 = buf;
+   ses_dev-page1_len = len;
+   buf = NULL;
 
result = ses_recv_diag(sdev, 2, hdr_buf, INIT_ALLOC_SIZE);
if (result)
@@ -489,6 +489,7 @@ static int ses_intf_add(struct class_dev
goto recv_failed;
ses_dev-page2 = buf;
ses_dev-page2_len = len;
+   buf = NULL;
 
/* The additional information page --- allows us
 * to match up the devices */
@@ -506,11 +507,12 @@ static int ses_intf_add(struct class_dev
goto recv_failed;
ses_dev-page10 = buf;
ses_dev-page10_len = len;
+   buf = NULL;
 
  no_page10:
-   scomp = kmalloc(sizeof(struct ses_component) * components, GFP_KERNEL);
+   scomp = kzalloc(sizeof(struct ses_component) * components, GFP_KERNEL);
if (!scomp)
-   goto  err_free;
+   goto err_free;
 
edev = enclosure_register(cdev-dev, sdev-sdev_gendev.bus_id,
  components, ses_enclosure_callbacks);
@@ -521,10 +523,9 @@ static int ses_intf_add(struct class_dev
 
edev-scratch = ses_dev;
for (i = 0; i  components; i++)
-   edev-component[i].scratch = scomp++;
+   edev-component[i].scratch = scomp + i;
 
/* Page 7 for the descriptors is optional */
-   buf = NULL;
result = ses_recv_diag(sdev, 7, hdr_buf, INIT_ALLOC_SIZE);
if (result)
goto simple_populate;
@@ -532,6 +533,8 @@ static int ses_intf_add(struct class_dev
len = (hdr_buf[2]  8) + hdr_buf[3] + 4;
/* add 1 for trailing '\0' we'll use */
buf = kzalloc(len + 1, GFP_KERNEL);
+   if (!buf)
+   goto simple_polulate;
result = ses_recv_diag(sdev, 7, buf, len);
if (result) {
  simple_populate:
@@ -598,6 +601,7 @@ static int ses_intf_add(struct class_dev
err = -ENODEV;
  err_free:
kfree(buf);
+   kfree(scomp);
kfree(ses_dev-page10);
kfree(ses_dev-page2);
kfree(ses_dev-page1);
@@ -630,6 +634,7 @@ static void ses_intf_remove(struct class
ses_dev = edev-scratch;
edev-scratch = NULL;
 
+   kfree(ses_dev-page10);
kfree(ses_dev-page1);
kfree(ses_dev-page2);
kfree(ses_dev);
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] scsi: ses fix for len and mem leaking when fail to add intf

[Yinghai Lu]

[PATCH] scsi: ses fix for len and mem leaking when fail to add intf

change to u32 before left shifting char
also fix leaking with scomp leaking when failing.

Signed-off-by: Yinghai Lu [EMAIL PROTECTED]

Index: linux-2.6/drivers/scsi/ses.c
===
--- linux-2.6.orig/drivers/scsi/ses.c
+++ linux-2.6/drivers/scsi/ses.c
@@ -369,7 +369,7 @@ static void ses_match_to_enclosure(struc
 VPD_INQUIRY_SIZE, NULL, SES_TIMEOUT, SES_RETRIES))
goto free;
 
-   len = (buf[2]  8) + buf[3];
+   len = ((u32)buf[2]  8) + buf[3];
desc = buf + 4;
while (desc  buf + len) {
enum scsi_protocol proto = desc[0]  4;
@@ -420,7 +420,7 @@ static int ses_intf_add(struct class_dev
 
if (!scsi_device_enclosure(sdev)) {
/* not an enclosure, but might be in one */
-   edev =  enclosure_find(sdev-host-shost_gendev);
+   edev = enclosure_find(sdev-host-shost_gendev);
if (edev) {
ses_match_to_enclosure(edev, sdev);
class_device_put(edev-cdev);
@@ -451,18 +451,18 @@ static int ses_intf_add(struct class_dev
goto err_free;
}
 
-   len = (hdr_buf[2]  8) + hdr_buf[3] + 4;
+   len = ((u32)hdr_buf[2]  8) + hdr_buf[3] + 4;
buf = kzalloc(len, GFP_KERNEL);
if (!buf)
goto err_free;
 
-   ses_dev-page1 = buf;
-   ses_dev-page1_len = len;
-
result = ses_recv_diag(sdev, 1, buf, len);
if (result)
goto recv_failed;
 
+   ses_dev-page1 = buf;
+   ses_dev-page1_len = len;
+
types = buf[10];
len = buf[11];
 
@@ -474,11 +474,12 @@ static int ses_intf_add(struct class_dev
components += type_ptr[1];
}
 
+   buf = NULL;
result = ses_recv_diag(sdev, 2, hdr_buf, INIT_ALLOC_SIZE);
if (result)
goto recv_failed;
 
-   len = (hdr_buf[2]  8) + hdr_buf[3] + 4;
+   len = ((u32)hdr_buf[2]  8) + hdr_buf[3] + 4;
buf = kzalloc(len, GFP_KERNEL);
if (!buf)
goto err_free;
@@ -492,11 +493,12 @@ static int ses_intf_add(struct class_dev
 
/* The additional information page --- allows us
 * to match up the devices */
+   buf = NULL;
result = ses_recv_diag(sdev, 10, hdr_buf, INIT_ALLOC_SIZE);
if (result)
goto no_page10;
 
-   len = (hdr_buf[2]  8) + hdr_buf[3] + 4;
+   len = ((u32)hdr_buf[2]  8) + hdr_buf[3] + 4;
buf = kzalloc(len, GFP_KERNEL);
if (!buf)
goto err_free;
@@ -506,16 +508,18 @@ static int ses_intf_add(struct class_dev
goto recv_failed;
ses_dev-page10 = buf;
ses_dev-page10_len = len;
+   buf = NULL;
 
  no_page10:
scomp = kmalloc(sizeof(struct ses_component) * components, GFP_KERNEL);
if (!scomp)
-   goto  err_free;
+   goto err_free;
 
edev = enclosure_register(cdev-dev, sdev-sdev_gendev.bus_id,
  components, ses_enclosure_callbacks);
if (IS_ERR(edev)) {
err = PTR_ERR(edev);
+   kfree(scomp);
goto err_free;
}
 
@@ -524,24 +528,27 @@ static int ses_intf_add(struct class_dev
edev-component[i].scratch = scomp++;
 
/* Page 7 for the descriptors is optional */
-   buf = NULL;
result = ses_recv_diag(sdev, 7, hdr_buf, INIT_ALLOC_SIZE);
if (result)
goto simple_populate;
 
-   len = (hdr_buf[2]  8) + hdr_buf[3] + 4;
+   len = ((u32)hdr_buf[2]  8) + hdr_buf[3] + 4;
/* add 1 for trailing '\0' we'll use */
buf = kzalloc(len + 1, GFP_KERNEL);
-   result = ses_recv_diag(sdev, 7, buf, len);
-   if (result) {
+   if (buf)
+   result = ses_recv_diag(sdev, 7, buf, len);
+   else
+   result = 7;
+
  simple_populate:
+   if (result) {
kfree(buf);
buf = NULL;
desc_ptr = NULL;
addl_desc_ptr = NULL;
} else {
desc_ptr = buf + 8;
-   len = (desc_ptr[2]  8) + desc_ptr[3];
+   len = ((u32)desc_ptr[2]  8) + desc_ptr[3];
/* skip past overall descriptor */
desc_ptr += len + 4;
addl_desc_ptr = ses_dev-page10 + 8;
@@ -554,7 +561,7 @@ static int ses_intf_add(struct class_dev
struct enclosure_component *ecomp;
 
if (desc_ptr) {
-   len = (desc_ptr[2]  8) + desc_ptr[3];
+   len = ((u32)desc_ptr[2]  8) + desc_ptr[3];
desc_ptr += 4;
/* Add trailing zero - pushes into
 * reserved space

Re: [PATCH] scsi: ses fix for len and mem leaking when fail to add intf

[Yinghai Lu]

On Feb 9, 2008 7:00 AM, James Bottomley
[EMAIL PROTECTED] wrote:

 On Sat, 2008-02-09 at 04:13 -0800, Yinghai Lu wrote:
  [PATCH] scsi: ses fix for len and mem leaking when fail to add intf
 
  change to u32 before left shifting char

 This one is a bit unnecessary; C promotion rules guarantee that
 everything is promoted to int (or above) before doing arithmetic.  Since
 it's only ever done on 16 bits, signed or unsigned int is adequate for
 the conversion.

thank. just learned that.

[EMAIL PROTECTED]:~/xx/xx/notes cat ctest.c
#include stdio.h
int main(int argc, char *argv[])
{
unsigned char buf[20];
int len;

buf[2] = 0x02;
buf[3] = 0x03;

len = (buf[2]  8) + buf[3];

printf(len = %x\n, len);
return 0;
}
[EMAIL PROTECTED]:~/xx/xx/notes gcc -o ctest ctest.c
[EMAIL PROTECTED]:~/xx/xx/notes ./ctest
len = 203
[EMAIL PROTECTED]:~/xx/xx/notes



  also fix leaking with scomp leaking when failing.

 Yes, I see that, thanks!  There's also the kmalloc of scomp which should
 be kzalloc if you care to fix that up in the resend.

  - edev =  enclosure_find(sdev-host-shost_gendev);
  + edev = enclosure_find(sdev-host-shost_gendev);

 Space cleanups also need mention in the changelog.

  - ses_dev-page1 = buf;
  - ses_dev-page1_len = len;
  -
result = ses_recv_diag(sdev, 1, buf, len);
if (result)
goto recv_failed;
 
  + ses_dev-page1 = buf;
  + ses_dev-page1_len = len;
  +

 Neither of us gets this right.  By removing the kfree(buf) from the
 err_free path, you cause a leak here.  I cause a double free.  I think
 putting back the kfree(buf) and keeping this hunk is the fix.

the buf already become sdev-page1, sdev-pag10, sdev-page2.
so it will be freed via them


types = buf[10];
len = buf[11];
 
  @@ -474,11 +474,12 @@ static int ses_intf_add(struct class_dev
components += type_ptr[1];
}
 
  + buf = NULL;

 Yes, prevents double free (but only if buf is freed).

it became sdev-page1 already


result = ses_recv_diag(sdev, 2, hdr_buf, INIT_ALLOC_SIZE);
if (result)
goto recv_failed;
 
  @@ -492,11 +493,12 @@ static int ses_intf_add(struct class_dev
 
/* The additional information page --- allows us
 * to match up the devices */
  + buf = NULL;

 It's probably better to move these closer to the statements that make
 them necessary (in this case above the comment).

OK


if (IS_ERR(edev)) {
err = PTR_ERR(edev);
  + kfree(scomp);
goto err_free;
}

 kfree(scomp) should be in the err_free path just in case someone else
 adds something to this.

ok.


/* add 1 for trailing '\0' we'll use */
buf = kzalloc(len + 1, GFP_KERNEL);
  - result = ses_recv_diag(sdev, 7, buf, len);
  - if (result) {
  + if (buf)
  + result = ses_recv_diag(sdev, 7, buf, len);
  + else
  + result = 7;
  +

 What exactly is this supposed to be doing, and why 7?  If you're
 thinking of conditioning the page 7 receive on the success of the
 allocation, we really need the allocation failure report more than we
 need the driver to attach.

want to move out label out of if later.


  - addl_desc_ptr += addl_desc_ptr[1] + 2;
  + addl_desc_ptr += 2 + addl_desc_ptr[1];

 This is rather pointless, isn't it?

err_free:
  - kfree(buf);

 You can't remove this.  Also add kfree(scomp) here.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] scsi: ses fix mem leaking when fail to add intf

[Yinghai Lu]

[PATCH] scsi: ses fix mem leaking when fail to add intf

fix leaking with scomp leaking when failing.
also remove one extra space.

Signed-off-by: Yinghai Lu [EMAIL PROTECTED]

Index: linux-2.6/drivers/scsi/ses.c
===
--- linux-2.6.orig/drivers/scsi/ses.c
+++ linux-2.6/drivers/scsi/ses.c
@@ -416,11 +416,11 @@ static int ses_intf_add(struct class_dev
int i, j, types, len, components = 0;
int err = -ENOMEM;
struct enclosure_device *edev;
-   struct ses_component *scomp;
+   struct ses_component *scomp = NULL;
 
if (!scsi_device_enclosure(sdev)) {
/* not an enclosure, but might be in one */
-   edev =  enclosure_find(sdev-host-shost_gendev);
+   edev = enclosure_find(sdev-host-shost_gendev);
if (edev) {
ses_match_to_enclosure(edev, sdev);
class_device_put(edev-cdev);
@@ -456,9 +456,6 @@ static int ses_intf_add(struct class_dev
if (!buf)
goto err_free;
 
-   ses_dev-page1 = buf;
-   ses_dev-page1_len = len;
-
result = ses_recv_diag(sdev, 1, buf, len);
if (result)
goto recv_failed;
@@ -473,6 +470,9 @@ static int ses_intf_add(struct class_dev
type_ptr[0] == ENCLOSURE_COMPONENT_ARRAY_DEVICE)
components += type_ptr[1];
}
+   ses_dev-page1 = buf;
+   ses_dev-page1_len = len;
+   buf = NULL;
 
result = ses_recv_diag(sdev, 2, hdr_buf, INIT_ALLOC_SIZE);
if (result)
@@ -489,6 +489,7 @@ static int ses_intf_add(struct class_dev
goto recv_failed;
ses_dev-page2 = buf;
ses_dev-page2_len = len;
+   buf = NULL;
 
/* The additional information page --- allows us
 * to match up the devices */
@@ -506,11 +507,26 @@ static int ses_intf_add(struct class_dev
goto recv_failed;
ses_dev-page10 = buf;
ses_dev-page10_len = len;
+   buf = NULL;
 
  no_page10:
-   scomp = kmalloc(sizeof(struct ses_component) * components, GFP_KERNEL);
+
+   /* Page 7 for the descriptors is optional */
+   result = ses_recv_diag(sdev, 7, hdr_buf, INIT_ALLOC_SIZE);
+   if (result)
+   goto simple_populate;
+
+   len = (hdr_buf[2]  8) + hdr_buf[3] + 4;
+   /* add 1 for trailing '\0' we'll use */
+   buf = kzalloc(len + 1, GFP_KERNEL);
+   if (!buf)
+   goto err_free;
+   result = ses_recv_diag(sdev, 7, buf, len);
+
+ simple_populate:
+   scomp = kzalloc(sizeof(struct ses_component) * components, GFP_KERNEL);
if (!scomp)
-   goto  err_free;
+   goto err_free;
 
edev = enclosure_register(cdev-dev, sdev-sdev_gendev.bus_id,
  components, ses_enclosure_callbacks);
@@ -521,20 +537,10 @@ static int ses_intf_add(struct class_dev
 
edev-scratch = ses_dev;
for (i = 0; i  components; i++)
-   edev-component[i].scratch = scomp++;
+   edev-component[i].scratch = scomp + i;
 
-   /* Page 7 for the descriptors is optional */
-   buf = NULL;
-   result = ses_recv_diag(sdev, 7, hdr_buf, INIT_ALLOC_SIZE);
-   if (result)
-   goto simple_populate;
-
-   len = (hdr_buf[2]  8) + hdr_buf[3] + 4;
-   /* add 1 for trailing '\0' we'll use */
-   buf = kzalloc(len + 1, GFP_KERNEL);
-   result = ses_recv_diag(sdev, 7, buf, len);
+   /* result and buf from page 7 check */
if (result) {
- simple_populate:
kfree(buf);
buf = NULL;
desc_ptr = NULL;
@@ -598,6 +604,7 @@ static int ses_intf_add(struct class_dev
err = -ENODEV;
  err_free:
kfree(buf);
+   kfree(scomp);
kfree(ses_dev-page10);
kfree(ses_dev-page2);
kfree(ses_dev-page1);
@@ -630,6 +637,7 @@ static void ses_intf_remove(struct class
ses_dev = edev-scratch;
edev-scratch = NULL;
 
+   kfree(ses_dev-page10);
kfree(ses_dev-page1);
kfree(ses_dev-page2);
kfree(ses_dev);
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mptsas: scan the logical volume at first

[Yinghai Lu]

Moore, Eric wrote:

On Monday, August 27, 2007 11:58 AM,  Yinghai Lu wrote:

[PATCH] mptsas: scan the logical volume at first

user like to see the raid show as /dev/sda before left raw disks.
So scan the volume at first to make their life easier.

Signed-off-by: Yinghai Lu [EMAIL PROTECTED]



Although I agree with the patch, there are people on this list that will
reject it due to the fact distro's ship today having udev label and
device id mapping, so device ordering should be an non-issue.  However


even so, user still can customize it to use /dev/sdaX as root instead of use 
/dev/scsi/by-id/...or LABEL=/ as root.


there are systems that ship that don't have BIOS BBS support, allowing
you to select the boot device. Without it BBS support, you are forced to
boot to the lowest device id.  There are HP and Dell systems like that.
Are SUN systems like that?   If so, there is an additional patch which I
have yet posted that will sort the raid volumes in acsending order.
Currently they are in descending order (due to Firmware putting them in
that order), which if you did a install to what you think is /dev/sda,
its really the highest target id, and when you reboot, the BIOS will
boot to the lowest id, which is /dev/sdb, and it will not find the boot
partition.
Yes, I was wondering why kernel.org mainline will have /dev/sdb for first raid. but it seems RHEL 5 kernel have first raid before second raid...( it 
after all left over raw devices..), maybe they already aplied some patch?


can you send out patch?

Thanks

Yinghai Lu
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [2/3] 2.6.22-rc3: known regressions v2

[Yinghai Lu]

On 6/1/07, Salyzyn, Mark [EMAIL PROTECTED] wrote:

Agree, but overstated somewhat.

The card in question that the regression is reported against is not a
released card and as such could have a flawed environment, Hardware,
Firmware or other Incompatibility. The fix for the root cause will
likely not touch the driver or the kernel


So aacraid.reset_devices=1 works with Vivek's test system?

YH
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kexec and aacraid broken

[Yinghai Lu]

SUN coguar with 11731

YH

On 5/31/07, Salyzyn, Mark [EMAIL PROTECTED] wrote:

 No, still get adapter kernel panic

Which adapter are you using?

Sincerely -- Mark Salyzyn


-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] aacraid: fix shutdown handler to also disable interrupts.

[Yinghai Lu]

On 5/30/07, Salyzyn, Mark [EMAIL PROTECTED] wrote:

Moves quiesce, thread and interrupt shutdown into aacraid drivers'
.shutdown handler. This fix to the aac_shutdown handler will remove the
superfluous reset of the adapter during a (clean) kexec.

This fix may mitigate the active investigation 'kexec and aacraid
broken' but it is unlikely to affect the root cause (issue likely
present in both kexec and kdump). This patch reduces the chance the
problem will occur with a kexec. The fix for root cause is currently
expected to be the minimum value check to the aacraid.startup_timeout
driver variable after an adapter reset within aacraid_commit_reset.patch
submitted on 05/22/2007 and awaiting testing by Yinghai to confirm.

This attached patch is against current scsi-misc-2.6

ObligatoryDisclaimer: Please accept my condolences regarding Outlook's
handling of patch attachments.

Signed-off-by: Mark Salyzyn [EMAIL PROTECTED]

Sincerely -- Mark Salyzyn


the kernel with this patch -4  and even without

1. [SCSI] aacraid: superfluous adapter reset for IBM 8 series
ServeRAID controllers
2. [SCSI] aacraid: kexec fix (reset interrupt handler)
3. aacraid_commit_reset.patch

can load other kernel with or without patch 1,2,3

YH
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kexec and aacraid broken

[Yinghai Lu]

On 5/30/07, Salyzyn, Mark [EMAIL PROTECTED] wrote:

Vivek Goyal [mailto:[EMAIL PROTECTED] writes:
 So most likely if we start disabling the interrupts
 in .shutdown routine we might skip resetting adapter
 on every kexec without any side affects?

Not that simple. The .shutdown would need to perform more resource
cleanups of the .remove call to prevent side effects. I need to move
some of the .remove activity into the .shutdown handler to make sure the
adapter is quiesced.

I will hold off on submitting any of these changes until they are
evaluated and tested; I am waiting for feedback from Yinghai on the
other mitigations that I feel are closer to the root cause.


1. [SCSI] aacraid: superfluous adapter reset for IBM 8 series
ServeRAID controllers
2. [SCSI] aacraid: kexec fix (reset interrupt handler)
3. aacraid_commit_reset.patch
4. [PATCH] aacraid: fix shutdown handler to also disable interrupts

the kernel with this patch -4  and even without 1, 2, 3

can load other kernel with or without patch 1,2,3

YH
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kexec and aacraid broken

[Yinghai Lu]

On 5/30/07, Salyzyn, Mark [EMAIL PROTECTED] wrote:

I believe this issue is a result of the aacraid_commit_reset patch (as
posted for scsi-misc-2.6, enclosed to permit testing) not yet propagated
to the 2.6.22-rc3 tree.

This is the adapter taking longer than 3 minutes to start after a reset.
I seriously doubt either of these patches suggested below will have an
affect. And if they do, they are not root cause, one reduces the chances
that the card will be reset during initialization (thus applied would
likely mitigate this problem), the other prevents a panic when the
Adapter is reset (removed, would result in dogs and cats sleeping with
each other).

Please use kernel parameter aacraid.startup_timeout=540 (merely larger
than the default 180 seconds) when spawning the kexec or see if the
aacraid_commit_reset.patch resolves the issue to confirm my hunch.



aacraid_commit_reset.patch is in the mainline already.

YH
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kexec and aacraid broken

[Yinghai Lu]

On 5/30/07, Salyzyn, Mark [EMAIL PROTECTED] wrote:

Yinghai Lu [mailto:[EMAIL PROTECTED] writes:
 aacraid_commit_reset.patch is in the mainline already.

But aacraid_commit_reset.patch is not in 2.6.22-rc3 (to which you report
the issue). Does the aacraid_commit_reset.patch work to resolve this
issue all by itself in the kexec'd kernel? Or alternatively did you try
aacraid.startup_timeout=540 as one of the kernel parameters passed to
the kexec'd kernel?


No, still get adapter kernel panic



The '[PATCH] aacraid: fix shutdown handler to also disable interrupts'
patch (you refer to this as patch 4) is not to be in the picture because
it will hide the root cause. I believe I have you correct in stating
that this patch (4) resolves the problem... but I expect the problem to
remain with kdump.


Oh.
without patch(4), latest kernel still can use kexec to 2.6.21.3
will try to load 2.6.22-rc1 etc.

YH
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html