On 2/3/20 4:20 AM, Tomas Henzl wrote:
> On 2/3/20 10:16 AM, Sumit Saxena wrote:
>> On Sat, Feb 1, 2020 at 10:57 PM Lee Duncan <lduncan@xxxxxxxx> wrote:
>>>
>>> On 1/31/20 5:23 AM, Tomas Henzl wrote:
>>>> Add a flag to dma mem allocation to silence a warning.
>>>>
>>>> Signed-off-by: Tomas Henzl <thenzl@xxxxxxxxxx>
>>>> ---
>>>> drivers/scsi/megaraid/megaraid_sas_fusion.c | 5 +++--
>>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c
>>>> index 0f5399b3e..1fa2d1449 100644
>>>> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
>>>> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
>>>> @@ -606,7 +606,8 @@ megasas_alloc_request_fusion(struct megasas_instance *instance)
>>>>
>>>> fusion->io_request_frames =
>>>> dma_pool_alloc(fusion->io_request_frames_pool,
>>>> - GFP_KERNEL, &fusion->io_request_frames_phys);
>>>> + GFP_KERNEL | __GFP_NOWARN,
>>>> + &fusion->io_request_frames_phys);
>>>> if (!fusion->io_request_frames) {
>>>> if (instance->max_fw_cmds >= (MEGASAS_REDUCE_QD_COUNT * 2)) {
>>>> instance->max_fw_cmds -= MEGASAS_REDUCE_QD_COUNT;
>>>> @@ -644,7 +645,7 @@ megasas_alloc_request_fusion(struct megasas_instance *instance)
>>>> open-isns-updates.diff.bz2
>>>> fusion->io_request_frames =
>>>> dma_pool_alloc(fusion->io_request_frames_pool,
>>>> - GFP_KERNEL,
>>>> + GFP_KERNEL | __GFP_NOWARN,
>>>> &fusion->io_request_frames_phys);
>>>>
>>>> if (!fusion->io_request_frames) {
>>>>
>>>
>>> I'm fairly sure this is a good fix, but I'd appreciate more information
>>> in the comment, such as what warning was silenced, and why it's okay to
>>> silence it rather than "fix" it. I know from experience that, when
>>> choosing which commits to backport, more information is better than less.
>> This code allocates DMA memory for driver's IO frames which may exceed
>> MAX_ORDER pages for few
>> megaraid_sas controllers(controllers with High Queue Depth). So there
>> is logic to keep on reducing controller
>> Queue Depth until DMA memory required for IO frames fits within
>> MAX_ORDER. So or impacted megaraid_sas controllers,
>> there would be multiple DMA allocation failure until driver settles
>> down to Controller Queue Depth which has memory requirement
>> within MAX_ORDER. These failed DMA allocation requests causes stack
>> traces in system logs which is not harmful and this patch
>> would silence those warnings/stack traces.
>>
>> With CMA (Contiguous Memory Allocator) enabled, it's possible to
>> allocate DMA memory exceeding MAX_ORDER.
>> And that is the reason of keeping this retry logic with less
>> controller Queue Depth instead of calculating controller Queue depth
>> at first hand which has memory requirement less than MAX_ORDER.
>
> Thank you Sumit for writing it down.
> An over-sized allocation failure is sanitized in a proper way. The
> warning may hide other allocation warnings in other parts of kernel as
> it is printed only once.
>
> I could have written more vecasue I've underestimated it and I'm sorry
> for that.
>
> Tomas
No problem! Thank you for adding this. Can you resubmit with this info?
If you're not familiar with submitting a 2nd version (V2), please look
at the mailing list archives (or email me directly for advice).
I'd be glad to add my reviewed tag once the patch is updated.
>
>
>> Thanks,
>> Sumit
>>>
>>> --
>>> Lee Duncan
>>
>
