Some problems about xhci_ring_expansion

Some problems about xhci_ring_expansion

[chao zeng]

hello!
  Thank you for taking the time to look at my question.

  At file xhci-ring.c
static inline int room_on_ring(struct xhci_hcd *xhci, struct xhci_ring *ring,
                unsigned int num_trbs)
{
        int num_trbs_in_deq_seg;

        if (ring->num_trbs_free < num_trbs)
                return 0;

        if (ring->type != TYPE_COMMAND && ring->type != TYPE_EVENT) {
                num_trbs_in_deq_seg = ring->dequeue - ring->deq_seg->trbs;
                if (ring->num_trbs_free < num_trbs + num_trbs_in_deq_seg)
                        return 0;////suppose return here
        }

        return 1;
}

Suppose the function room_on_ring returns in my bolded condition.
num_trbs_needed will be a very large value because the num_trbs <
num_trbs_free. In this way , we will just double the total ring size.
Is this as expected or should add one segment size instead?
                num_trbs_needed = num_trbs - ep_ring->num_trbs_free;//
unsigned int num_trbs_needed will be very large value
                if (xhci_ring_expansion(xhci, ep_ring, num_trbs_needed,
                                        mem_flags)) {
                        xhci_err(xhci, "Ring expansion failed\n");
                        return -ENOMEM;
                }



BR
Chao.Zeng

Re: Some problems about xhci_ring_expansion

[Mathias Nyman]

Hi

On 16.11.2022 10.46, chao zeng wrote:
> hello!
>    Thank you for taking the time to look at my question.
> 
>    At file xhci-ring.c
> static inline int room_on_ring(struct xhci_hcd *xhci, struct xhci_ring *ring,
>                  unsigned int num_trbs)
> {
>          int num_trbs_in_deq_seg;
> 
>          if (ring->num_trbs_free < num_trbs)
>                  return 0;
> 
>          if (ring->type != TYPE_COMMAND && ring->type != TYPE_EVENT) {
>                  num_trbs_in_deq_seg = ring->dequeue - ring->deq_seg->trbs;
>                  if (ring->num_trbs_free < num_trbs + num_trbs_in_deq_seg)
>                          return 0;////suppose return here
>          }
> 
>          return 1;
> }
> 
> Suppose the function room_on_ring returns in my bolded condition.
> num_trbs_needed will be a very large value because the num_trbs <
> num_trbs_free. In this way , we will just double the total ring size.

You are correct, good point.
So it turns out we almost always double the ring size when we need
more space, and we do this just because num_trbs_neeed is completely incorrect.
(trying to store negative value in unsigned int)

> Is this as expected or should add one segment size instead?

That's a good question.
Code should be fixed, but do we want to continue doubling ring size, or add
just enough segments to fit actual num_trbs_needed, or perhaps add enough
segments to fit twice the amount of needed trbs?

Would you like work on this? patches are welcome

Thanks
-Mathias