public inbox for linux-usb@vger.kernel.org
 help / color / mirror / Atom feed
From: "Michał Pecio" <michal.pecio@gmail.com>
To: Mathias Nyman <mathias.nyman@linux.intel.com>
Cc: linux-usb@vger.kernel.org, niklas.neronin@linux.intel.com
Subject: Re: My transfer ring grew to 740 segments
Date: Thu, 13 Mar 2025 09:46:32 +0100	[thread overview]
Message-ID: <20250313094632.037db6b3@foxbook> (raw)
In-Reply-To: <20250311234139.0e73e138@foxbook>

On Tue, 11 Mar 2025 23:41:39 +0100, Michał Pecio wrote:
> [102711.994235] xhci_hcd 0000:02:00.0: Event dma 0x00000000ffef4a50 for ep 6 status 4 not part of TD at 00000000eb22b790 - 00000000eb22b790
> [102711.994243] xhci_hcd 0000:02:00.0: Ring seg 0 dma 0x00000000ffef4000
> [102711.994246] xhci_hcd 0000:02:00.0: Ring seg 1 dma 0x00000000ffeee000
> [102711.994249] xhci_hcd 0000:02:00.0: Ring seg 2 dma 0x00000000ffc4e000
> 
> [ ... 735 lines omitted for brevity ... ]
> 
> [102711.995935] xhci_hcd 0000:02:00.0: Ring seg 738 dma 0x00000000eb2e2000
> [102711.995937] xhci_hcd 0000:02:00.0: Ring seg 739 dma 0x00000000eb22b000

And what are your thoughts about this noise? It's absurd to print such
long debug dumps under failure conditions (and hold a spinlock for 2ms
to do so), and I would argue that it is pointless even normally:

1. Almost always exactly two segments exist, and either
  a. the event and the TD are in the same segment, so who cares where
     the other segment is
  b. they are in different segments, and you can deduce both segments
     from dma pointers so the dump tells you absolutely nothing new

2. With more segments, the dump can tell if there were other segments
   between the event and the TD, but is it really important?

3. It might help with finding out-of-ring events, but this is extremely
   rare and should be done automatically (xhci_dma_to_trb() or similar).


Bottom line, the driver never printed it and no one other than Niklas
(Cc) seemed to really miss such a feature. 

I would be inclined to submit a small patch which removes this segment
dump, as I have already done so locally. Or at least make it xhci_dbg()
if somebody can present a convincing case for having it around.

Note that debugfs exists and provides this and much more information,
at least so long as the class driver doesn't disable the endpoint.

Regards,
Michal

  parent reply	other threads:[~2025-03-13  8:46 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-11 22:41 My transfer ring grew to 740 segments Michał Pecio
2025-03-12 13:37 ` Mathias Nyman
2025-03-13  7:54   ` Michał Pecio
2025-03-13  8:46 ` Michał Pecio [this message]
2025-03-13  9:45   ` Neronin, Niklas
2025-03-14  8:10     ` Michał Pecio
2025-03-13 14:43   ` Mathias Nyman
2025-03-14 19:15 ` David Laight
2025-03-16 10:27   ` Michał Pecio
2025-03-16 13:20     ` David Laight

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=20250313094632.037db6b3@foxbook \
    --to=michal.pecio@gmail.com \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@linux.intel.com \
    --cc=niklas.neronin@linux.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