Currently dev_watchdog() loops through num_tx_queues[Number of TX queues
allocated at alloc_netdev_mq() time] instead of real_num_tx_queues
[Number of TX queues currently active in device] to detect transmit
queue time out. Make this efficient by using real_num_tx_queues.
Signed-off-by: Praveen Kumar Kannoju <[email protected]>
---
PS: Please let me know if I am missing something obvious here.
net/sched/sch_generic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index a9aadc4e6858..e7d41a25f0e8 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -506,7 +506,7 @@ static void dev_watchdog(struct timer_list *t)
unsigned int i;
unsigned long trans_start;
- for (i = 0; i < dev->num_tx_queues; i++) {
+ for (i = 0; i < dev->real_num_tx_queues; i++) {
struct netdev_queue *txq;
txq = netdev_get_tx_queue(dev, i);
--
2.31.1
Ping.
> -----Original Message-----
> From: Praveen Kumar Kannoju <[email protected]>
> Sent: 16 March 2023 12:04 AM
> To: [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; [email protected]
> Cc: Rajesh Sivaramasubramaniom <[email protected]>; Rama Nichanamatlu
> <[email protected]>; Manjunath Patil <[email protected]>; Praveen Kannoju
> <[email protected]>
> Subject: [PATCH RFC] net/sched: use real_num_tx_queues in dev_watchdog()
>
> Currently dev_watchdog() loops through num_tx_queues[Number of TX queues allocated at alloc_netdev_mq() time] instead of
> real_num_tx_queues [Number of TX queues currently active in device] to detect transmit queue time out. Make this efficient by
> using real_num_tx_queues.
>
> Signed-off-by: Praveen Kumar Kannoju <[email protected]>
> ---
> PS: Please let me know if I am missing something obvious here.
> net/sched/sch_generic.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c index a9aadc4e6858..e7d41a25f0e8 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -506,7 +506,7 @@ static void dev_watchdog(struct timer_list *t)
> unsigned int i;
> unsigned long trans_start;
>
> - for (i = 0; i < dev->num_tx_queues; i++) {
> + for (i = 0; i < dev->real_num_tx_queues; i++) {
> struct netdev_queue *txq;
>
> txq = netdev_get_tx_queue(dev, i);
> --
> 2.31.1
On Tue, Mar 21, 2023 at 3:05 AM Praveen Kannoju
<[email protected]> wrote:
>
> Ping.
>
I do not think dev_watchdog() needs to be efficient ?
In any case, reading dev->real_num_tx_queues from a timer handler
could be racy vs RTNL.
While reading dev->num_tx_queues is not racy.
I think you should describe what problem you are trying to solve.
> > -----Original Message-----
> > From: Praveen Kumar Kannoju <[email protected]>
> > Sent: 16 March 2023 12:04 AM
> > To: [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected]; [email protected]
> > Cc: Rajesh Sivaramasubramaniom <[email protected]>; Rama Nichanamatlu
> > <[email protected]>; Manjunath Patil <[email protected]>; Praveen Kannoju
> > <[email protected]>
> > Subject: [PATCH RFC] net/sched: use real_num_tx_queues in dev_watchdog()
> >
> > Currently dev_watchdog() loops through num_tx_queues[Number of TX queues allocated at alloc_netdev_mq() time] instead of
> > real_num_tx_queues [Number of TX queues currently active in device] to detect transmit queue time out. Make this efficient by
> > using real_num_tx_queues.
> >
> > Signed-off-by: Praveen Kumar Kannoju <[email protected]>
> > ---
> > PS: Please let me know if I am missing something obvious here.
> > net/sched/sch_generic.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c index a9aadc4e6858..e7d41a25f0e8 100644
> > --- a/net/sched/sch_generic.c
> > +++ b/net/sched/sch_generic.c
> > @@ -506,7 +506,7 @@ static void dev_watchdog(struct timer_list *t)
> > unsigned int i;
> > unsigned long trans_start;
> >
> > - for (i = 0; i < dev->num_tx_queues; i++) {
> > + for (i = 0; i < dev->real_num_tx_queues; i++) {
> > struct netdev_queue *txq;
> >
> > txq = netdev_get_tx_queue(dev, i);
> > --
> > 2.31.1
>
Thank you for the reply, Eric.
During net device allocation through the routine "alloc_netdev_mqs()", both the variables "num_tx_queues" and "real_num_tx_queues" are being initialized with same value. After which, drivers chose the number of active queues which are being supported by them and update the variable " real_num_tx_queues " using the routine "netif_set_real_num_tx_queues()".
The intention of sending this patch was, Is it not efficient to monitor only the queues which are actively being used by the device instead of ones which are not at all participating in data communication.
For example we have seen that for a bnxt interface the values of "num_tx_queues" and "real_num_tx_queues" are 74 and 8 respectively, and for IGB interface they were 8 and 4. So around 50% to 90% of un-necessary queues are being monitored by the device watchdog at every interval.
-
Praveen
> -----Original Message-----
> From: Eric Dumazet <[email protected]>
> Sent: 21 March 2023 04:38 PM
> To: Praveen Kannoju <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Rajesh Sivaramasubramaniom <[email protected]>;
> Rama Nichanamatlu <[email protected]>; Manjunath Patil <[email protected]>
> Subject: Re: [PATCH RFC] net/sched: use real_num_tx_queues in dev_watchdog()
>
> On Tue, Mar 21, 2023 at 3:05 AM Praveen Kannoju <[email protected]> wrote:
> >
> > Ping.
> >
>
> I do not think dev_watchdog() needs to be efficient ?
>
> In any case, reading dev->real_num_tx_queues from a timer handler could be racy vs RTNL.
>
> While reading dev->num_tx_queues is not racy.
>
> I think you should describe what problem you are trying to solve.
>
> > > -----Original Message-----
> > > From: Praveen Kumar Kannoju <[email protected]>
> > > Sent: 16 March 2023 12:04 AM
> > > To: [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]
> > > Cc: Rajesh Sivaramasubramaniom
> > > <[email protected]>; Rama Nichanamatlu
> > > <[email protected]>; Manjunath Patil
> > > <[email protected]>; Praveen Kannoju
> > > <[email protected]>
> > > Subject: [PATCH RFC] net/sched: use real_num_tx_queues in
> > > dev_watchdog()
> > >
> > > Currently dev_watchdog() loops through num_tx_queues[Number of TX
> > > queues allocated at alloc_netdev_mq() time] instead of
> > > real_num_tx_queues [Number of TX queues currently active in device] to detect transmit queue time out. Make this efficient by
> using real_num_tx_queues.
> > >
> > > Signed-off-by: Praveen Kumar Kannoju <[email protected]>
> > > ---
> > > PS: Please let me know if I am missing something obvious here.
> > > net/sched/sch_generic.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c index
> > > a9aadc4e6858..e7d41a25f0e8 100644
> > > --- a/net/sched/sch_generic.c
> > > +++ b/net/sched/sch_generic.c
> > > @@ -506,7 +506,7 @@ static void dev_watchdog(struct timer_list *t)
> > > unsigned int i;
> > > unsigned long trans_start;
> > >
> > > - for (i = 0; i < dev->num_tx_queues; i++) {
> > > + for (i = 0; i < dev->real_num_tx_queues; i++)
> > > + {
> > > struct netdev_queue *txq;
> > >
> > > txq = netdev_get_tx_queue(dev, i);
> > > --
> > > 2.31.1
> >