KFD event handling questions

KFD event handling questions

[Felix Kuehling]

[Hari, can you comment on the last paragraph below regarding the events
limit? Would it hurt the runtime much to lose 8 signals out of 4096?]

Hi Oded,

I'm looking into upstreaming some event handling enhancements. I see
that you actually worked on increasing the number of events to 4096 and
working around some problems with debug events when you were at AMD.

As I'm looking at this, I'm confused by the code that allocates signal
pages. It allocates a single signal page that's big enough to
accommodate all signals. But allocate_signal_page looks like it was
meant to allocate smaller signal pages incrementally that are
accumulated in a kfd_process::signal_event_pages list. However, this
feature never gets used. It would also break user mode because the Thunk
expects to be able to map a single signal page for all its signals.

Therefore I'm inclined to simplify allocate_signal_page and related code
to only deal with a single allocation. Do you have any concerns or
objections about that?

Somewhat related to that, you also added a workaround that increases the
signal number to 4096+512 to accommodate 8 debug events while
maintaining page alignment. However, the signal page is allocated with
__get_free_pages, which allocates in powers of 2. So in fact it needs to
allocate 8192 signals, wasting about 28KB per process. It's not a lot of
waste. But an alternative would be to instead sacrifice 8 user signals.
The runtime has a fallback for when it runs out of signals, and the
difference between 4096 and 4088 seems insignificant. What do you think?

Regards,
  Felix

-- 
F e l i x   K u e h l i n g
PMTS Software Development Engineer | Vertical Workstation/Compute
1 Commerce Valley Dr. East, Markham, ON L3T 7X6 Canada
(O) +1(289)695-1597
   _     _   _   _____   _____
  / \   | \ / | |  _  \  \ _  |
 / A \  | \M/ | | |D) )  /|_| |
/_/ \_\ |_| |_| |_____/ |__/ \|   facebook.com/AMD | amd.com

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

RE: KFD event handling questions

[Thangirala, Hari]

Hi Felix,

Event IDs reserved for debug events was an artifact of a debugger design. We changed our tools strategy so those limitations do not exist anymore. There's no need to reserve special event-IDs for debugger. 4096 is good enough and the new debug trap handler will use events from that pool.

Thanks
Hari



-----Original Message-----
From: Kuehling, Felix 
Sent: Thursday, September 21, 2017 9:36 PM
To: amd-gfx@lists.freedesktop.org; Oded Gabbay <oded.gabbay@gmail.com>; Thangirala, Hari <Hari.Thangirala@amd.com>
Subject: KFD event handling questions

[Hari, can you comment on the last paragraph below regarding the events limit? Would it hurt the runtime much to lose 8 signals out of 4096?]

Hi Oded,

I'm looking into upstreaming some event handling enhancements. I see that you actually worked on increasing the number of events to 4096 and working around some problems with debug events when you were at AMD.

As I'm looking at this, I'm confused by the code that allocates signal pages. It allocates a single signal page that's big enough to accommodate all signals. But allocate_signal_page looks like it was meant to allocate smaller signal pages incrementally that are accumulated in a kfd_process::signal_event_pages list. However, this feature never gets used. It would also break user mode because the Thunk expects to be able to map a single signal page for all its signals.

Therefore I'm inclined to simplify allocate_signal_page and related code to only deal with a single allocation. Do you have any concerns or objections about that?

Somewhat related to that, you also added a workaround that increases the signal number to 4096+512 to accommodate 8 debug events while maintaining page alignment. However, the signal page is allocated with __get_free_pages, which allocates in powers of 2. So in fact it needs to allocate 8192 signals, wasting about 28KB per process. It's not a lot of waste. But an alternative would be to instead sacrifice 8 user signals.
The runtime has a fallback for when it runs out of signals, and the difference between 4096 and 4088 seems insignificant. What do you think?

Regards,
  Felix

-- 
F e l i x   K u e h l i n g
PMTS Software Development Engineer | Vertical Workstation/Compute
1 Commerce Valley Dr. East, Markham, ON L3T 7X6 Canada
(O) +1(289)695-1597
   _     _   _   _____   _____
  / \   | \ / | |  _  \  \ _  |
 / A \  | \M/ | | |D) )  /|_| |
/_/ \_\ |_| |_| |_____/ |__/ \|   facebook.com/AMD | amd.com

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

Re: KFD event handling questions

[Oded Gabbay]

On Sat, Sep 23, 2017 at 3:44 AM, Thangirala, Hari
<Hari.Thangirala@amd.com> wrote:
> Hi Felix,
>
> Event IDs reserved for debug events was an artifact of a debugger design. We changed our tools strategy so those limitations do not exist anymore. There's no need to reserve special event-IDs for debugger. 4096 is good enough and the new debug trap handler will use events from that pool.
>
> Thanks
> Hari
>
>
>
> -----Original Message-----
> From: Kuehling, Felix
> Sent: Thursday, September 21, 2017 9:36 PM
> To: amd-gfx@lists.freedesktop.org; Oded Gabbay <oded.gabbay@gmail.com>; Thangirala, Hari <Hari.Thangirala@amd.com>
> Subject: KFD event handling questions
>
> [Hari, can you comment on the last paragraph below regarding the events limit? Would it hurt the runtime much to lose 8 signals out of 4096?]
>
> Hi Oded,
>
> I'm looking into upstreaming some event handling enhancements. I see that you actually worked on increasing the number of events to 4096 and working around some problems with debug events when you were at AMD.
>
> As I'm looking at this, I'm confused by the code that allocates signal pages. It allocates a single signal page that's big enough to accommodate all signals. But allocate_signal_page looks like it was meant to allocate smaller signal pages incrementally that are accumulated in a kfd_process::signal_event_pages list. However, this feature never gets used. It would also break user mode because the Thunk expects to be able to map a single signal page for all its signals.
>
> Therefore I'm inclined to simplify allocate_signal_page and related code to only deal with a single allocation. Do you have any concerns or objections about that?

Feel free to simplify that code. At the time I believe we didn't know
how many events will be needed so we prepared a more generic mechanism
for allocation. I guess we are now smarter (after almost 4 years) so
simplification seems like the right thing to do if it matches the rest
of the stack.

>
> Somewhat related to that, you also added a workaround that increases the signal number to 4096+512 to accommodate 8 debug events while maintaining page alignment. However, the signal page is allocated with __get_free_pages, which allocates in powers of 2. So in fact it needs to allocate 8192 signals, wasting about 28KB per process. It's not a lot of waste. But an alternative would be to instead sacrifice 8 user signals.
> The runtime has a fallback for when it runs out of signals, and the difference between 4096 and 4088 seems insignificant. What do you think?

I believe Hari already answered that and I don't have anything more to add.

Thanks,
Oded

>
> Regards,
>   Felix
>
> --
> F e l i x   K u e h l i n g
> PMTS Software Development Engineer | Vertical Workstation/Compute
> 1 Commerce Valley Dr. East, Markham, ON L3T 7X6 Canada
> (O) +1(289)695-1597
>    _     _   _   _____   _____
>   / \   | \ / | |  _  \  \ _  |
>  / A \  | \M/ | | |D) )  /|_| |
> /_/ \_\ |_| |_| |_____/ |__/ \|   facebook.com/AMD | amd.com
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: KFD event handling questions

[Kuehling, Felix]

Hi Hari,

Is the "new debug trap handler" already working? It seems right now I'm breaking the "old" debugger backend test. However, given the current status of that debugger, I guess we can disable those tests for now?

Can you speak on behalf of the debugger team, or should I consult someone else on their end as well?

Regards,
  Felix

-----Original Message-----
From: Thangirala, Hari 
Sent: Friday, September 22, 2017 8:44 PM
To: Kuehling, Felix; amd-gfx@lists.freedesktop.org; Oded Gabbay
Subject: RE: KFD event handling questions

Hi Felix,

Event IDs reserved for debug events was an artifact of a debugger design. We changed our tools strategy so those limitations do not exist anymore. There's no need to reserve special event-IDs for debugger. 4096 is good enough and the new debug trap handler will use events from that pool.

Thanks
Hari



-----Original Message-----
From: Kuehling, Felix 
Sent: Thursday, September 21, 2017 9:36 PM
To: amd-gfx@lists.freedesktop.org; Oded Gabbay <oded.gabbay@gmail.com>; Thangirala, Hari <Hari.Thangirala@amd.com>
Subject: KFD event handling questions

[Hari, can you comment on the last paragraph below regarding the events limit? Would it hurt the runtime much to lose 8 signals out of 4096?]

Hi Oded,

I'm looking into upstreaming some event handling enhancements. I see that you actually worked on increasing the number of events to 4096 and working around some problems with debug events when you were at AMD.

As I'm looking at this, I'm confused by the code that allocates signal pages. It allocates a single signal page that's big enough to accommodate all signals. But allocate_signal_page looks like it was meant to allocate smaller signal pages incrementally that are accumulated in a kfd_process::signal_event_pages list. However, this feature never gets used. It would also break user mode because the Thunk expects to be able to map a single signal page for all its signals.

Therefore I'm inclined to simplify allocate_signal_page and related code to only deal with a single allocation. Do you have any concerns or objections about that?

Somewhat related to that, you also added a workaround that increases the signal number to 4096+512 to accommodate 8 debug events while maintaining page alignment. However, the signal page is allocated with __get_free_pages, which allocates in powers of 2. So in fact it needs to allocate 8192 signals, wasting about 28KB per process. It's not a lot of waste. But an alternative would be to instead sacrifice 8 user signals.
The runtime has a fallback for when it runs out of signals, and the difference between 4096 and 4088 seems insignificant. What do you think?

Regards,
  Felix

-- 
F e l i x   K u e h l i n g
PMTS Software Development Engineer | Vertical Workstation/Compute
1 Commerce Valley Dr. East, Markham, ON L3T 7X6 Canada
(O) +1(289)695-1597
   _     _   _   _____   _____
  / \   | \ / | |  _  \  \ _  |
 / A \  | \M/ | | |D) )  /|_| |
/_/ \_\ |_| |_| |_____/ |__/ \|   facebook.com/AMD | amd.com

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

Re: KFD event handling questions

[Jay Cornwall]

On Mon, Oct 2, 2017, at 08:22, Kuehling, Felix wrote:
> Is the "new debug trap handler" already working? It seems right now I'm
> breaking the "old" debugger backend test. However, given the current
> status of that debugger, I guess we can disable those tests for now?
> 
> Can you speak on behalf of the debugger team, or should I consult someone
> else on their end as well?

The existing debug API was designed to interact with live wavefronts on
the device. This created problems for the scheduler, which needs to be
able to remove wavefronts at any time without notice. The unprivileged
debugger could interfere with privileged operations (e.g. debugging in
the presence of oversubscribed processes, world switch in SR-IOV). It
wasn't developed beyond internal test cases and the ioctls should not
have been upstreamed.

The debugger was redesigned to work with offline wavefront state
collected through wavefront context save (already implemented in the
scheduler and controlled through hsaKmtUpdateQueue). This respects the
privilege model and is robust in all scheduling scenarios.

There are stil some yet-to-be-determined interfaces to control
per-process debugging features. This could extend/repurpose an existing
ioctl or require a new one.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: KFD event handling questions

[Thangirala, Hari]

Yes, should be okay to disable debugger tests as we don't support old debugger post-1.5 release. I've sent an email to Tools team and cc'd so we give them a heads-up. As Jay said, we're actively working on the new debugger design and don't have a need for hard-coding special event IDs anymore.  

Thanks
Hari

-----Original Message-----
From: Jay Cornwall [mailto:jay@jcornwall.com] 
Sent: Monday, October 2, 2017 10:05 AM
To: Kuehling, Felix <Felix.Kuehling@amd.com>; Thangirala, Hari <Hari.Thangirala@amd.com>; amd-gfx@lists.freedesktop.org; Oded Gabbay <oded.gabbay@gmail.com>
Subject: Re: KFD event handling questions

On Mon, Oct 2, 2017, at 08:22, Kuehling, Felix wrote:
> Is the "new debug trap handler" already working? It seems right now 
> I'm breaking the "old" debugger backend test. However, given the 
> current status of that debugger, I guess we can disable those tests for now?
> 
> Can you speak on behalf of the debugger team, or should I consult 
> someone else on their end as well?

The existing debug API was designed to interact with live wavefronts on the device. This created problems for the scheduler, which needs to be able to remove wavefronts at any time without notice. The unprivileged debugger could interfere with privileged operations (e.g. debugging in the presence of oversubscribed processes, world switch in SR-IOV). It wasn't developed beyond internal test cases and the ioctls should not have been upstreamed.

The debugger was redesigned to work with offline wavefront state collected through wavefront context save (already implemented in the scheduler and controlled through hsaKmtUpdateQueue). This respects the privilege model and is robust in all scheduling scenarios.

There are stil some yet-to-be-determined interfaces to control per-process debugging features. This could extend/repurpose an existing ioctl or require a new one.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx