netdev - Re: [PATCH] sched: Optimize return value of qdisc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <OFF6C86B0A.6A40B31A-ON652572D7.004EC8D4-652572D7.004F49D0@in.ibm.com>
Date:	Thu, 10 May 2007 19:56:02 +0530
From:	Krishna Kumar2 <krkumar2@...ibm.com>
To:	hadi@...erus.ca
Cc:	davem@...emloft.net, Herbert Xu <herbert@...dor.apana.org.au>,
	netdev@...r.kernel.org, Roberto Nibali <ratz@...gphish.ch>,
	Thomas Graf <tgraf@...g.ch>
Subject: Re: [PATCH] sched: Optimize return value of qdisc_restart

I would guess that even with this change, qdisc_restart can get called with
no
skbs in the queue due to softirq which was enabled (but by the time it ran,
more skbs enqueue could have dequeue'd those packets). But it would
definitely reduce by once each iteration of qdisc_run (if one packet then
50% reduction, if 9 packets then 10% reduction, and so on).

Herbert,

-            while (qdisc_restart(dev) < 0 && !netif_queue_stopped(dev))
-                        /* NOTHING */;
+            do {
+                        if (!qdisc_restart(dev))
+                                    break;
+            } while (!netif_queue_stopped(dev));

This could be changed to old format, if required :

+            while (qdisc_restart(dev)) && !netif_queue_stopped(dev))
+                 /* NOTHING */;

thanks,

- KK

J Hadi Salim <j.hadi123@...il.com> wrote on 05/10/2007 07:42:55 PM:

> On Thu, 2007-10-05 at 23:52 +1000, Herbert Xu wrote:
> > On Thu, May 10, 2007 at 09:18:04AM -0400, jamal wrote:
> > >
> > > I wonder how much performance improvement this give now that the
extra
> > > incursion into qdisc_restart is avoided.
> >
> > Probably very little since the only way it can make a difference is if
> > there is lots of contention.  But if there is lots of contention we
> > should stay in the function anyway.
>
> Less to do with CPU contention and more with Krishna's change which
> avoids entering that region all together which is useful even in the
> absence of contention. In the past (based on some measurement i did)
> upto 50% of the executions of qdisc restart were on an empty queue
> depending on traffic rates.
>
> BTW, given that i couldnt recall the layout of the locks on qdisc
> restart; I think i will come back with the old patch i had to clean up
> the readability of qdisc restart so it is easy to tell just by quick
> inspection.
>
> Since Canada seems to be comfortably taking revenge on the Swiss (my
> condolonces to Thomas and Ratz) - i think it is time for me to head
> out ;-> ttl.
>
> cheers,
> jamal
>

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html