Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/23/2015 12:45 PM, James Bottomley wrote:
On Tue, 2015-06-23 at 12:33 -0500, Timothy Pearson wrote:
On 06/23/2015 08:35 AM, James Bottomley wrote:
On Tue, 2015-06-23 at 18:06 +0530, Sreekanth Reddy wrote:
Hi,

Upstream contributor should not add copyright to this driver code.

I can't agree with that as a general rule: it depends on the
significance of the contribution.  The somewhat ill defined standard for
this is the contribution must be "an original work of authorship".

I can agree that a trivial bug fix like this is not an original work of
authorship, so in this case adding copyright isn't appropriate.

James

Regards,
Sreekanth

On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence<joe.lawrence@xxxxxxxxxxx>   wrote:


On 06/21/2015 02:46 PM, Timothy Pearson wrote:
On 06/16/2015 01:49 PM, Timothy Pearson wrote:
On 06/16/2015 12:42 PM, Joe Lawrence wrote:
On 06/16/2015 12:28 PM, Timothy Pearson wrote:
On 06/12/2015 05:05 PM, Timothy Pearson wrote:
The mpt2sas driver crashes if the BIOS does 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.

This patch aborts initialization and prints a warning if no memory I/O
resources are found.

Signed-off-by: Timothy Pearson<tpearson@xxxxxxxxxxxxxxxxxxxxxxxx>
Tested-by: Timothy Pearson<tpearson@xxxxxxxxxxxxxxxxxxxxxxxx>
---
drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
b/drivers/scsi/mpt2sas/mpt2sas_base.c
index 11248de..15c9504 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -6,6 +6,8 @@
* Copyright (C) 2007-2014 LSI Corporation
* Copyright (C) 20013-2014 Avago Technologies
* (mailto: MPT-FusionLinux.pdl@xxxxxxxxxxxxx)
+ * Copyright (C) 2015 Raptor Engineering
+ * (mailto: support@xxxxxxxxxxxxxxxxxxxxxxxxx)
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
@@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
MPT2SAS_ADAPTER
*ioc)
}
}

+ if (ioc->chip == NULL) {
+ printk(MPT2SAS_ERR_FMT "unable to map "
+ "adapter memory (resource not found)!\n", ioc->name);
+ r = -EINVAL;
+ goto out_fail;
+ }
+
_base_mask_interrupts(ioc);

r = _base_get_ioc_facts(ioc, CAN_SLEEP);

Just following up on this patch as I have not yet received any
response.

Thanks!

Hi Tim -- just curious, why was the similar check on ioc->chip just a
few lines above the one added by the patch insufficient?

That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
only sets ioc->chip once. I wonder if the fix might be simpler if the
existing ioc->chip check relocated entirely to where you put it (maybe
also pulling the entire error text onto one line for easier grepping).

Regards,

-- Joe

If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
the BIOS does not run resource allocation on the mpt2sas device) then
the check you are referring to is not executed, and the driver attempts
to perform operations on a null ioc->chip pointer.

I can relocate the check if desired.


On looking more closely at the code I think having the two separate
checks is useful for debugging.  The first error message is triggered if
the resource exists but Linux cannot map it for some reason, and the
second error message is triggered if the resource does not exist at all
(buggy BIOS).

Fair enough.  The two error messages are unique, so grepping will lead
to the correct check.

Only other nits would be the addition of a copyright (up to Avago I
guess) and an mpt3sas version (these drivers are 99% the same code).

Acked-by: Joe Lawrence<joe.lawrence@xxxxxxxxxxx>

-- Joe





Thank you for clarifying the guidelines on copyright lines.  I wasn't
sure if this was significant enough of a contribution to justify the
line or not.

Do I need to re-send or can I just state here that the patch can be
merged without the copyright line?

It's easier if you send a clean copy rather than risk me botching
something in the hand edit.

OK, will do.

I don't have any mpt3sas hardware to test with; should I just send in a
patch anyway without a Tested-by line?

Well, if you haven't actually tested it, adding a tested-by line
wouldn't be correct.

Of course not. My question was, should I send in a patch anyway (with appropriate commit message fields) even though it has not been tested?

Thanks!

--
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645
http://www.raptorengineeringinc.com
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux