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

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

 



Hi,

Upstream contributor should not add copyright to this driver code.

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



-- 

Regards,
Sreekanth
--
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