Re: My transfer ring grew to 740 segments

[Mathias Nyman <mathias.nyman@linux.intel.com>]

On 13.3.2025 10.46, Michał Pecio wrote:
> 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.

I don't object to that, we can get rid of it.

But to be fair, didn't it assist in detecting the ~700 segment ring expansion
you just found :)

Thanks
Mathias