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

[Kirill Tkhai]

On 05.03.2020 19:31, Konstantin Khorenko wrote:
> 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?

Yes, since fast path reqs may be queued from any processor.

>> +
>> +    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
>>
>>
>> .
>>