[Devel] [PATCH rh7 1/2] ploop: Stop fast path reqs living their own lives

[Konstantin Khorenko]

On 03/03/2020 06:13 PM, Kirill Tkhai wrote:
> Doing ploop_quiesce() we want any IO becomes stopped.
> But fast path requests live their own lives, and this
> completelly ignores quiesce state. This may be a reason
> of unpredictable behaviour.
>
> This patch fixes the problem.
>
> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
> ---
>  drivers/block/ploop/dev.c   |   30 ++++++++++++++++++++++++++++--
>  include/linux/ploop/ploop.h |    2 ++
>  2 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c
> index 3f70f674e112..9e01866860bd 100644
> --- a/drivers/block/ploop/dev.c
> +++ b/drivers/block/ploop/dev.c
> @@ -164,6 +164,22 @@ static void ploop_uncongest(struct ploop_device *plo)
>  	}
>  }
>
> +static int fast_path_reqs_count(struct ploop_device *plo)
> +{
> +	int ret;
> +
> +	spin_lock_irq(&plo->lock);
> +	ret = plo->fastpath_reqs;
> +	spin_unlock_irq(&plo->lock);

Do we really need to take a lock here?

> +
> +	return ret;
> +}
> +
> +static void wait_fast_path_reqs(struct ploop_device *plo)
> +{
> +	wait_event(plo->fast_path_waitq, fast_path_reqs_count(plo) == 0);
> +}
> +
>  static void ploop_init_request(struct ploop_request *preq)
>  {
>  	preq->eng_state = PLOOP_E_ENTRY;
> @@ -656,6 +672,8 @@ DEFINE_BIO_CB(ploop_fast_end_io)
>  	    (test_bit(PLOOP_S_EXITING, &plo->state) ||
>  	     !list_empty(&plo->entry_queue)))
>  		wake_up_interruptible(&plo->waitq);
> +	if (plo->fast_path_disabled_count && !plo->fastpath_reqs)
> +		wake_up(&plo->fast_path_waitq);
>  	spin_unlock_irqrestore(&plo->lock, flags);
>
>  	BIO_ENDIO(plo->queue, orig, err);
> @@ -969,7 +987,8 @@ static void ploop_make_request(struct request_queue *q, struct bio *bio)
>
>  	/* No fast path, when maintenance is in progress.
>  	 * (PLOOP_S_TRACK was checked immediately above) */
> -	if (FAST_PATH_DISABLED(plo->maintenance_type))
> +	if (FAST_PATH_DISABLED(plo->maintenance_type) ||
> +	    plo->fast_path_disabled_count)
>  		goto queue;
>
>  	/* Attention state, always queue */
> @@ -3442,7 +3461,6 @@ static int ploop_replace_delta(struct ploop_device * plo, unsigned long arg)
>  	return err;
>  }
>
> -
>  void ploop_quiesce(struct ploop_device * plo)
>  {
>  	struct completion qcomp;
> @@ -3452,6 +3470,8 @@ void ploop_quiesce(struct ploop_device * plo)
>  		return;
>
>  	spin_lock_irq(&plo->lock);
> +	plo->fast_path_disabled_count++;
> +
>  	preq = ploop_alloc_request(plo, true);
>  	preq->req_rw = 0;
>  	preq->state = (1 << PLOOP_REQ_SYNC) | (1 << PLOOP_REQ_BARRIER);
> @@ -3469,6 +3489,7 @@ void ploop_quiesce(struct ploop_device * plo)
>  		wake_up_interruptible(&plo->waitq);
>  	spin_unlock_irq(&plo->lock);
>
> +	wait_fast_path_reqs(plo);
>  	wait_for_completion(&qcomp);
>  	plo->quiesce_comp = NULL;
>  }
> @@ -3476,6 +3497,10 @@ EXPORT_SYMBOL(ploop_quiesce);
>
>  void ploop_relax(struct ploop_device * plo)
>  {
> +	spin_lock_irq(&plo->lock);
> +	plo->fast_path_disabled_count--;
> +	spin_unlock_irq(&plo->lock);
> +
>  	if (!test_bit(PLOOP_S_RUNNING, &plo->state))
>  		return;
>
> @@ -5456,6 +5481,7 @@ static struct ploop_device *__ploop_dev_alloc(int index)
>  	init_waitqueue_head(&plo->req_waitq);
>  	init_waitqueue_head(&plo->freeze_waitq);
>  	init_waitqueue_head(&plo->event_waitq);
> +	init_waitqueue_head(&plo->fast_path_waitq);
>  	plo->tune = DEFAULT_PLOOP_TUNE;
>  	map_init(plo, &plo->map);
>  	track_init(plo);
> diff --git a/include/linux/ploop/ploop.h b/include/linux/ploop/ploop.h
> index 66591623f79d..8495ca2145fd 100644
> --- a/include/linux/ploop/ploop.h
> +++ b/include/linux/ploop/ploop.h
> @@ -407,6 +407,7 @@ struct ploop_device
>  	wait_queue_head_t	req_waitq;
>  	wait_queue_head_t	freeze_waitq;
>  	wait_queue_head_t	event_waitq;
> +	wait_queue_head_t	fast_path_waitq;
>
>  	struct ploop_map	map;
>  	struct ploop_map	*trans_map;
> @@ -473,6 +474,7 @@ struct ploop_device
>  	struct block_device *dm_crypt_bdev;
>
>  	unsigned long		locking_state; /* plo locked by userspace */
> +	unsigned int		fast_path_disabled_count;
>  };
>
>  enum
>
>
> .
>