Am 19.06.23 um 12:12 schrieb Boris Brezillon:
[SNIP]
Note that the drm_exec_until_all_locked() helper I introduced is taking
an expression, so in theory, you don't have to define a separate
function.

        drm_exec_until_all_locked(&exec, {
                /* inlined-code */
                int ret;

                ret = blabla()
                if (ret)
                        goto error;

                ...

error:
                /* return value. */
                ret;
        });

This being said, as soon as you have several failure paths,
it makes things a lot easier/controllable if you make it a function,
and I honestly don't think the readability would suffer from having a
function defined just above the user. My main concern with the original
approach was the risk of calling continue/break_if_contended() in the
wrong place, and also the fact you can't really externalize things to
a function if you're looking for a cleaner split. At least with
drm_exec_until_all_locked() you can do both.

Yeah, but that means that you can't use return inside your code block and instead has to define an error label for handling "normal" contention which is what I'm trying to avoid here.

How about:

#define drm_exec_until_all_locked(exec)    \
        __drm_exec_retry: if (drm_exec_cleanup(exec))


#define drm_exec_retry_on_contention(exec)              \
        if (unlikely(drm_exec_is_contended(exec)))      \
                goto __drm_exec_retry


And then use it like:

drm_exec_until_all_locked(exec)
{
    ret = drm_exec_prepare_obj(exec, obj);
    drm_exec_retry_on_contention(exec);
}

The only problem I can see with this is that the __drm_exec_retry label would be function local.

Regards,
Christian.


Regards,

Boris

Reply via email to