On Sat, Sep 23, 2017 at 3:44 AM, Thangirala, Hari <Hari.Thangirala at 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 at lists.freedesktop.org; Oded Gabbay <oded.gabbay at gmail.com>; Thangirala, Hari <Hari.Thangirala at 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 >
