On 2/4/25 7:22 AM, Maarten Lankhorst wrote:
> Ignore my previous series please, it should have been sent to intel-xe, was sent to intel-gfx.
>
> Instead of all this repetition of
>
> {
> unsigned fw_ref;
>
> fw_ref = xe_force_wake_get(fw, domain);
> if (!xe_force_wake_ref_has_domain(..))
> return -ETIMEDOUT;
>
> ...
>
> out:
> xe_force_wake_put(fw_ref);
> return ret;
> }
>
> I thought I would look at how to replace it with the guard helpers.
> It is easy, but it required some minor fixes to make DEFINE_LOCK_GUARD_1
> work with extra init arguments.
>
> So I changed the function signature slightly to make the first optional argument
> a struct member (current behavior), and any additional argument goes to the init
> call.
>
> This replaces the previous code with
> {
> scoped_cond_guard(xe_force_wake_get, return -ETIMEDOUT, fw, domain) {
> ....
>
> return ret;
> }
> }
>
> I' ve thought also of playing with this:
> {
> CLASS(xe_force_wake_get, fw_ref)(fw, domain);
> if (!fw_ref.lock))
> return -ETIMEDOUT;
>
> ...
> return ret;
> }
>
> I'm just fearing that the scoped_cond_guard makes it imposssible to get this
> wrong, while in the second example it's not clear that it can fail, and that
> you have to check.
>
> Let me know what you think!
> Feedback welcome for the header change as well, should probably go into the locking tree..
>
When I tried to make a similar improvement, Linus said to please stop trying
with the conditional guard stuff [1]. So my advice is don't do it.
Just replace the:
> ...
>
> out:
with a helper function if you want to get rid of the gotos.
That is what we are doing in the iio subsystem [2][3].
[1]: https://lore.kernel.org/all/CAHk-=whn07tnDosPfn+UcAtWHBcLg=KqA16SHVv0GV4t8P1fHw@xxxxxxxxxxxxxx/
[2]: https://lore.kernel.org/all/20250105172613.1204781-1-jic23@xxxxxxxxxx/
[3]: https://lore.kernel.org/all/20250202210045.1a9e85d7@jic23-huawei/
