On 5/18/26 10:28, Boris Brezillon wrote:
> On Thu, 14 May 2026 11:34:52 -0700
> Chia-I Wu <[email protected]> wrote:
>
>> On Wed, May 13, 2026 at 10:24 AM Boris Brezillon
>> <[email protected]> wrote:
>>>
>>> Define a conditional drm_dev_access guard to automate the
>>> drm_dev_{enter,exit}() sequence.
>>>
>>> Signed-off-by: Boris Brezillon <[email protected]>
>>> ---
>>> include/drm/drm_drv.h | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
>>> index 42fc085f986d..79d1958f93e4 100644
>>> --- a/include/drm/drm_drv.h
>>> +++ b/include/drm/drm_drv.h
>>> @@ -490,6 +490,15 @@ void drm_dev_unplug(struct drm_device *dev);
>>> int drm_dev_wedged_event(struct drm_device *dev, unsigned long method,
>>> struct drm_wedge_task_info *info);
>>>
>>> +/*
>>> + * Only the conditional drm_dev_access guard is valid. The drm_dev one is
>>> + * here so we can extend it with a conditional variant.
>>> + */
>>> +DEFINE_LOCK_GUARD_1(drm_dev, struct drm_device,
>>> + { WARN_ON("Use cond guards"); _T->idx = -1; },
>>> + drm_dev_exit(_T->idx), int idx);
>> If this is ever mis-used, drm_dev_exit(-1) seems to cause OOB access.
>> Is BUG more appropriate than WARN_ON?
>
> I actually had
>
> if (_T->idx >= 0) drm_dev_exit(_T->idx),
>
> at some point, and I ditched it thinking the WARN_ON_ONCE()
> in srcu_read_unlock() would cover for that. I can add it back, of
> course.
>
> I'd be fine with a BUG_ON() too, but every time I tried to add one I've
> been encouraged to handle the unexpected case instead.
>
> Ideally, we would have a DEFINE_LOCK_GUARD_COND() variant that, instead
> of expanding a non-conditional one, would define the whole thing so
> that the non-conditional variant is never exposed.
Would it be possible to use BUILD_BUG() here?
Otherwise I think we should somehow come up with just the conditional guard
version since the guard one is clearly broken for SRCUs.
Regards,
Christian.
>
>>
>>> +DEFINE_LOCK_GUARD_1_COND(drm_dev, _access, drm_dev_enter(_T->lock,
>>> &_T->idx));
>>> +
>>> /**
>>> * drm_dev_is_unplugged - is a DRM device unplugged
>>> * @dev: DRM device
>>>
>>> --
>>> 2.54.0
>>>
>
