* [PATCH] thunderbolt: Confirm the necessity to configure asym link first @ 2025-06-26 8:41 zhangjianrong 2025-06-26 9:30 ` Mika Westerberg 0 siblings, 1 reply; 5+ messages in thread From: zhangjianrong @ 2025-06-26 8:41 UTC (permalink / raw) To: andreas.noever, michael.jamet, mika.westerberg, YehezkelShB, linux-usb, linux-kernel Cc: guhengsheng, caiyadong, xuetao09, lixinghang1 Current implementation can cause allocation failures in tb_alloc_dp_bandwidth() in some cases. For example: allocated_down(30Gbps), allocated_up(50Gbps), requested_down(10Gbps). Signed-off-by: zhangjianrong <zhangjianrong5@huawei.com> --- drivers/thunderbolt/tb.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c index a7c6919fbf97..558455d9716b 100644 --- a/drivers/thunderbolt/tb.c +++ b/drivers/thunderbolt/tb.c @@ -1039,6 +1039,9 @@ static int tb_configure_asym(struct tb *tb, struct tb_port *src_port, break; if (downstream) { + /* Does consumed + requested exceed the threshold */ + if (consumed_down + requested_down < asym_threshold) + continue; /* * Downstream so make sure upstream is within the 36G * (40G - guard band 10%), and the requested is above @@ -1048,20 +1051,17 @@ static int tb_configure_asym(struct tb *tb, struct tb_port *src_port, ret = -ENOBUFS; break; } - /* Does consumed + requested exceed the threshold */ - if (consumed_down + requested_down < asym_threshold) - continue; width_up = TB_LINK_WIDTH_ASYM_RX; width_down = TB_LINK_WIDTH_ASYM_TX; } else { /* Upstream, the opposite of above */ + if (consumed_up + requested_up < asym_threshold) + continue; if (consumed_down + requested_down >= TB_ASYM_MIN) { ret = -ENOBUFS; break; } - if (consumed_up + requested_up < asym_threshold) - continue; width_up = TB_LINK_WIDTH_ASYM_TX; width_down = TB_LINK_WIDTH_ASYM_RX; -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] thunderbolt: Confirm the necessity to configure asym link first 2025-06-26 8:41 [PATCH] thunderbolt: Confirm the necessity to configure asym link first zhangjianrong @ 2025-06-26 9:30 ` Mika Westerberg 2025-06-26 11:50 ` Charalampos Mitrodimas 0 siblings, 1 reply; 5+ messages in thread From: Mika Westerberg @ 2025-06-26 9:30 UTC (permalink / raw) To: zhangjianrong Cc: andreas.noever, michael.jamet, YehezkelShB, linux-usb, linux-kernel, guhengsheng, caiyadong, xuetao09, lixinghang1 Hi, On Thu, Jun 26, 2025 at 04:41:07PM +0800, zhangjianrong wrote: > Current implementation can cause allocation failures in > tb_alloc_dp_bandwidth() in some cases. For example: > allocated_down(30Gbps), allocated_up(50Gbps), > requested_down(10Gbps). I'm not sure I understand the above. Can you describe in which real life situation this can happen? > > Signed-off-by: zhangjianrong <zhangjianrong5@huawei.com> > --- > drivers/thunderbolt/tb.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c > index a7c6919fbf97..558455d9716b 100644 > --- a/drivers/thunderbolt/tb.c > +++ b/drivers/thunderbolt/tb.c > @@ -1039,6 +1039,9 @@ static int tb_configure_asym(struct tb *tb, struct tb_port *src_port, > break; > > if (downstream) { > + /* Does consumed + requested exceed the threshold */ > + if (consumed_down + requested_down < asym_threshold) > + continue; > /* > * Downstream so make sure upstream is within the 36G > * (40G - guard band 10%), and the requested is above > @@ -1048,20 +1051,17 @@ static int tb_configure_asym(struct tb *tb, struct tb_port *src_port, > ret = -ENOBUFS; > break; > } > - /* Does consumed + requested exceed the threshold */ > - if (consumed_down + requested_down < asym_threshold) > - continue; > > width_up = TB_LINK_WIDTH_ASYM_RX; > width_down = TB_LINK_WIDTH_ASYM_TX; > } else { > /* Upstream, the opposite of above */ > + if (consumed_up + requested_up < asym_threshold) > + continue; > if (consumed_down + requested_down >= TB_ASYM_MIN) { > ret = -ENOBUFS; > break; > } > - if (consumed_up + requested_up < asym_threshold) > - continue; > > width_up = TB_LINK_WIDTH_ASYM_TX; > width_down = TB_LINK_WIDTH_ASYM_RX; > -- > 2.34.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] thunderbolt: Confirm the necessity to configure asym link first 2025-06-26 9:30 ` Mika Westerberg @ 2025-06-26 11:50 ` Charalampos Mitrodimas 2025-06-27 4:46 ` Mika Westerberg 0 siblings, 1 reply; 5+ messages in thread From: Charalampos Mitrodimas @ 2025-06-26 11:50 UTC (permalink / raw) To: Mika Westerberg Cc: zhangjianrong, andreas.noever, michael.jamet, YehezkelShB, linux-usb, linux-kernel, guhengsheng, caiyadong, xuetao09, lixinghang1 Mika Westerberg <mika.westerberg@linux.intel.com> writes: > Hi, > > On Thu, Jun 26, 2025 at 04:41:07PM +0800, zhangjianrong wrote: >> Current implementation can cause allocation failures in >> tb_alloc_dp_bandwidth() in some cases. For example: >> allocated_down(30Gbps), allocated_up(50Gbps), >> requested_down(10Gbps). > > I'm not sure I understand the above. > > Can you describe in which real life situation this can happen? I suppose this can happen when reducing bandwidth while total upstream bandwidth usage on the link exceeds TB_ASYM_MIN (36 Gbps). The allocation fails at the asymmetric limit check before checking whether the downstream request actually needs asymmetric mode. > >> >> Signed-off-by: zhangjianrong <zhangjianrong5@huawei.com> >> --- >> drivers/thunderbolt/tb.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c >> index a7c6919fbf97..558455d9716b 100644 >> --- a/drivers/thunderbolt/tb.c >> +++ b/drivers/thunderbolt/tb.c >> @@ -1039,6 +1039,9 @@ static int tb_configure_asym(struct tb *tb, struct tb_port *src_port, >> break; >> >> if (downstream) { >> + /* Does consumed + requested exceed the threshold */ >> + if (consumed_down + requested_down < asym_threshold) >> + continue; >> /* >> * Downstream so make sure upstream is within the 36G >> * (40G - guard band 10%), and the requested is above >> @@ -1048,20 +1051,17 @@ static int tb_configure_asym(struct tb *tb, struct tb_port *src_port, >> ret = -ENOBUFS; >> break; >> } >> - /* Does consumed + requested exceed the threshold */ >> - if (consumed_down + requested_down < asym_threshold) >> - continue; >> >> width_up = TB_LINK_WIDTH_ASYM_RX; >> width_down = TB_LINK_WIDTH_ASYM_TX; >> } else { >> /* Upstream, the opposite of above */ >> + if (consumed_up + requested_up < asym_threshold) >> + continue; >> if (consumed_down + requested_down >= TB_ASYM_MIN) { >> ret = -ENOBUFS; >> break; >> } >> - if (consumed_up + requested_up < asym_threshold) >> - continue; >> >> width_up = TB_LINK_WIDTH_ASYM_TX; >> width_down = TB_LINK_WIDTH_ASYM_RX; >> -- >> 2.34.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] thunderbolt: Confirm the necessity to configure asym link first 2025-06-26 11:50 ` Charalampos Mitrodimas @ 2025-06-27 4:46 ` Mika Westerberg 2025-06-28 11:59 ` zhangjianrong (E) 0 siblings, 1 reply; 5+ messages in thread From: Mika Westerberg @ 2025-06-27 4:46 UTC (permalink / raw) To: Charalampos Mitrodimas Cc: zhangjianrong, andreas.noever, michael.jamet, YehezkelShB, linux-usb, linux-kernel, guhengsheng, caiyadong, xuetao09, lixinghang1 Hi, On Thu, Jun 26, 2025 at 11:50:00AM +0000, Charalampos Mitrodimas wrote: > Mika Westerberg <mika.westerberg@linux.intel.com> writes: > > > Hi, > > > > On Thu, Jun 26, 2025 at 04:41:07PM +0800, zhangjianrong wrote: > >> Current implementation can cause allocation failures in > >> tb_alloc_dp_bandwidth() in some cases. For example: > >> allocated_down(30Gbps), allocated_up(50Gbps), > >> requested_down(10Gbps). > > > > I'm not sure I understand the above. > > > > Can you describe in which real life situation this can happen? > > I suppose this can happen when reducing bandwidth while total upstream > bandwidth usage on the link exceeds TB_ASYM_MIN (36 Gbps). The > allocation fails at the asymmetric limit check before checking whether > the downstream request actually needs asymmetric mode. Right, but I would like to see here in the changelog explanation of the situation and preferably parts of the dmesg showing the error as well. @zhangjianrong, can you do that and resend? Note, I will be on vacation after today so expect delay from my side. > >> > >> Signed-off-by: zhangjianrong <zhangjianrong5@huawei.com> > >> --- > >> drivers/thunderbolt/tb.c | 10 +++++----- > >> 1 file changed, 5 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c > >> index a7c6919fbf97..558455d9716b 100644 > >> --- a/drivers/thunderbolt/tb.c > >> +++ b/drivers/thunderbolt/tb.c > >> @@ -1039,6 +1039,9 @@ static int tb_configure_asym(struct tb *tb, struct tb_port *src_port, > >> break; > >> > >> if (downstream) { > >> + /* Does consumed + requested exceed the threshold */ > >> + if (consumed_down + requested_down < asym_threshold) > >> + continue; > >> /* > >> * Downstream so make sure upstream is within the 36G > >> * (40G - guard band 10%), and the requested is above > >> @@ -1048,20 +1051,17 @@ static int tb_configure_asym(struct tb *tb, struct tb_port *src_port, > >> ret = -ENOBUFS; > >> break; > >> } > >> - /* Does consumed + requested exceed the threshold */ > >> - if (consumed_down + requested_down < asym_threshold) > >> - continue; > >> > >> width_up = TB_LINK_WIDTH_ASYM_RX; > >> width_down = TB_LINK_WIDTH_ASYM_TX; > >> } else { > >> /* Upstream, the opposite of above */ > >> + if (consumed_up + requested_up < asym_threshold) > >> + continue; > >> if (consumed_down + requested_down >= TB_ASYM_MIN) { > >> ret = -ENOBUFS; > >> break; > >> } > >> - if (consumed_up + requested_up < asym_threshold) > >> - continue; > >> > >> width_up = TB_LINK_WIDTH_ASYM_TX; > >> width_down = TB_LINK_WIDTH_ASYM_RX; > >> -- > >> 2.34.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] thunderbolt: Confirm the necessity to configure asym link first 2025-06-27 4:46 ` Mika Westerberg @ 2025-06-28 11:59 ` zhangjianrong (E) 0 siblings, 0 replies; 5+ messages in thread From: zhangjianrong (E) @ 2025-06-28 11:59 UTC (permalink / raw) To: Mika Westerberg, Charalampos Mitrodimas Cc: andreas.noever, michael.jamet, YehezkelShB, linux-usb, linux-kernel, guhengsheng, caiyadong, xuetao09, lixinghang1 On 6/27/2025 12:46 PM, Mika Westerberg wrote: > Hi, > > On Thu, Jun 26, 2025 at 11:50:00AM +0000, Charalampos Mitrodimas wrote: >> Mika Westerberg <mika.westerberg@linux.intel.com> writes: >> >>> Hi, >>> >>> On Thu, Jun 26, 2025 at 04:41:07PM +0800, zhangjianrong wrote: >>>> Current implementation can cause allocation failures in >>>> tb_alloc_dp_bandwidth() in some cases. For example: >>>> allocated_down(30Gbps), allocated_up(50Gbps), >>>> requested_down(10Gbps). >>> >>> I'm not sure I understand the above. >>> >>> Can you describe in which real life situation this can happen? >> >> I suppose this can happen when reducing bandwidth while total upstream >> bandwidth usage on the link exceeds TB_ASYM_MIN (36 Gbps). The >> allocation fails at the asymmetric limit check before checking whether >> the downstream request actually needs asymmetric mode. > > Right, but I would like to see here in the changelog explanation of the > situation and preferably parts of the dmesg showing the error as well. OK, I will update the commit message, but I don't have the dmesg because I can't find a host router that contains two dp adapters. > > @zhangjianrong, can you do that and resend? > > Note, I will be on vacation after today so expect delay from my side. > >>>> >>>> Signed-off-by: zhangjianrong <zhangjianrong5@huawei.com> >>>> --- >>>> drivers/thunderbolt/tb.c | 10 +++++----- >>>> 1 file changed, 5 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c >>>> index a7c6919fbf97..558455d9716b 100644 >>>> --- a/drivers/thunderbolt/tb.c >>>> +++ b/drivers/thunderbolt/tb.c >>>> @@ -1039,6 +1039,9 @@ static int tb_configure_asym(struct tb *tb, struct tb_port *src_port, >>>> break; >>>> >>>> if (downstream) { >>>> + /* Does consumed + requested exceed the threshold */ >>>> + if (consumed_down + requested_down < asym_threshold) >>>> + continue; >>>> /* >>>> * Downstream so make sure upstream is within the 36G >>>> * (40G - guard band 10%), and the requested is above >>>> @@ -1048,20 +1051,17 @@ static int tb_configure_asym(struct tb *tb, struct tb_port *src_port, >>>> ret = -ENOBUFS; >>>> break; >>>> } >>>> - /* Does consumed + requested exceed the threshold */ >>>> - if (consumed_down + requested_down < asym_threshold) >>>> - continue; >>>> >>>> width_up = TB_LINK_WIDTH_ASYM_RX; >>>> width_down = TB_LINK_WIDTH_ASYM_TX; >>>> } else { >>>> /* Upstream, the opposite of above */ >>>> + if (consumed_up + requested_up < asym_threshold) >>>> + continue; >>>> if (consumed_down + requested_down >= TB_ASYM_MIN) { >>>> ret = -ENOBUFS; >>>> break; >>>> } >>>> - if (consumed_up + requested_up < asym_threshold) >>>> - continue; >>>> >>>> width_up = TB_LINK_WIDTH_ASYM_TX; >>>> width_down = TB_LINK_WIDTH_ASYM_RX; >>>> -- >>>> 2.34.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-06-28 11:59 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-06-26 8:41 [PATCH] thunderbolt: Confirm the necessity to configure asym link first zhangjianrong 2025-06-26 9:30 ` Mika Westerberg 2025-06-26 11:50 ` Charalampos Mitrodimas 2025-06-27 4:46 ` Mika Westerberg 2025-06-28 11:59 ` zhangjianrong (E)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox