[Devel] [PATCH vz9 v1 33/63] dm-ploop: convert md page rw lock to spin lock

[Alexander Atanasov]

On 6.02.25 10:42, Pavel Tikhomirov wrote:
> 
> 
> On 1/24/25 23:36, Alexander Atanasov wrote:
>> diff --git a/drivers/md/dm-ploop-cmd.c b/drivers/md/dm-ploop-cmd.c
>> index f98542e952e1..fa9f8c9d8d40 100644
>> --- a/drivers/md/dm-ploop-cmd.c
>> +++ b/drivers/md/dm-ploop-cmd.c
>> @@ -34,7 +34,7 @@ static void ploop_advance_holes_bitmap(struct ploop 
>> *ploop,
>>           return;
>>       cmd->resize.stage++;
>> -    write_lock_irq(&ploop->bat_rwlock);
>> +    write_lock_irqsave(&ploop->bat_rwlock, flags);
> 
> Why s/irq/irqsave/ ?
> 
>    +-< ploop_advance_holes_bitmap
>      +-< ploop_process_resize_cmd
>      | +-< ploop_resize
>      | | +-< ploop_message
>            +-< ploop_target->message
>              +-< target_message # ioctl - process context
> 
> It can't be called in interrupt context, thus should be "_irq".
> 
>>       /* Copy and swap holes_bitmap */
>>       size = DIV_ROUND_UP(ploop->hb_nr, 8);
>>       memcpy(cmd->resize.holes_bitmap, ploop->holes_bitmap, size);
>> @@ -46,7 +46,7 @@ static void ploop_advance_holes_bitmap(struct ploop 
>> *ploop,
>>           ploop_init_be_iter(ploop, md->id, &i, &end);
>>           bat_entries = md->kmpage;
>> -        spin_lock_irqsave(&md->md_lock, flags); /* read */
>> +        spin_lock(&md->md_lock); /* read */
>>           for (; i <= end; i++) {
>>               if (!ploop_md_page_cluster_is_in_top_delta(ploop, md, i))
>>                   continue;
>> @@ -57,9 +57,9 @@ static void ploop_advance_holes_bitmap(struct ploop 
>> *ploop,
>>                   ploop_hole_clear_bit(dst_clu, ploop);
>>               }
>>           }
>> -        spin_unlock_irqrestore(&md->md_lock, flags);
>> +        spin_unlock(&md->md_lock);
>>       }
>> -    write_unlock_irq(&ploop->bat_rwlock);
>> +    write_unlock_irqrestore(&ploop->bat_rwlock, flags);
> 
> Same.
> 
>>   }
>>   static int ploop_wait_for_completion_maybe_killable(struct 
>> completion *comp,
> 


Please, remember that we are called from DM in irq context and we try to 
prepare reqests as early as possible. Which means it does allocations 
and prepares BAT updates which do modify holes bitmap. Same code paths 
are called again in irq context where we complete requests and 
eventually restart delayed.

I don't know exactly what traces are you using but they miss the entry 
points.

Overall we must use irqsave/irqrestore almost every where except may be 
management commands that change only status.

-- 
Regards,
Alexander Atanasov