On 2019/08/06 10:06, Dmitry Fomichev wrote:
> There are several places in reclaim code where errors are not
> propagated to the main function, dmz_reclaim(). This function
> is responsible for unlocking zones that might be still locked
> at the end of any failed reclaim iterations. As the result,
> some device zones may be left permanently locked for reclaim,
> degrading target's capability to reclaim zones.
>
> This patch fixes these issues as follows -
>
> Make sure that dmz_reclaim_buf(), dmz_reclaim_seq_data() and
> dmz_reclaim_rnd_data() return error codes to the caller.
>
> dmz_reclaim() function is renamed to dmz_do_reclaim() to avoid
> clashing with "struct dmz_reclaim" and is modified to return the
> error to the caller.
>
> dmz_get_zone_for_reclaim() now returns an error instead of NULL
> pointer and reclaim code checks for that error.
>
> Error logging/debug messages are added where necessary.
>
> Fixes: 3b1a94c88b79 ("dm zoned: drive-managed zoned block device target")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@xxxxxxx>
> ---
> drivers/md/dm-zoned-metadata.c | 4 ++--
> drivers/md/dm-zoned-reclaim.c | 28 +++++++++++++++++++---------
> 2 files changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/md/dm-zoned-metadata.c b/drivers/md/dm-zoned-metadata.c
> index ded4984d18c9..6b7fbbd735ef 100644
> --- a/drivers/md/dm-zoned-metadata.c
> +++ b/drivers/md/dm-zoned-metadata.c
> @@ -1543,7 +1543,7 @@ static struct dm_zone *dmz_get_rnd_zone_for_reclaim(struct dmz_metadata *zmd)
> struct dm_zone *zone;
>
> if (list_empty(&zmd->map_rnd_list))
> - return NULL;
> + return ERR_PTR(-EBUSY);
>
> list_for_each_entry(zone, &zmd->map_rnd_list, link) {
> if (dmz_is_buf(zone))
> @@ -1554,7 +1554,7 @@ static struct dm_zone *dmz_get_rnd_zone_for_reclaim(struct dmz_metadata *zmd)
> return dzone;
> }
>
> - return NULL;
> + return ERR_PTR(-EBUSY);
> }
>
> /*
> diff --git a/drivers/md/dm-zoned-reclaim.c b/drivers/md/dm-zoned-reclaim.c
> index 260e3598199e..26e34493a2db 100644
> --- a/drivers/md/dm-zoned-reclaim.c
> +++ b/drivers/md/dm-zoned-reclaim.c
> @@ -216,7 +216,7 @@ static int dmz_reclaim_buf(struct dmz_reclaim *zrc, struct dm_zone *dzone)
>
> dmz_unlock_flush(zmd);
>
> - return 0;
> + return ret;
> }
>
> /*
> @@ -260,7 +260,7 @@ static int dmz_reclaim_seq_data(struct dmz_reclaim *zrc, struct dm_zone *dzone)
>
> dmz_unlock_flush(zmd);
>
> - return 0;
> + return ret;
> }
>
> /*
> @@ -313,7 +313,7 @@ static int dmz_reclaim_rnd_data(struct dmz_reclaim *zrc, struct dm_zone *dzone)
>
> dmz_unlock_flush(zmd);
>
> - return 0;
> + return ret;
> }
>
> /*
> @@ -335,7 +335,7 @@ static void dmz_reclaim_empty(struct dmz_reclaim *zrc, struct dm_zone *dzone)
> /*
> * Find a candidate zone for reclaim and process it.
> */
> -static void dmz_reclaim(struct dmz_reclaim *zrc)
> +static int dmz_do_reclaim(struct dmz_reclaim *zrc)
> {
> struct dmz_metadata *zmd = zrc->metadata;
> struct dm_zone *dzone;
> @@ -345,8 +345,8 @@ static void dmz_reclaim(struct dmz_reclaim *zrc)
>
> /* Get a data zone */
> dzone = dmz_get_zone_for_reclaim(zmd);
> - if (!dzone)
> - return;
> + if (IS_ERR(dzone))
> + return PTR_ERR(dzone);
>
> start = jiffies;
>
> @@ -392,13 +392,20 @@ static void dmz_reclaim(struct dmz_reclaim *zrc)
> out:
> if (ret) {
> dmz_unlock_zone_reclaim(dzone);
> - return;
> + return ret;
> }
>
> - (void) dmz_flush_metadata(zrc->metadata);
> + ret = dmz_flush_metadata(zrc->metadata);
> + if (ret) {
> + dmz_dev_debug(zrc->dev,
> + "Metadata flush for zone %u failed, err %d\n",
> + dmz_id(zmd, rzone), ret);
> + return ret;
> + }
>
> dmz_dev_debug(zrc->dev, "Reclaimed zone %u in %u ms",
> dmz_id(zmd, rzone), jiffies_to_msecs(jiffies - start));
> + return 0;
> }
>
> /*
> @@ -443,6 +450,7 @@ static void dmz_reclaim_work(struct work_struct *work)
> struct dmz_metadata *zmd = zrc->metadata;
> unsigned int nr_rnd, nr_unmap_rnd;
> unsigned int p_unmap_rnd;
> + int ret;
>
> if (!dmz_should_reclaim(zrc)) {
> mod_delayed_work(zrc->wq, &zrc->work, DMZ_IDLE_PERIOD);
> @@ -472,7 +480,9 @@ static void dmz_reclaim_work(struct work_struct *work)
> (dmz_target_idle(zrc) ? "Idle" : "Busy"),
> p_unmap_rnd, nr_unmap_rnd, nr_rnd);
>
> - dmz_reclaim(zrc);
> + ret = dmz_do_reclaim(zrc);
> + if (ret)
> + dmz_dev_debug(zrc->dev, "Reclaim error %d\n", ret);
>
> dmz_schedule_reclaim(zrc);
> }
>
Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxx>
--
Damien Le Moal
Western Digital Research
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
