Re: [Intel-gfx] [PATCH v4 03/12] drm/i915: Introduce a ww transaction helper

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On 6/16/21 1:00 PM, Maarten Lankhorst wrote:
> Op 15-06-2021 om 15:14 schreef Thomas Hellström:
>> Introduce a for_i915_gem_ww(){} utility to help make the code
>> around a ww transaction more readable.
>>
>> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_gem_ww.h | 31 +++++++++++++++++++++++++++++-
>>   1 file changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_ww.h b/drivers/gpu/drm/i915/i915_gem_ww.h
>> index f2d8769e4118..f6b1a796667b 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_ww.h
>> +++ b/drivers/gpu/drm/i915/i915_gem_ww.h
>> @@ -11,11 +11,40 @@ struct i915_gem_ww_ctx {
>>   	struct ww_acquire_ctx ctx;
>>   	struct list_head obj_list;
>>   	struct drm_i915_gem_object *contended;
>> -	bool intr;
>> +	unsigned short intr;
>> +	unsigned short loop;
>>   };
>>   
>>   void i915_gem_ww_ctx_init(struct i915_gem_ww_ctx *ctx, bool intr);
>>   void i915_gem_ww_ctx_fini(struct i915_gem_ww_ctx *ctx);
>>   int __must_check i915_gem_ww_ctx_backoff(struct i915_gem_ww_ctx *ctx);
>>   void i915_gem_ww_unlock_single(struct drm_i915_gem_object *obj);
>> +
>> +/* Internal functions used by the inlines! Don't use. */
>> +static inline int __i915_gem_ww_fini(struct i915_gem_ww_ctx *ww, int err)
>> +{
>> +	ww->loop = 0;
>> +	if (err == -EDEADLK) {
>> +		err = i915_gem_ww_ctx_backoff(ww);
>> +		if (!err)
>> +			ww->loop = 1;
>> +	}
>> +
>> +	if (!ww->loop)
>> +		i915_gem_ww_ctx_fini(ww);
>> +
>> +	return err;
>> +}
>> +
>> +static inline void
>> +__i915_gem_ww_init(struct i915_gem_ww_ctx *ww, bool intr)
>> +{
>> +	i915_gem_ww_ctx_init(ww, intr);
>> +	ww->loop = 1;
>> +}
>> +
>> +#define for_i915_gem_ww(_ww, _err, _intr)			\
>> +	for (__i915_gem_ww_init(_ww, _intr); (_ww)->loop;	\
>> +	     _err = __i915_gem_ww_fini(_ww, _err))
>> +
>>   #endif
> With some more macro abuse, we should be able to kill off ww->loop,

Killing off ww->loop in itself is a good thing, I think. But the below 
is a  bit hard to follow, I think, :/

> for (err = ({i915_gem_ww_ctx_init(ww, intr), -EDEADLK}); err == -EDEADLK; err = (err == -EDEADLK && !(err = i915_gem_ww_ctx_backoff(ww))) ? -EDEADLK : err)

Could we simply keep the inlines and use err=-EDEADLK as the loop 
condition instead?

/Thomas


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx