[Devel] [RFC PATCH vz9 v6 21/62] dm-ploop: introduce per-md page locking

[Pavel Tikhomirov]

On 12/6/24 05:55, Alexander Atanasov wrote:
> @@ -1045,10 +1054,12 @@ static int ploop_check_delta_before_flip(struct ploop *ploop, struct file *file)
>   	/* Points to hdr since md_page[0] also contains hdr. */
>   	d_md = ploop_md_first_entry(&md_root);
>   
> -	write_lock_irq(&ploop->bat_rwlock);
>   	ploop_for_each_md_page(ploop, md, node) {
>   		init_be_iter(nr_be, md->id, &i, &end);
>   		d_bat_entries = d_md->kmpage;
> +
> +		read_lock_irqsave(&md->lock, flags);
> +		read_lock(&d_md->lock);
>   		for (; i <= end; i++) {
>   			if (ploop_md_page_cluster_is_in_top_delta(ploop, md, i) &&
>   			    d_bat_entries[i] != BAT_ENTRY_NONE) {

Why do we replace _irq locks to _irqsave? The rule of thumb is:

- lock with no prefix should be used in interrupt context,
- lock with _irq prefix should be used in non-interrupt context,
- lock with _irqsave prefix should be used in places where both 
interrupt and non-interrupt context are possible.

For instance for the use of _irqsave in ploop_delay_if_md_busy (which 
exists before this patch) we have both interrupt and process contexts:

   +-< ploop_delay_if_md_busy
     +-< ploop_submit_cow_index_wb
       +-< ploop_process_discard_pios
         +-< do_ploop_run_work
     +-< ploop_locate_new_cluster_and_attach_pio
       +-< ploop_process_one_deferred_bio
         +-< ploop_process_deferred_pios
           +-< do_ploop_run_work
         +-< ploop_submit_embedded_pio
           +-< ploop_resume_submitting_pios
             +-< ploop_suspend_submitting_pios
               +-< ploop_resize
               +-< ploop_merge_latest_snapshot
               +-< ploop_delta_clusters_merged
               +-< ploop_update_delta_index
             +-< ploop_resize
               +-< ploop_message
             +-< ploop_merge_latest_snapshot
               +-< ploop_message
             +-< ploop_delta_clusters_merged
               +-< ploop_notify_merged
                 +-< ploop_message
             +-< ploop_update_delta_index
               +-< ploop_message
           +-< ploop_enospc_timer # interrupt context from timer
     +-< ploop_process_one_discard_pio
       +-< ploop_process_discard_pios
         +-< do_ploop_run_work

   +-< ploop_message
     +-< ploop_target->message
       +-< target_message # ioctl - process context

   +-< do_ploop_run_work
     +-< do_ploop_work # workqueue - process context
     +-< ploop_worker # kthread - process context

If we'll change everything to _irqsave we would lose deep understanding 
why we need to protect against irq context at all, now it looks that we 
only need it for enospc timers.

-- 
Best regards, Tikhomirov Pavel
Senior Software Developer, Virtuozzo.