linux-kernel - RE: [PATCH v2 5/6] debugfs: add debugfs_create_atomic64_t for atomic64

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: <979c8cf9ab2d4fcba811adc0c563b8bb@AcuMS.aculab.com>
Date:   Thu, 6 Apr 2023 08:15:51 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Luis Chamberlain' <mcgrof@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>
CC:     "david@...hat.com" <david@...hat.com>,
        "patches@...ts.linux.dev" <patches@...ts.linux.dev>,
        "linux-modules@...r.kernel.org" <linux-modules@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "pmladek@...e.com" <pmladek@...e.com>,
        "petr.pavlu@...e.com" <petr.pavlu@...e.com>,
        "prarit@...hat.com" <prarit@...hat.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "rafael@...nel.org" <rafael@...nel.org>,
        "christophe.leroy@...roup.eu" <christophe.leroy@...roup.eu>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "song@...nel.org" <song@...nel.org>,
        "rppt@...nel.org" <rppt@...nel.org>,
        "dave@...olabs.net" <dave@...olabs.net>,
        "willy@...radead.org" <willy@...radead.org>,
        "vbabka@...e.cz" <vbabka@...e.cz>,
        "mhocko@...e.com" <mhocko@...e.com>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "colin.i.king@...il.com" <colin.i.king@...il.com>,
        "jim.cromie@...il.com" <jim.cromie@...il.com>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "jbaron@...mai.com" <jbaron@...mai.com>,
        "rick.p.edgecombe@...el.com" <rick.p.edgecombe@...el.com>
Subject: RE: [PATCH v2 5/6] debugfs: add debugfs_create_atomic64_t for
 atomic64_t

From: Luis Chamberlain  Luis Chamberlain
> Sent: 05 April 2023 17:53
> 
> On Wed, Apr 05, 2023 at 09:23:09AM -0700, Linus Torvalds wrote:
> > On Wed, Apr 5, 2023 at 9:11 AM Luis Chamberlain <mcgrof@...nel.org> wrote:
> > >
> > > Oh but I don't get the atomic incs, so we'd need debugfs_create_atomic_long_t().
> >
> >   debugfs_create_ulong("total_mod_size",
> >        0400, mod_debugfs_root,
> >        &total_mod_size.counter);
> >
> > but I didn't actually try to compile that kind of version.
> >
> > (I think "counter" is actually a _signed_ long, so maybe the types don't match).
> 
> I see yes, sadly we'd have to cast to (unsigned long *) to make that
> work as atomic_long counters are long long int:
> 
>    debugfs_create_ulong("total_mod_size",
>         0400, mod_debugfs_root,
> -        &total_mod_size.counter);
> +        (unsigned long *)&total_mod_size.counter);
> 
> That's:
> 
> unsigned long min bits 32
> long long     min bits 64
> 
> But since we'd be doing our accounting in atomic_long and just use
> debugfs for prints I think that's fine. Just a bit ugly.

That isn't going to work.
It is pretty much never right to do *(integer_type *)&integer_variable.

But are you really sure 'atomic_long' is 'long long'
doesn't sound right at all.
'long long' is 128bit on 64bit and 64bit on 32bit - so is always
a double-register access.
This is worse than atomic_u64.

I should probably try to lookup and/or measure the performance
of atomic operations (esp. cmpxchg) on x86.
Historically they were a separate read and write bus cycles with
the 'lock' signal asserted (and still are if they cross cache
line boundaries).
But it is possible that at least some of the locked operations
(esp. the xchg ones) are implemented within the cache itself
so are just a single cpu -> cache operation.
If not it is actually possible that the _local variants that
seem to have been added should not use the locked instructions!

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)