Re: [PATCH v2 5/6] debugfs: add debugfs_create_atomic64_t for atomic64_t

[Christophe Leroy]

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