[Devel] [PATCH vz9] dm-ploop: fix and rework md updates

[Pavel Tikhomirov]

On 2/11/25 22:25, Alexander Atanasov wrote:
> @@ -2067,21 +2109,46 @@ static inline int ploop_submit_metadata_writeback(struct ploop *ploop, int force
>   	 */
>   	llist_for_each_safe(pos, t, ll_wb_batch) {
>   		md = list_entry((struct list_head *)pos, typeof(*md), wb_link);
> +		INIT_LIST_HEAD(&md->wb_link);
>   		/* XXX: fixme delay results in a hang - TBD */
>   		if (1 || !llist_empty(&md->wait_llist) || force ||
>   		    test_bit(MD_HIGHPRIO, &md->status) ||
>   		    time_before(md->dirty_timeout, timeout) ||
>   		    ploop->force_md_writeback) {
> +			spin_lock_irqsave(&ploop->bat_lock, flags);
> +			if (test_bit(MD_UPDATING, &md->status) ||
> +			    test_bit(MD_WRITEBACK, &md->status)) {
> +				/* if updating or there is a writeback then delay */
> +				spin_unlock_irqrestore(&ploop->bat_lock, flags);
> +				llist_add((struct llist_node *)&md->wb_link,
> +					  &ploop->wb_batch_llist);
> +				continue;
> +			}
> +
>   			/* L1L2 mustn't be redirtyed, when wb in-flight! */
> -			WARN_ON_ONCE(!test_bit(MD_DIRTY, &md->status));
> -			WARN_ON_ONCE(test_bit(MD_WRITEBACK, &md->status));
> -			set_bit(MD_WRITEBACK, &md->status);
> -			clear_bit(MD_DIRTY, &md->status);
> +			if (WARN_ON_ONCE(!test_bit(MD_DIRTY, &md->status)))
> +				PL_ERR("wb but dirty not set\n");

Maybe:

WARN_ONCE(!test_bit(MD_DIRTY, &md->status), PL_FMT("wb but dirty not 
set"), ploop_device_name(ploop));

> +
> +			if (WARN_ON_ONCE(test_bit(MD_WRITEBACK, &md->status)))
> +				PL_ERR("wb but writeback already set\n");

Having both WARN and PL_ERR is a bit ugly.

> +
>   			clear_bit(MD_HIGHPRIO, &md->status);
> -			ploop_index_wb_submit(ploop, md->piwb);
> -			ret++;
> +			if (md->piwb) {
> +				struct ploop_index_wb *piwb;
> +				/* New updates will go in wait list */
> +				set_bit(MD_WRITEBACK, &md->status);
> +				clear_bit(MD_DIRTY, &md->status);
> +				/* at this point we can clear md->piwb */
> +				piwb = md->piwb;
> +				md->piwb = NULL;
> +				/* hand off to threads */
> +				ploop_index_wb_submit(ploop, piwb);
> +				ret++;
> +			} else {
> +				PL_ERR("Unexpected md piwb is null");
> +			}
> +			spin_unlock_irqrestore(&ploop->bat_lock, flags);
>   		} else {
> -			INIT_LIST_HEAD(&md->wb_link);
>   			llist_add((struct llist_node *)&md->wb_link, &ploop->wb_batch_llist);
>   		}
>   	}

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