Re: [RFC 00/10] xhci: re-work command queue management

From: Mathias Nyman
Date: Wed Jan 15 2014 - 10:35:57 EST


On 01/14/2014 02:37 PM, David Laight wrote:
From: Mathias Nyman
...
The fact that you are having to allocate memory ion an ISR ought also to be
ringing alarm bells.

It did.
Other options are as you said to use a 'software command ring'. Either
just pre-allocate a full command ring (64 trbs * sizeof(struct
xhci_command)), roughly 2300 bytes when driver loads,
or then a smaller pool of commands just used for ISR submitted commands.
(We then need to either either expand pool, or start allocating in isr
if pool is empty)

If you pre-allocate mapping 1-1 with the command ring entries
you don't need many of the fields of xhci_command at all.
So the allocated size will be much smaller.

True, then the struct list_head could be removed.
Even the trb pointer could be removed if the command ring stays in one segment(just use the trb - base as an index )

...

Looking at the number of fields in xci_command, why do you need the
caller to pass a structure at all?
Just make the fields parameters to the 'send a command' function
along with a parameter that says whether it can sleep (in kmalloc()
or if the ring is full depending on the implementation).

Command completion events only tell which trb completed and its status. The structure is needed to map the trb to the right completion so that the caller can continue running, check status and move on.

The xhci_command fields forISR allocated commands are really not used that much. Only status is used if the command timeouts. But we want these command structures on the command list to make sure eveything else works (timeout works, commands completion events are in the same order as commands on our command list etc)

So in a way we're allocating memory in ISR which is not really even used.

I'll try to create something to that avoid that

-Mathias


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/