Re: [PATCH 1/1] iommu/arm-smmu-v3: remove unnecessary oom message

["Leizhen (ThunderTown)" <thunder.leizhen@huawei.com>]

On 2021/6/15 19:55, Will Deacon wrote:
> On Tue, Jun 15, 2021 at 12:51:38PM +0100, Robin Murphy wrote:
>> On 2021-06-15 12:34, Will Deacon wrote:
>>> On Tue, Jun 15, 2021 at 07:22:10PM +0800, Leizhen (ThunderTown) wrote:
>>>>
>>>>
>>>> On 2021/6/11 18:32, Will Deacon wrote:
>>>>> On Wed, Jun 09, 2021 at 08:54:38PM +0800, Zhen Lei wrote:
>>>>>> Fixes scripts/checkpatch.pl warning:
>>>>>> WARNING: Possible unnecessary 'out of memory' message
>>>>>>
>>>>>> Remove it can help us save a bit of memory.
>>>>>>
>>>>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>>>>>> ---
>>>>>>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 8 ++------
>>>>>>   1 file changed, 2 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>>>>> index 2ddc3cd5a7d1..fd7c55b44881 100644
>>>>>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>>>>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>>>>> @@ -2787,10 +2787,8 @@ static int arm_smmu_init_l1_strtab(struct arm_smmu_device *smmu)
>>>>>>   	void *strtab = smmu->strtab_cfg.strtab;
>>>>>>   	cfg->l1_desc = devm_kzalloc(smmu->dev, size, GFP_KERNEL);
>>>>>> -	if (!cfg->l1_desc) {
>>>>>> -		dev_err(smmu->dev, "failed to allocate l1 stream table desc\n");
>>>>>> +	if (!cfg->l1_desc)
>>>>>
>>>>> What error do you get if devm_kzalloc() fails? I'd like to make sure it's
>>>>> easy to track down _which_ allocation failed in that case -- does it give
>>>>> you a line number, for example?
>>>>
>>>> When devm_kzalloc() fails, the OOM information is printed. No line number information, but the
>>>> size(order) and call stack is printed. It doesn't matter which allocation failed, the failure
>>>> is caused by insufficient system memory rather than the fault of the SMMU driver. Therefore,
>>>> the current printing is not helpful for locating the problem of insufficient memory. After all,
>>>> when memory allocation fails, the SMMU driver cannot work at a lower specification.
>>>
>>> I don't entirely agree. Another reason for the failure is because the driver
>>> might be asking for a huge (or negative) allocation, in which case it might
>>> be instructive to have a look at the actual caller, particularly if the
>>> size is derived from hardware or firmware properties.
>>
>> Agreed - other than deliberately-contrived situations I don't think I've
>> ever hit a genuine OOM, but I definitely have debugged attempts to allocate
>> -1 of something. If the driver-specific message actually calls out the
>> critical information, e.g. "failed to allocate %d stream table entries", it
>> gives debugging a head start since the miscalculation is obvious, but a
>> static message that only identifies the callsite really only saves a quick
>> trip to scripts/faddr2line, and personally I've never found that
>> particularly valuable.
> 
> So it sounds like this particular patch is fine, but the one for smmuv2
> should leave the IRQ allocation message alone (by virtue of it printing
> something a bit more useful -- the number of irqs).

        num_irqs = 0;
        while ((res = platform_get_resource(pdev, IORESOURCE_IRQ, num_irqs))) {
                num_irqs++;
        }

As the above code, num_irqs is calculated based on the number of dtb or acpi
configuration items, it can't be too large. That is, there is almost zero chance
that devm_kcalloc() will fail because num_irqs is too large.


> 
> Will
> 
> .
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel