Re: [PATCH 5/5] btrfs: zoned: make auto-reclaim less aggressive

From: Pankaj Raghav <p.raghav@samsung.com>
To: Johannes Thumshirn <Johannes.Thumshirn@wdc.com>,
	David Sterba <dsterba@suse.com>,
	Naohiro Aota <Naohiro.Aota@wdc.com>
Cc: Josef Bacik <josef@toxicpanda.com>,
	"linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	"Javier Gonzalez" <javier.gonz@samsung.com>
Subject: Re: [PATCH 5/5] btrfs: zoned: make auto-reclaim less aggressive
Date: Wed, 23 Mar 2022 20:37:16 +0100	[thread overview]
Message-ID: <ebfd2c53-583d-749c-764a-7d9f9b86ae54@samsung.com> (raw)
In-Reply-To: <PH0PR04MB7416D06ED74EE14B13C1DB3E9B189@PH0PR04MB7416.namprd04.prod.outlook.com>

On 2022-03-23 12:52, Johannes Thumshirn wrote:

>> This looks good. And the test btrfs/237 is failing, as it should be
>> because of the change in reclaim condition. Are you planning to update
>> this test in fstests later?
> 
> Yes, once I have an idea how to do. Probably just fill the FS until
> ~75% of the drive is filled and then use the original logic.
> 
Perfect.
>> I still have one more question: shouldn't we use the usable disk bytes
>> (zcap * nr_zones) instead of disk_total_bytes (zsze * nr_zones) to
>> calculate the `total` variable? The `used` value is a part of the usable
>> disk space so I feel it makes more sense to calculate the `factor` with
>> the usable disk bytes instead of the disk_total_bytes.
>>
> 
> disk_total_bytes comes from the value the underlying device driver set
> for the gendisk's capacity via set_capacity().
Yes, I understand that part. My comment was more about how pedantic we
need to be for reclaim as set capacity via the device driver will
include the unusable holes (lbas between zcap and zsze).

For e.g., zns drive with 96M zone capacity and 128M zone size with 10
zones will have a total disk capacity of 1280M but the usuable capacity
is 960M.

Let us say the `used` value is 128M, then the `factor` value with the
current approach is 128 / 1280 = 10%.

But if we use the usable capacity of a zns drive, then the `factor`
value will be 128 / 960 = 13.3%.

This might not be problem for lower value of `used` but my concern is
when the drive is nearing its capacity.

Let's take the same example where the `used` value is now 900M. Then the
factor with the current approach is 70% (900 / 1280), still below the
default 75 for bg_reclaim_threshold but when used with the usable
capacity, it is 93% (900 / 960).

So essentially we are postponing the reclaim assuming we have enough
room left but in reality it is not.

I still don't have a complete understanding of the full stack with btrfs
on zoned devices so please correct me if I am wrong.

-- 
Regards,
Pankaj