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  PHC 
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 15 May 2020 15:11:59 +0200
From:   Arnd Bergmann <>
To:     Bartosz Golaszewski <>
Cc:     Jonathan Corbet <>, Rob Herring <>,
        "David S . Miller" <>,
        Matthias Brugger <>,
        John Crispin <>,
        Sean Wang <>,
        Mark Lee <>,
        Jakub Kicinski <>,
        Fabien Parent <>,
        Heiner Kallweit <>,
        Edwin Peer <>,
        DTML <>,
        "" <>,
        Networking <>,
        Linux ARM <>,
        "moderated list:ARM/Mediatek SoC..." 
        Stephane Le Provost <>,
        Pedro Tsai <>,
        Andrew Perepech <>,
        Bartosz Golaszewski <>
Subject: Re: [PATCH v3 10/15] net: ethernet: mtk-eth-mac: new driver

On Fri, May 15, 2020 at 2:56 PM Bartosz Golaszewski <> wrote:
> pt., 15 maj 2020 o 14:04 Arnd Bergmann <> napisaƂ(a):
> > On Fri, May 15, 2020 at 9:11 AM Bartosz Golaszewski <> wrote:

> > > >
> > > > It looks like most of the stuff inside of the loop can be pulled out
> > > > and only done once here.
> > > >
> > >
> > > I did that in one of the previous submissions but it was pointed out
> > > to me that a parallel TX path may fill up the queue before I wake it.
> >
> > Right, I see you plugged that hole, however the way you hold the
> > spinlock across the expensive DMA management but then give it
> > up in each loop iteration feels like this is not the most efficient
> > way.
> >
> Maybe my thinking is wrong here, but I assumed that with a spinlock
> it's better to give other threads the chance to run in between each
> iteration. I didn't benchmark it though.

It depends. You want to avoid lock contention (two threads trying to
get the lock at the same time) but you also want to avoid bouncing
around the spinlock between the caches.

In the contention case, what I think would happen here is that the
cleanup thread gives up the lock and the xmit function gets it, but
then the cleanup thread is spinning again, so you are still blocked
on one of the two CPUs but also pay the overhead of synchronizing
between the two.

Holding the lock the whole time would speed up both the good case
(no contention) and the bad case (bouncing the lock) a little bit
because it saves some overhead. Holding the lock for shorter
times (i.e. not during the cache operations) would reduce the
amount of lock-contention but not help in the good case.

Not needing a lock at all is generally best, but getting it right
is tricky ;-)


Powered by blists - more mailing lists