[Intel-gfx] [RFC] GPU reset notification interface

[Ian Romanick]

On 07/18/2012 09:55 AM, Daniel Vetter wrote:
> On Wed, Jul 18, 2012 at 09:23:46AM -0700, Ian Romanick wrote:
>> On 07/18/2012 02:20 AM, Daniel Vetter wrote:
>>> - The "all contexts in a share group need to receive a reset notification"
>>>    wording is irking me a bit because we currently only track all the
>>>    actively used things. So if another context in that share group isn't
>>>    affected (i.e. doesn't even use any of the potentially corrupted bos),
>>>    is the idea that the kernel grows the required tracking, or should
>>>    userspace always ask the reset state for all contexts and do the math
>>>    itself?
>>
>> There are a couple reasons that all contexts in a share group need
>> to get the reset notification.  Consider an application with two
>> contexts, A and B.  Context A is a worker context that does a bunch
>> of render-to-texture operations, and context B will eventually
>> consume those textures.  If context A receives a reset, context B,
>> even if it hasn't done any rendering in five minutes, has lost data.
>>
>> The kernel should never have any knowledge about GL share groups.
>> This is where my simplifying assumption (that all contexts in a
>> share group share an address space and an fd) comes in handy.  If
>> context A queries the reset status from the kernel first, it can
>> reach over and poke the reset status of context B (in the gl_context
>> structure).  Likewise, if context B queries the kernel first, it can
>> see that another kernel context in its GL context share group got
>> reset.
>>
>>> - The above essentially links in with how we blame the guilt and figure
>>>    out who's affected. Especially when there's no hw context around, e.g.
>>>    on the blitter or bsd rings, or when an object is used by another
>>>    process and gets corrupted there. Does the spec make any guarantees for
>>>    such a case? Or should we just blame an unknown_reset for all the
>>>    contexts that belong to the fd that submitted the bad batch.
>>
>> That sounds right.  If we can't assess innocence or guilt, we should
>> say guilt is unknown.  There are some GL commands that get generate
>> blits, but I don't think there's anything that can get commands on
>> the BSD ring.  That's just for media, right?
>>
>>> As an idea for the above two issues, what about the kernel also keeps a
>>> per-fd reset statistics, which will also be returned with this ioctl?
>>> We'd then restrict the meaning of the ctx fields to only mean "and the
>>> context was actually active". Userspace could then wrap all the per-fd
>>> hang reports into reset_unknown for arb_robustness, but I think this would
>>> be a bit more flexible for other userspace.
>>
>> Ah, right.  So the DDX or libva could detect resets that affect
>> them. That's reasonable.
>>
>>> struct drm_context_reset_counts {
>>> 	__u32 ctx_id;
>>>
>>> 	/**
>>>           * Index of the most recent reset where this context was
>>> 	 * guilty.  Zero if none.
>>>           */
>>> 	__u32 ctx_guilty;
>>>
>>> 	/**
>>>           * Index of the most recent reset where this context was active,
>>> 	 * not guilty.  Zero if none.
>>>           */
>>> 	__u32 ctx_not_guilty;
>>>
>>> 	/**
>>>           * Index of the most recent reset where this context was active,
>>> 	 * but guilt was unknown. Zero if none.
>>>           */
>>> 	__u32 ctx_unknown_guilt;
>>>
>>> 	/**
>>>           * Index of the most recent reset where any batchbuffer submitted
>>> 	 * through this fd was guilty.  Zero if none.
>>>           */
>>> 	__u32 fd_guilty;
>>>
>>> 	/**
>>>           * Index of the most recent reset where any batchbuffer submitted
>>> 	 * through this fd was not guilty, but affected.  Zero if none.
>>>           */
>>> 	__u32 fd_not_guilty;
>>>
>>> 	/**
>>>           * Index of the most recent reset where any batchbuffer submitted
>>> 	 * through this fd was affected, but no guilt for the hang could
>>> 	 * be assigned.  Zero if none.
>>>           */
>>> 	__u32 fd_unknown_guilt;
>>
>> Since these three fields are per-fd, shouldn't they go in the
>> proposed drm_reset_counts structure instead?  If we do that, it
>> might be better to split into two queries.  One for the per-fd
>> information, and one for the detailed per-context information.  If
>> we expect the common case to be no-reset, user space could
>
> Oh, I've missed a bit that your ioctl would read out the state for all
> contexts on a given fd. My idea was that this is all the data that the
> ioctl retuns (and it would only fill out the ctx fields if you spec a
> nonzero context). Userspace would then only need to check whether the
> context has a new reset number somewhere (= context suffered from a gpu
> reset while it was actively used) and then also check whether the fd
> suffered from a reset (= something in that gl share group is potentially
> corrupted), mapping it to either victim_reset or unknown_reset (depending
> upon what exactly the spec wants).

Consider the case where a process has four contexts, A, B, X, and Y.  A 
and B are in a share group, and X and Y are in a share group not with A 
and B.  B sees a reset.  When the application queries the reset status 
on X or Y, it should not see a reset.  Knowing that there was a reset 
for some other context on the same fd isn't enough information.

> Less complexity in userspace, and imo not too much fuss for the kernel to
> do the book-keeping: Instead of only blaming contexts, we also blame
> file_privs. That would be a generally useful feature to e.g. cut off
> certain fds selectively from the gpu because they hang it too fast - i.e.
> to avoid the dreaded global "hanging to fast" when developing drivers ;-)
>
>>> };
>>>
>>> The above could also be important to know when a hw context is toast and
>>> should better not be used any more.
>>>
>>> - I like Chris' suggestion of using our seqno breadcrumbs for this. If you
>>>    also add a small extension to execbuf to return the seqno of the new
>>>    batch, you should also have all the tools in place to implement your
>>>    extension to notify userspace up to which glFence things have completed.
>>>    Note though that returning the seqno in execbuffer is only correct once
>>>    we've eliminated the flushing_list.
>>
>> It's possible, but I need to finish working out that idea (see
>> below). I think the context only needs one seqno, not one per
>> guiltiness level.  "This is the last seqno that was retired before
>> this context lost some data."
>
> The idea was to use the existing seqno as the reset counter in your ioctl.
> It might wrap around, but you need to emit quite a few commands to get to
> that point ;-) So we could use that right away.
>
>> That may still leave the context in a weird state.  Think about this
>> timeline:
>>
>>      Application      GPU
>>      draw A
>>                       submit A
>>      draw B
>>                       submit B
>>                       drawing A completes
>>                       reset (B is lost)
>>      draw C
>> 	             submit C
>>                       drawing C completes
>>      query reset
>>
>> If / when we implement this feature, the kernel may need to drop any
>> work submitted between a reset and an ack of the reset.  Dunno.
>
> Well, we could simply add a robust_context flag, set at create time. If
> such a context suffers from a gpu reset (while it's active) the kernel
> could refuse to queue up more batches with -EIO. Userspace could then

If we go that way, I think it would be better to just drop the requests. 
  OpenGL doesn't let me start generating errors after a reset, so I'd 
have to selectively ignore these errors from the kernel.  It sounds like 
it would add a bunch of annoying complexity to user space.

Of course, we're talking about step 38 here, and we're still on step 2.

> inquire the kernel with your ioctl about what's going on. Similarly we
> could add such a flag to the fd, to catch everyone else not using hw
> contexts. There we need a way to clear that flag though, so that you can
> keep on using all the bo textures that survived (whereas hw contexts are
> pretty much worthless on their own after reset, so we could demand that
> they be destroyed).
>
>>> - Last but not least, how often is userspace expected to call this ioctl?
>>>    Around once per patchbuffer submission or way more/less?
>>
>> I don't know.  This is a fairly new extension, and there are few
>> users.  As far as I'm aware, only Chrome and Firefox use it.  I can
>> find out some details from them.  My guess is somewhere between once
>> per frame and once per draw call.
>
> Cheers, Daniel