[Devel] [PATCH vz9 v2] ploop: port and fix the standby mode feature.
Alexander Atanasov
alexander.atanasov at virtuozzo.com
Tue Nov 1 10:36:00 MSK 2022
On 31.10.22 21:27, Denis V. Lunev wrote:
> On 10/31/22 19:12, Alexander Atanasov wrote:
>> Initially in commit 80da9c2abac9 ("ploop: add a standby mode") a flag
>> on the block device's request queue was added to put the queue
>> into standby mode on EBUSY. Later on, the list with errors was extended
>> in commit e868f1e0b1a3 ("ploop: move to standby after -ENOTCONN too") and
>> in commit 4b1eb3f667eb ("ploop: kaio: Enter standby mode on EIO as
>> well").
>>
>> These were introduced to solve a problem on a specific device,
>> that will clear the standby flag at some point.
>> But the problem is that the clear counterpart was missing
>> for the rest of the devices, so once ploop gets one of the errors
>> it stops processing requests indefinitely.
>>
>> When porting the feature restrict the standby mode flag to work only
>> on devices that handle it. To acheve this introduce a static key
>> to enable the flag handling only when key is enabled - checks are
>> at the start and at the end of every request and we want to avoid
>> performance impact from these checks.
>>
>> On systems where we are in mixed mode, meaning we have both devices that
>> support the standby flag and devices that do not, a QUEUE_FLAG_STANDBY_EN
>> is introduced to indicate the standby support from the device.
>> Setting the QUEUE_FLAG_STANDBY_EN means the device promises to clear
>> QUEUE_FLAG_STANDBY flag when it recovers, so ploop can continue
>> processing
>> requests.
>>
>> The state of the flags is exported via /sys/block/*/queue/standby,
>> or at /sys/devices/virtual/block/*/queue depending on the device.
>> not supported - not enabled on the queue
>> on - queue is in standby mode, not processing requests
>> off - queue is processing requests
>>
>> https://jira.sw.ru/browse/PSBM-142759
>> Signed-off-by: Alexander Atanasov <alexander.atanasov at virtuozzo.com>
>> ---
>> block/blk-sysfs.c | 10 ++++++++++
>> drivers/md/dm-ploop-map.c | 36 +++++++++++++++++++++++++++++++++++-
>> drivers/md/dm-ploop-target.c | 12 +++++++++++-
>> drivers/md/dm-ploop.h | 1 +
>> include/linux/blkdev.h | 4 ++++
>> kernel/ve/ve.c | 9 +++++++++
>> 6 files changed, 70 insertions(+), 2 deletions(-)
>>
>> Cc: Denis V. Lunev <den at virtuozzo.com>
>> Cc: Kui Liu <Kui.Liu at acronis.com>
>> Cc: Alexey Kuznetsov <kuznet at acronis.com>
>>
>> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
>> index 4be6462c0008..c558c6c9a0ad 100644
>> --- a/block/blk-sysfs.c
>> +++ b/block/blk-sysfs.c
>> @@ -552,6 +552,14 @@ static ssize_t queue_dax_show(struct
>> request_queue *q, char *page)
>> return queue_var_show(blk_queue_dax(q), page);
>> }
>> +static ssize_t queue_standby_show(struct request_queue *q, char *page)
>> +{
>> + if (!blk_queue_standby_en(q))
>> + return sprintf(page, "not supported\n");
>> + return sprintf(page, "%s\n",
>> + blk_queue_standby(q) ? "on" : "off");
>> +}
>> +
>> #define QUEUE_RO_ENTRY(_prefix, _name) \
>> static struct queue_sysfs_entry _prefix##_entry = { \
>> .attr = { .name = _name, .mode = 0444 }, \
>> @@ -606,6 +614,7 @@ QUEUE_RO_ENTRY(queue_dax, "dax");
>> QUEUE_RW_ENTRY(queue_io_timeout, "io_timeout");
>> QUEUE_RW_ENTRY(queue_wb_lat, "wbt_lat_usec");
>> QUEUE_RO_ENTRY(queue_virt_boundary_mask, "virt_boundary_mask");
>> +QUEUE_RO_ENTRY(queue_standby, "standby");
>> #ifdef CONFIG_BLK_DEV_THROTTLING_LOW
>> QUEUE_RW_ENTRY(blk_throtl_sample_time, "throttle_sample_time");
>> @@ -667,6 +676,7 @@ static struct attribute *queue_attrs[] = {
>> &blk_throtl_sample_time_entry.attr,
>> #endif
>> &queue_virt_boundary_mask_entry.attr,
>> + &queue_standby_entry.attr,
>> NULL,
>> };
>> diff --git a/drivers/md/dm-ploop-map.c b/drivers/md/dm-ploop-map.c
>> index e8288e522e28..cb6f64814128 100644
>> --- a/drivers/md/dm-ploop-map.c
>> +++ b/drivers/md/dm-ploop-map.c
>> @@ -19,6 +19,7 @@
>> #include <uapi/linux/falloc.h>
>> #include "dm-ploop.h"
>> #include "dm-rq.h"
>> +#include "dm-core.h"
>> #define PREALLOC_SIZE (128ULL * 1024 * 1024)
>> @@ -28,6 +29,8 @@ static void ploop_prq_endio(struct pio *pio, void
>> *prq_ptr,
>> #define DM_MSG_PREFIX "ploop"
>> +extern struct static_key_false ploop_standby_check;
>> +
>> static unsigned int ploop_pio_nr_segs(struct pio *pio)
>> {
>> struct bvec_iter bi = {
>> @@ -1163,6 +1166,26 @@ static void ploop_queue_resubmit(struct pio *pio)
>> queue_work(ploop->wq, &ploop->worker);
>> }
>> +static void ploop_check_standby_mode(struct ploop *ploop, long res)
>> +{
>> + struct request_queue *q = ploop_blk_queue(ploop);
>> + int prev;
>> +
>> + if (!blk_queue_standby_en(q))
>> + return;
>> +
>> + /* move to standby if delta lease was stolen or mount is gone */
>> + if (res != -EBUSY && res != -ENOTCONN && res != -EIO)
>> + return;
>> +
>> + spin_lock_irq(&q->queue_lock);
>> + prev = blk_queue_flag_test_and_set(QUEUE_FLAG_STANDBY, q);
>> + spin_unlock_irq(&q->queue_lock);
>> +
>> + if (!prev)
>> + pr_info("ploop: switch into standby mode\n");
>
> some ploop identifier is needed here to determine device name
I'll add the device name here. dm-ploop logs without device name
everywhere - i'll check that too.
>
>
>> +}
>> +
>> static void ploop_data_rw_complete(struct pio *pio)
>> {
>> bool completed;
>> @@ -1175,6 +1198,8 @@ static void ploop_data_rw_complete(struct pio *pio)
>> ploop_queue_resubmit(pio);
>> return;
>> }
>> + if (static_branch_unlikely(&ploop_standby_check))
>> + ploop_check_standby_mode(pio->ploop, pio->ret);
>> pio->bi_status = errno_to_blk_status(pio->ret);
>> }
>> @@ -1815,8 +1840,11 @@ void do_ploop_fsync_work(struct work_struct *ws)
>> ret = vfs_fsync(file, 0);
>> while ((pio = ploop_pio_list_pop(&flush_pios)) != NULL) {
>> - if (unlikely(ret))
>> + if (unlikely(ret)) {
>> pio->bi_status = errno_to_blk_status(ret);
>> + if (static_branch_unlikely(&ploop_standby_check))
>> + ploop_check_standby_mode(ploop, ret);
>> + }
>> ploop_pio_endio(pio);
>> }
>> }
>> @@ -1869,6 +1897,12 @@ int ploop_clone_and_map(struct dm_target *ti,
>> struct request *rq,
>> struct ploop_rq *prq;
>> struct pio *pio;
>> +
>> + if (static_branch_unlikely(&ploop_standby_check)) {
>> + if (blk_queue_standby(ploop_blk_queue(ploop)))
>> + return DM_MAPIO_KILL;
>> + }
>> +
>> if (blk_rq_bytes(rq) && ploop_rq_valid(ploop, rq) < 0)
>> return DM_MAPIO_KILL;
>> diff --git a/drivers/md/dm-ploop-target.c b/drivers/md/dm-ploop-target.c
>> index 4f7dc36eee0c..29c89dc8ba99 100644
>> --- a/drivers/md/dm-ploop-target.c
>> +++ b/drivers/md/dm-ploop-target.c
>> @@ -21,17 +21,19 @@
>> #include <linux/uio.h>
>> #include <linux/error-injection.h>
>> #include "dm-ploop.h"
>> +#include "dm-core.h"
>> #define DM_MSG_PREFIX "ploop"
>> bool ignore_signature_disk_in_use = false; /* For development
>> purposes */
>> module_param(ignore_signature_disk_in_use, bool, 0444);
>> MODULE_PARM_DESC(ignore_signature_disk_in_use,
>> - "Does not check for SIGNATURE_DISK_IN_USE");
>> + "Does not check for SIGNATURE_DISK_IN_USE");
> please do not touch unrelated places
ok
>
>> static struct kmem_cache *prq_cache;
>> static struct kmem_cache *pio_cache;
>> struct kmem_cache *cow_cache;
>> +extern struct static_key_false ploop_standby_check;
>> static void ploop_aio_do_completion(struct pio *pio)
>> {
>> @@ -406,6 +408,14 @@ static int ploop_ctr(struct dm_target *ti,
>> unsigned int argc, char **argv)
>> ti->private = ploop;
>> ploop->ti = ti;
>> + if (blk_queue_standby_en(ploop_blk_queue(ploop))) {
>> + blk_queue_flag_clear(QUEUE_FLAG_STANDBY,
>> ploop_blk_queue(ploop));
>> + if (static_branch_unlikely(&ploop_standby_check)) {
> this point should NOT be reached. If it is reached - SCST has violated
> the protocol at my opinion
If any other device wants to user the flag and it sets it before _ctr it
will catch it and enable the key. I can change it to a WARN_ON if _en is
set but key is not.
[snip]
--
Regards,
Alexander Atanasov
More information about the Devel
mailing list