[PATCH v2] arm_mpam: make the mon_sel guard conditional only
Ben Horgan
ben.horgan at arm.com
Mon Oct 12 08:00:10 PDT 2026
+CC: Catalin, Will
Hi Catalin,
This is the clang build fix I mentioned. Just adding a note here as I missed to CC you before.
Thanks,
Ben
On 09/10/2026 17:59, Ben Horgan wrote:
> Hi Sasha,
>
> On 10/9/26 17:32, Sasha Levin wrote:
>> Building arm64 allmodconfig with clang fails:
>>
>> drivers/resctrl/mpam_internal.h:207:7: error: ignoring return value
>> of function declared with 'warn_unused_result' attribute
>> [-Werror,-Wunused-result]
>> 207 | mpam_mon_sel_lock(_T), mpam_mon_sel_unlock(_T));
>>
>> mpam_mon_sel_lock() is __must_check and can fail: for an MSC accessed
>> through the MPAM-Fb firmware interface it returns false when called
>> from a context that cannot sleep. The mon_sel guard was defined with
>> DEFINE_GUARD(), which calls it unconditionally and discards the
>> result, so a guard(mon_sel) user would carry on without the lock and
>> release it on scope exit. The unconditional guard only exists as the
>> base for the conditional mon_sel_lock variant, which is the one
>> actually used.
>>
>> Define mon_sel_lock directly as a conditional guard class instead, as
>> posix-timers does for lock_timer: the constructor returns NULL when
>> the lock cannot be taken, and the destructor only releases a lock
>> that was taken. ACQUIRE(mon_sel_lock, ...) works as before, including
>> ACQUIRE_ERR() returning -EBUSY on failure, and there is no
>> unconditional variant left to misuse.
>>
>> Reported-by: kernelci.org bot <bot at kernelci.org>
>> Closes: https://d.kernelci.org/i/maestro:66f869ecbbb5b8592562ffd89f049c5ddf682547
>> Closes: https://d.kernelci.org/i/maestro:647269216758644837afcaac8e67d8dbe713a0e2
>> Fixes: 0db1715e4c9c ("arm_mpam: propagate MSC access errors for hw_probe functions")
>> Assisted-by: LLM
>> Signed-off-by: Sasha Levin <sashal at kernel.org>
>
> Looks good to me.
>
> Reviewed-by: Ben Horgan <ben.horgan at arm.com>
>
> Thanks,
>
> Ben
>
>> ---
>> Changes in v2:
>> - Use an if statement in the destructor, as suggested by Ben Horgan.
>>
>> drivers/resctrl/mpam_internal.h | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
>> index ee7c3953a9b6c..1f86cc158100b 100644
>> --- a/drivers/resctrl/mpam_internal.h
>> +++ b/drivers/resctrl/mpam_internal.h
>> @@ -203,9 +203,10 @@ static inline int mpam_mon_sel_lock_init(struct device *dev,
>> return devm_mutex_init(dev, &msc->mon_sel_mutex);
>> }
>>
>> -DEFINE_GUARD(mon_sel, struct mpam_msc *,
>> - mpam_mon_sel_lock(_T), mpam_mon_sel_unlock(_T));
>> -DEFINE_GUARD_COND(mon_sel, _lock, mpam_mon_sel_lock(_T), _RET);
>> +DEFINE_CLASS(mon_sel_lock, struct mpam_msc *,
>> + if (_T) mpam_mon_sel_unlock(_T),
>> + mpam_mon_sel_lock(msc) ? msc : NULL, struct mpam_msc *msc);
>> +DEFINE_CLASS_IS_COND_GUARD(mon_sel_lock);
>>
>> /* Bits for mpam features bitmaps */
>> enum mpam_device_features {
>
More information about the linux-arm-kernel
mailing list