On Mon, 19 Jun 2023 12:44:06 +0200
Christian König <christian.koenig@xxxxxxx> wrote:
> 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.
Sorry, didn't pay attention to this particular concern. Indeed, if you
want to return inside the expression, that's a problem.
>
> 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.
Yeah, I'm not sure it's safe to use non-local labels for that, because,
as soon as you have more than one drm_exec_until_all_locked() call in a
given function it won't work, which is why I placed things in a block
with local labels, which in turn means you can't return directly,
unfortunately.
