Re: [PATCH 11/33] arm_mpam: Add the class and component structures for firmware described ris

Ben Horgan
2025-11-13T16:39:55Z

Hi Fenghua,

On 11/13/25 03:23, Fenghua Yu wrote:
> Hi, Ben and James,
>
> On 11/7/25 04:34, Ben Horgan wrote:
>> From: James Morse <james.morse@arm.com>
>
> [SNIP]
>
>> +static int mpam_ris_create_locked(struct mpam_msc *msc, u8 ris_idx,
>> +                  enum mpam_class_types type, u8 class_id,
>> +                  int component_id)
>> +{
>> +    int err;
>> +    struct mpam_vmsc *vmsc;
>> +    struct mpam_msc_ris *ris;
>> +    struct mpam_class *class;
>> +    struct mpam_component *comp;
>> +    struct platform_device *pdev = msc->pdev;
>> +
>> +    lockdep_assert_held(&mpam_list_lock);
>> +
>> +    if (ris_idx > MPAM_MSC_MAX_NUM_RIS)
>> +        return -EINVAL;
>> +
>> +    if (test_and_set_bit(ris_idx, &msc->ris_idxs))
>> +        return -EBUSY;
>> +
>> +    ris = devm_kzalloc(&msc->pdev->dev, sizeof(*ris), GFP_KERNEL);
>> +    if (!ris)
>> +        return -ENOMEM;
>
> The ris_idx bit in msc->ris_idxs is not cleared on error paths in this
> function. The bit cannot be set again.
>
> Not sure if this is a real problem in any case. Clearing the bit on
> error paths may be clean code.

I think this would just add noise. There is no anticipation of any
functionality after failing to create RIS and I expect there are more
places in the code where this assumption is relied on.

>
> [SNIP]
>
> Thanks.
>
> -Fenghua

Thanks,

Ben