From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: Ikjoon Jang <ikjn@chromium.org>, linux-usb@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Mathias Nyman <mathias.nyman@intel.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] xhci: fix unmatched num_trbs_free
Date: Fri, 16 Jul 2021 15:56:35 +0300 [thread overview]
Message-ID: <e13a1d38-cc0a-51b4-fe19-7a9145e75b1d@linux.intel.com> (raw)
In-Reply-To: <20210708164256.1.Ib344a977b52486ec81b60f9820338f1b43655f8d@changeid>
On 8.7.2021 11.43, Ikjoon Jang wrote:
> When unlinked urbs are queued to the cancelled td list, many tds
> might be located after hw dequeue pointer and just marked as no-op
> but not reclaimed to num_trbs_free. This bias can leads to unnecessary
> ring expansions and leaks in atomic pool.
Good point, in that case trbs turned no-op never get added to free trb count.
>
> To prevent this bias, this patch counts free TRBs every time xhci moves
> dequeue pointer. This patch utilizes existing
> update_ring_for_set_deq_completion() function, renamed it to move_deq().
>
> When it walks through to the new dequeue pointer, it also counts
> free TRBs manually. This patch adds a fast path for the most cases
> where the new dequeue pointer is still in the current segment.
>
This looks like an option.
Another approach would be to keep the normal case fast, and the special case code simple.
Something like:
finish_td()
...
/* Update ring dequeue pointer */
if (ep_ring->dequeue == td->first_trb) {
ep_ring->dequeue = td->last_trb;
ep_ring->deq_seg = td->last_trb_seg;
ep_ring->num_trbs_free += td->num_trbs - 1;
inc_deq(xhci, ep_ring);
} else {
move_deq(...);
}
move_deq(...)
{
while(ring->dequeue != new_dequeue)
inc_deq(ring);
inc_deq(ring);
}
inc_deq() increases the num_trbs_free count.
I haven't looked at the details of this yet, but I'm away for the next two weeks so
I wanted to share this first anyway.
-Mathias
next prev parent reply other threads:[~2021-07-16 12:54 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-08 8:43 [PATCH] xhci: fix unmatched num_trbs_free Ikjoon Jang
2021-07-16 12:56 ` Mathias Nyman [this message]
2021-07-19 4:36 ` Ikjoon Jang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e13a1d38-cc0a-51b4-fe19-7a9145e75b1d@linux.intel.com \
--to=mathias.nyman@linux.intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=ikjn@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).