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
| ||
|
Message-ID: <20170904120009.ah2qu3lbgdqdgz6i@hirez.programming.kicks-ass.net> Date: Mon, 4 Sep 2017 14:00:09 +0200 From: Peter Zijlstra <peterz@...radead.org> To: "Reshetova, Elena" <elena.reshetova@...el.com> Cc: Thomas Gleixner <tglx@...utronix.de>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>, "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>, "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>, "tj@...nel.org" <tj@...nel.org>, "mingo@...hat.com" <mingo@...hat.com>, "hannes@...xchg.org" <hannes@...xchg.org>, "lizefan@...wei.com" <lizefan@...wei.com>, "acme@...nel.org" <acme@...nel.org>, "alexander.shishkin@...ux.intel.com" <alexander.shishkin@...ux.intel.com>, "eparis@...hat.com" <eparis@...hat.com>, "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>, "arnd@...db.de" <arnd@...db.de>, "luto@...nel.org" <luto@...nel.org>, "keescook@...omium.org" <keescook@...omium.org>, "dvhart@...radead.org" <dvhart@...radead.org>, "ebiederm@...ssion.com" <ebiederm@...ssion.com> Subject: Re: [PATCH 14/15] futex: convert futex_pi_state.refcount to refcount_t On Mon, Sep 04, 2017 at 10:31:54AM +0000, Reshetova, Elena wrote: > > > But can they make "fast" implementation on ARM that would give stronger > > > memory guarantees? > > > > Whatever for? > > Well, maybe just by default when arch.-specific implementation is > done. But I was just trying to speculate to understand. I will resend > this one with new comment added. So the generic lib/refcount.c already has weak ordering. It doesn't make sense for an arch specific implementation (on a weakly ordered machine) to provide stronger guarantees (it would make things slower). The weaker ordering of the refcount_t primitives is sufficient if we're talking pure refcounts. If for some reason code relies on stronger ordering there _SHOULD_ be a comment with describing the additional ordering requirements. But that's a fairly big 'should'. I can well imagine the comment not being there. In fact, see below. > Still not sure if I need to resend the whole series with updated > commits or break this up by individual patches further for the > separate merges. I've yet to look at the ones targeted at subsystems I do, I'm forever and terminally behind on review :/ I called out the issue on futex in particular because it is fairly tricky code that. Now Thomas would like you to mention the fact that refcount_t doesn't provide the exact same ordering as the atomic_t usages it replaces and I think it would be good if you could hand-wave an argument on why the futex code doesn't care. Now, suppose we were to convert i_count to refcount_t (yes, I know, my initial conversion wasn't well received), then we need to add futex_get_inode() similar to futex_get_mm(). That is, smp_mb__{before,after}_atomic() works as expected and can be used to fortify the implied barriers by refcount_t. --- Subject: fs,inode: Add comment explaining additional ordering Add a note to ihold() to document the ordering futex relies upon. Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org> --- fs/inode.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/inode.c b/fs/inode.c index 50370599e371..17192ba92fef 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -395,6 +395,10 @@ void __iget(struct inode *inode) */ void ihold(struct inode *inode) { + /* + * Note: futex.c:get_futex_key_refs() relies on this function + * implying an smp_mb(). + */ WARN_ON(atomic_inc_return(&inode->i_count) < 2); } EXPORT_SYMBOL(ihold);
Powered by blists - more mailing lists