On Thu, 10 Feb 2011, loody wrote:
> Hi:
>
> 2011/2/9 Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>:
> > On Wed, 9 Feb 2011, loody wrote:
> >
> >> Hi all:
> >> I have some questions about usb driver:
> >> 1. what is qh->stamp used for?
> >
> > It is used to prevent the CPU from doing unnecessary and unwanted work.
> >
> >> I trace the source and I found it seems only used to increase the
> >> performance such that we will avoid re-scan the previous qh.
> >
> > That's right.
> >
> >> But if we don't check qh->stamp, in qh_completions, it will return
> >> immediately if the qh has been scanned before, since the qtd_list of
> >> the qh is empty, right.
Wrong. Again, why do you think the qtd_list will be empty?
> > What makes you think the qtd_list will be empty? Didn't you read the
> > code in scan_async?
> >
> > /* clean any finished work for this qh */
> > if (!list_empty (&qh->qtd_list)
> > && qh->stamp != ehci->stamp) {
> >
> > Obviously the call to qh_completions won't occur if the list is empty.
>
> if so, why we still add below adjournment in qh_completions, even we
> use unlikely in the "if statement"?
> if (unlikely (list_empty (&qh->qtd_list)))
> return count;
Because qh_completions is called from two different places, and in one
of those places (the call in end_unlink_async, not the call mentioned
above) there is no check for the list being empty.
> Per your explanation, "qh->stamp is used for prevent the CPU from
> doing unnecessary and unwanted work."
>
> If I trace the code in the correct way, "the unnecessary and unwanted
> work" is scanning qtd_list and it will be empty after the first time
> the qh is been scanned.
No. It _might_ be empty, but it doesn't have to be.
> The next time we bump to the same qh it will be screen out in the
> statement , "if (!list_empty (&qh->qtd_list)", except the qh is
> submitted with any new urbs in the past.
>
> Is there any situation that "!list_empty(&qh->qtd_list) && (qh->stamp
> == ehci->stamp)"?
You have ignored these lines in qh_completions:
/* ignore urbs submitted during completions we reported */
if (qtd == end)
break;
and also:
/* stop scanning when we reach qtds the hc is using */
} else if (likely (!stopped
&& HC_IS_RUNNING (ehci_to_hcd(ehci)->state))) {
break;
The loop will end and the function will return if either "break"
statement is hit, even if qtd_list isn't empty. If scan_async then
executes the "goto rescan" statement, it will create the situation
you're asking about.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
