Re: [PATCH v2 2/3] dm zoned: do not reclaim a random zone into another random zone

[illofspeed <illofspeed@gmail.com>]

Damien Le Moal wrote:
>> +	 * Without a free sequential zone, moving a cache zone into a random
>> +	 * zone of the zoned device still frees cache space. Moving a random
>> +	 * zone into another random zone gains nothing, and when the target
>> +	 * is idle it repeats forever. Only fall back for cache zones.
>
> "fall back" to what? This comment is not clear.

To a random destination zone: the switch from DMZ_ALLOC_SEQ to
DMZ_ALLOC_RND just below. v3 rewords it:

	/*
	 * If no free sequential zone is available, a cache zone can still be
	 * moved into a free random zone of the zoned device, which frees cache
	 * space. Do not do this for a random zone: moving it into another
	 * random zone frees nothing and, on an idle target, repeats forever.
	 */

>> +	if (!szone && alloc_flags == DMZ_ALLOC_SEQ && dmz_nr_cache_zones(zmd) &&
>
> Long line. Please split after "DMZ_ALLOC_SEQ &&".
> Also, is the test for dmz_nr_cache_zones() needed? dmz_is_cache(dzone) will
> never be true if there are no cache zones.

You are right: DMZ_CACHE is only set on zones of the regular device, so
the test is redundant. Without it the condition fits on one line:

	if (!szone && alloc_flags == DMZ_ALLOC_SEQ && dmz_is_cache(dzone)) {

With this change the reproducer from the cover letter still shows 0 MB/s
on the idle target. I will send v3 once the Signed-off-by question on
1/3 is settled; patches 1 and 3 stay unchanged.

illofspeed