Re: [PATCH 14/38] drm/i915: Introduce the i915_user_extension_method

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

 




On 04/03/2019 09:04, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-03-04 08:54:10)

On 01/03/2019 18:57, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-03-01 15:39:13)

On 01/03/2019 14:03, Chris Wilson wrote:
+int i915_user_extensions(struct i915_user_extension __user *ext,
+                      const i915_user_extension_fn *tbl,
+                      unsigned long count,
+                      void *data)
+{
+     unsigned int stackdepth = 512;
+
+     while (ext) {
+             int err;
+             u64 x;
+
+             if (!stackdepth--) /* recursion vs useful flexibility */
+                     return -EINVAL;

I don't get this. What stack? Did you mean "static unsigned int
stackdepth" in case someone puts i915_user_extension into the extension
table? Or just a limit on number of chained extensions? But you are not
processing the recursively here.

It's iterative recursion :)

I still think of this loop in terms of its simple tail recursion.

And if we need to individual levels for unwind, that is a manual stack.
Okay..

One thought I had - if unwind is too unwieldy, could we mandate each
user extension user has an additional output field in the embedding
struct which would hold the last processed extension id?

id? I guess we could do depth, though I guess pointer is more practical.
I don't think you mean ext.name as that we expect to repeat a few times.

Pointer yes, I started with a pointer but then for some reason wrote id, where I meant name.

This way the state of the object would be less undefined after failure.
Userspace would be able to tell how far in the extension chain i915
managed to get to.

But I'm not convinced how practical that is. The answer is pretty much
always -EINVAL (anything else is much less ambiguous) and even then you
have no idea which field tripped EINVAL or why?

Which field tripped -EINVAL is why is always true.

I am not 100% convinced myself, just worry about usability. But I don't have an usecase at the moment which would be practically helped with knowing which extension failed. In all cases I can think of it's a programming error ie. should be caught during development and not happen in production.

Regards,

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




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux