[Devel] [RFC PATCH vz9 v6 39/62] dm-ploop: prepare bat updates under bat_lock

[Alexander Atanasov]

On 17.01.25 9:10, Pavel Tikhomirov wrote:
> Here is possible scenario of double-unlock of bat_lock:
>

fixed. also removed the goto out without cleanup.

> On 12/6/24 05:56, Alexander Atanasov wrote:
>> @@ -1758,23 +1789,29 @@ static void 
>> ploop_process_one_discard_pio(struct ploop *ploop, struct pio *pio)
>>       struct ploop_index_wb *piwb;
>>       struct md_page *md;
>>       map_index_t *to;
>> +    unsigned long flags;
>>       WARN_ON(ploop->nr_deltas != 1 ||
>>           pio->queue_list_id != PLOOP_LIST_DISCARD);
>>       page_id = ploop_bat_clu_to_page_nr(clu);
>>       md = ploop_md_page_find(ploop, page_id);
>> -    if (ploop_delay_if_md_busy(ploop, md, PIWB_TYPE_DISCARD, pio))
> 
> take bat_lock:
> 
>> +    spin_lock_irqsave(&ploop->bat_lock, flags);
>> +    if (ploop_delay_if_md_busy(ploop, md, PIWB_TYPE_DISCARD, pio)) {
>> +        spin_unlock_irqrestore(&ploop->bat_lock, flags);
>>           goto out;
>> +    }
>> +
>>       if (!test_bit(MD_DIRTY, &md->status)) {
>> -         /* Unlocked since MD_DIRTY is set and cleared from this work */
>>           if (ploop_prepare_bat_update(ploop, md, PIWB_TYPE_DISCARD) < 
>> 0) {
>>               pio->bi_status = BLK_STS_RESOURCE;
>>               goto err;
>>           }
>>           bat_update_prepared = true;
>> +        ploop_md_make_dirty(ploop, md);
>>       }
> 
> release bat_lock:
> 
>> +    spin_unlock_irqrestore(&ploop->bat_lock, flags);
>>       piwb = md->piwb;
>> @@ -1784,18 +1821,20 @@ static void 
>> ploop_process_one_discard_pio(struct ploop *ploop, struct pio *pio)
>>       to = piwb->kmpage;
>>       if (WARN_ON_ONCE(!to[clu])) {
>>           pio->bi_status = BLK_STS_IOERR;
>> +        clear_bit(MD_DIRTY, &md->status);
> 
> goto err:
> 
>>           goto err;
>>       } else {
>>           WRITE_ONCE(to[clu], 0);
>> +        spin_lock_irqsave(&piwb->lock, flags);
>>           list_add_tail(&pio->list, &piwb->ready_data_pios);
>> +        spin_unlock_irqrestore(&piwb->lock, flags);
>>       }
>> -    if (bat_update_prepared)
>> -        ploop_md_make_dirty(ploop, md);
>>       ploop_md_up_prio(ploop, md);
>>   out:
>>       return;
>>   err:
> 
> release bat lock again:
> 
>> +    spin_unlock_irqrestore(&ploop->bat_lock, flags);
>>       if (bat_update_prepared)
>>           ploop_break_bat_update(ploop, md);
>>       ploop_pio_endio(pio);
> 

-- 
Regards,
Alexander Atanasov