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  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]
Date:   Fri, 13 Mar 2020 20:31:36 -0500
From:   Jeff Layton <jlayton@...nel.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>,
        NeilBrown <neilb@...e.de>
Cc:     yangerkun <yangerkun@...wei.com>,
        kernel test robot <rong.a.chen@...el.com>,
        LKML <linux-kernel@...r.kernel.org>, lkp@...ts.01.org,
        Bruce Fields <bfields@...ldses.org>,
        Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [locks] 6d390e4b5d: will-it-scale.per_process_ops -96.6%
 regression

On Thu, 2020-03-12 at 09:07 -0700, Linus Torvalds wrote:
> On Wed, Mar 11, 2020 at 9:42 PM NeilBrown <neilb@...e.de> wrote:
> > It seems that test_and_set_bit_lock() is the preferred way to handle
> > flags when memory ordering is important
> 
> That looks better.
> 
> The _preferred_ way is actually the one I already posted: do a
> "smp_store_release()" to store the flag (like a NULL pointer), and a
> smp_load_acquire() to load it.
> 
> That's basically optimal on most architectures (all modern ones -
> there are bad architectures from before people figured out that
> release/acquire is better than separate memory barriers), not needing
> any atomics and only minimal memory ordering.
> 
> I wonder if a special flags value (keeping it "unsigned int" to avoid
> the issue Jeff pointed out) might be acceptable?
> 
> IOW, could we do just
> 
>         smp_store_release(&waiter->fl_flags, FL_RELEASED);
> 
> to say that we're done with the lock? Or do people still look at and
> depend on the flag values at that point?

I think nlmsvc_grant_block does. We could probably work around it
there, but we'd need to couple this change with some clear
documentation to make it clear that you can't rely on fl_flags after
locks_delete_block returns.

If avoiding new locks is preferred here (and I'm fine with that), then
maybe we should just go with the patch you sent originally (along with
changing the waiters to wait on fl_blocked_member going empty instead
of the fl_blocker going NULL)?

-- 
Jeff Layton <jlayton@...nel.org>

Powered by blists - more mailing lists