[Devel] [RFC PATCH vz9 v6 02/62] dm-ploop: Use READ_ONCE/WRITE_ONCE to access md page data

[Alexander Atanasov]

On 20.01.25 12:54, Pavel Tikhomirov wrote:
> 
> 
> On 1/20/25 16:03, Alexander Atanasov wrote:
>> On 20.01.25 9:23, Pavel Tikhomirov wrote:
>>>
>>>
>>> On 12/6/24 05:55, Alexander Atanasov wrote:
>>>> @@ -1090,9 +1090,9 @@ static int ploop_alloc_cluster(struct ploop 
>>>> *ploop, struct ploop_index_wb *piwb,
>>>>       clu -= piwb->page_id * PAGE_SIZE / sizeof(map_index_t) - 
>>>> PLOOP_MAP_OFFSET;
>>>>       to = piwb->kmpage;
>>>> -    if (to[clu]) {
>>>> +    if (READ_ONCE(to[clu])) {
>>>>           /* Already mapped by one of previous bios */
>>>> -        *dst_clu = to[clu];
>>>> +        *dst_clu = READ_ONCE(to[clu]);
>>>>           already_alloced = true;
>>>>       }
>>>
>>> The above hunk does not look good, we explicitly do 
>>> "READ_ONCE(to[clu])" twice, likely that's not what we want.
>>
>>
>> We can do without inner READ_ONCE but for consistency i think it is 
>> better. /it would be optimised by the compiler anyway/
>>
>> The idea is that if a clu is already mapped we read !=0 in the if,
>> inside we will always read the !=0 too, since discard is synchronized 
>> by the holes bitmap.
> 
> That may be, if clu can't be un-mapped or somehow reset... Properly 
> validating it will require checking all 24 uses of ->kmpage.


md->kmpage is mapped at init time and unmapped at destroy time.
( [01/62] dm-ploop: md_pages map all pages at creation time )
piwb->kmpage is gone in updated patches.


> It would be much cleaner and simpler to do:
> 
> tmp_var = READ_ONCE(smth)
> if (tmp_var) {
>      *ptr_var = tmp_var;
> }

Ok.

> I doubt that you will find any other example of doing READ_ONCE of the 
> same shared memory/variable twice in one function.

There might be but this is irrelevant.

in this case it can be done without temp variable.

if (READ_ONCE(var)) {
       *ptr_var = var;
}

*ptr_var = var; will be changed to *ptr_var = value_from_read_once(var);

so it is the same

-- 
Regards,
Alexander Atanasov