On Wed, Jun 16, 2021 at 09:47:18AM +0800, Leizhen (ThunderTown) wrote: > > > 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.leiz...@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.
Right, because firmware is never wrong about anything :) Will _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu