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: <48f2c6f2-f776-be29-7d74-67f1b6cf5467@csgroup.eu>
Date:   Thu, 6 Apr 2023 13:38:07 +0000
From:   Christophe Leroy <christophe.leroy@...roup.eu>
To:     David Laight <David.Laight@...LAB.COM>,
        '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>,
        "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



Le 06/04/2023 à 10:15, David Laight a écrit :
> 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.

On powerpc 'long long' is 64 bits on both PPC32 and PPC64.

Christophe


> 
> 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)