Re: [PATCH 01/13] drm: execution context for GEM buffers v4

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux