Received-SPF: pass (google.com: domain of linux-kernel+bounces-33176-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Naohiro Aota <Naohiro.Aota@wdc.com>
To: Johannes Thumshirn <Johannes.Thumshirn@wdc.com>
CC: Josef Bacik <josef@toxicpanda.com>, David Sterba <dsterba@suse.com>,
	"linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Damien Le Moal
	<dlemoal@kernel.org>
Subject: Re: Re: [PATCH 2/2] btrfs: zoned: wake up cleaner sooner if needed
Thread-Topic: Re: [PATCH 2/2] btrfs: zoned: wake up cleaner sooner if needed
Thread-Index: AQHaTSDvOd+kWBN8wkSOr2ka96SkX7DlwQgAgAACYgCAACQAgA==
Date: Mon, 22 Jan 2024 14:39:44 +0000
Message-ID: <k7fiky6xm4hshkr5q2xukfjndcseiesfanlpc4oozztvuyclbw@ftrhgjeicsfs>
References: <20240122-reclaim-fix-v1-0-761234a6d005@wdc.com>
 <20240122-reclaim-fix-v1-2-761234a6d005@wdc.com>
 <x6bi4u2u65q37tde3s357lzhce4wglpobfgp7qgzhun4iadg3m@2pewiu6xuts4>
 <f5d54836-5edf-4cd0-88c8-f2d474368ea9@wdc.com>
In-Reply-To: <f5d54836-5edf-4cd0-88c8-f2d474368ea9@wdc.com>
Accept-Language: ja-JP, en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?5oGYuZp/En91ODKgwEHIp9U1dJ/FLaJfAV28KiB97b6osGsLkZRFLn1etyqe?=
 =?us-ascii?Q?LxEDfNFLQ3sj8GEAk7JPBu917OshCQoq6iV9yEeg+k4QvrXjtt4c5BOMXZgJ?=
 =?us-ascii?Q?CkEss4E9mrZ0uuBC6HLBLH+C7f04FLgubO9O1hWgc2XzrehTzCzX5YYDlxMe?=
 =?us-ascii?Q?PjJ2p30fASZGQfg+6UOCjHIIBdcXyfGPBiSRGtn/da8T64PmsQhyVpO1zRhI?=
 =?us-ascii?Q?ZyE8NNIPlA6IW4AwFtHPAVyHgmxUpPuuKt+6LWAp9T9CyZc0yzF9ceg/zlTM?=
 =?us-ascii?Q?lk25y1IZ7OMlNmI1LMpGzdeEi98XcM16ml0KXQhXq38mjXoC/k/QPZMKYiKB?=
 =?us-ascii?Q?RWQK6GTMzZg34zoL0C9/1c152px8KPLiIqfIJeseLR9NNv0xV8TWQ/+bIYb3?=
 =?us-ascii?Q?SHXCetCIpQ1TIBfj7/rGbnU6dcyjLdSUNKFAAtJaxjlRsDDdMDHKfi140Uyv?=
 =?us-ascii?Q?VTDGoLzbFSz83m3HT50h/WwT11RYaFYhUhYDARR5Igitsr5/InDbKrqWkxF0?=
 =?us-ascii?Q?DebYIPAHhHcIVDKP+CbhnrQtG0nV2CHRSek4rnJYu0KByHyE9lSZyEITMn/+?=
 =?us-ascii?Q?jV2iMP5TRTBEx2CEkfIjmPBdnITkvn/fYrD0nFu+wwC9jQnSEnU8MbZHVSFl?=
 =?us-ascii?Q?fXxyH3LTlipADCRDtRUJsc+ap90t3hW1ebxurVPii7spINmbXWcQHS+k6Z8+?=
 =?us-ascii?Q?ZonxEEBJyyD7Jk0TqI+/Eenp0qIi8hCBZhweRjrQM9I2KpWZiXZAhA9VjF+w?=
 =?us-ascii?Q?5d9/Ap7MI3NW+nulIjaOCOVGFuWNF/t6av0Q1u0Z3GlhK27qcpjDmNiVv7z4?=
 =?us-ascii?Q?Ksqy/H86cTGJYIwBD27lmvVfMVTkqEmLpROzy0cDOCJjfPe171wk9zXubgN8?=
 =?us-ascii?Q?AuaNJq7DFZBe5vIJXU6sJ+XX7auOC3u3du7CBuMXVFIoqwze4XjUmzvmI01Y?=
 =?us-ascii?Q?W2uqDSvIL4UkDIoauBd15qzdrZXSM8tRM/Tl79EAJSI4HXJ36ruhLMitXwSH?=
 =?us-ascii?Q?Jcn7HV6ELdEwis1n2yAdIBxhAqJK5g0VbnHe86FClLauqpstd3Gq0DdKhfy1?=
 =?us-ascii?Q?QQi1TLaLOeksWjvTKTIU/2BVRVyvZj4ru9jSgbjExU1sVhi70kCfXcdIt/Si?=
 =?us-ascii?Q?0RYPZdthnHVXRpGOSxf8R1EEW4m6Aeen6o6jKUrAjVYwoJRJFImzsBAcKHTK?=
 =?us-ascii?Q?L5tMcp+KQ4K8TI42Gxse0IllTisFJ5KddEOf1Ep3RYIuu4cYf6dliCy0j+6h?=
 =?us-ascii?Q?sKyKjicf2bB/IM65/G3Vjbvta8+mYc6ceM7vRQc45JFabs1aN/bPhlfLJWFu?=
 =?us-ascii?Q?yk2HFm/h2IDWUE4sU6oA8qlVxN1M6ZoGBVzg7Sa/9dFqpUTj8U1JaE3aWBay?=
 =?us-ascii?Q?mGLsstt6h9wnIeEDLkZyWcLeENQ8aLPN2QNscRPkkJOLGzfmVYPC9/LYRvFw?=
 =?us-ascii?Q?4AEw5FMHsnPQuMOKWYSHk8DLdZYVgMgIb/V1c/sVr0a00uR+XUNMz7QayHQP?=
 =?us-ascii?Q?9hJhCgRfyJIrB4h8InKR637VcOdzVT/tHAHmFdbO5PLUnjwEujAAcw7+bc/8?=
 =?us-ascii?Q?jEJSah9gvD5LeVz+maTH3r+OMypFVPhB1TDIWBc6+HqvAPcYYqG/wE086XhR?=
 =?us-ascii?Q?dg=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <AE37F9CF087A494D845CE5C5C7DCCDFC@namprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0:
	gFdW/8O9jYwjMd6GIuK36rzkj7DuVt/Cu0xHffwIZQfJmaudJKneuelun2RDiNg8e6b/MZACAMOkbtN31WNov8EEYsUH/fQIyzy7pADE5IQUCRws6PWwNMbXhTaVJDZigStVo02R99vGADDCovKvRQ29Uj7/Ut7qmo1Tw0FDkxbvo5pZRS+5aQatc3cKoTLkYL/QORJBTiqyFSU3Neh/TWGPKSlJ/tqn/s3ahB4BkC7Whkw7Q1En71ewbbeby2/yECDb7jPM76zV/e0VuNMMP+HU6Xm0RYZijzT59HBJ1lzSXTSK64qMmzeqIRcqqAnGAulTeZIxVkIrzzHkxfUPqHF2uphqIKPvEx2VNkguF+EnvZ9HGs0UX1L1kEvsOd9PCOqJfFcpxhxp/vuy8SCTVRQX1sUKqJHH7LImYiF/hBllEmAZBvEq45CM2tDxAXc57/MWLFiHD0isH0PfJztx5FVC8xTXYVZr+xQePOhZjnub6P6AvZuus0jF9H0mmE/WQleoWYBxkr+vt5VCuyt6U31aCCxcppgr534XpJBoZehJXCpzsuyCUb7pr8SqIBHvmvQV/jpew8TInESe7PFx7qL2qwxxQ/8NZ/baddXtbxdQsZYduWwytXWCAMNPiDHe
X-OriginatorOrg: wdc.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR04MB7776.namprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 60463a25-2be1-43db-1d88-08dc1b57f96e
X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Jan 2024 14:39:44.0375
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: z16C9ZBqcrJtk+V7R3aHfxoqlpQRN0yFm21DtWKaFwJfQ6nqwT3xi45H8TVh6lb9HeVFDWKjr0N/oz/SYNe5eQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR04MB6274

On Mon, Jan 22, 2024 at 12:30:52PM +0000, Johannes Thumshirn wrote:
> On 22.01.24 13:22, Naohiro Aota wrote:
> > On Mon, Jan 22, 2024 at 02:51:04AM -0800, Johannes Thumshirn wrote:
> >> On very fast but small devices, waiting for a transaction commit can b=
e
> >> too long of a wait in order to wake up the cleaner kthread to remove u=
nused
> >> and reclaimable block-groups.
> >>
> >> Check every time we're adding back free space to a block group, if we =
need
> >> to activate the cleaner kthread.
> >>
> >> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> >> ---
> >>   fs/btrfs/free-space-cache.c | 6 ++++++
> >>   1 file changed, 6 insertions(+)
> >>
> >> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> >> index d372c7ce0e6b..2d98b9ca0e83 100644
> >> --- a/fs/btrfs/free-space-cache.c
> >> +++ b/fs/btrfs/free-space-cache.c
> >> @@ -30,6 +30,7 @@
> >>   #include "file-item.h"
> >>   #include "file.h"
> >>   #include "super.h"
> >> +#include "zoned.h"
> >>  =20
> >>   #define BITS_PER_BITMAP		(PAGE_SIZE * 8UL)
> >>   #define MAX_CACHE_BYTES_PER_GIG	SZ_64K
> >> @@ -2694,6 +2695,7 @@ int __btrfs_add_free_space(struct btrfs_block_gr=
oup *block_group,
> >>   static int __btrfs_add_free_space_zoned(struct btrfs_block_group *bl=
ock_group,
> >>   					u64 bytenr, u64 size, bool used)
> >>   {
> >> +	struct btrfs_fs_info *fs_info =3D block_group->fs_info;
> >>   	struct btrfs_space_info *sinfo =3D block_group->space_info;
> >>   	struct btrfs_free_space_ctl *ctl =3D block_group->free_space_ctl;
> >>   	u64 offset =3D bytenr - block_group->start;
> >> @@ -2745,6 +2747,10 @@ static int __btrfs_add_free_space_zoned(struct =
btrfs_block_group *block_group,
> >>   		btrfs_mark_bg_to_reclaim(block_group);
> >>   	}
> >>  =20
> >> +	if (btrfs_zoned_should_reclaim(fs_info) &&
> >> +	    !test_bit(BTRFS_FS_CLEANER_RUNNING, &fs_info->flags))
> >> +		wake_up_process(fs_info->cleaner_kthread);
> >> +
> >=20
> > Isn't it too costly to call btrfs_zoned_should_reclaim() every time
> > something updated? Can we wake up it in btrfs_mark_bg_to_reclaim and
> > btrfs_mark_bg_unused ?
>=20
> Hmm yes, I've thought of adding a list_count() for the reclaim and=20
> unused lists, and only calling into should_reclaim if these lists have=20
> entries. Or even better !list_is_singular().

That sounds reasonable.

> >=20
> > Also, looking into btrfs_zoned_should_reclaim(), it sums device->bytes_=
used
> > for each fs_devices->devices. And, device->bytes_used is set at
> > create_chunk() or at btrfs_remove_chunk(). Isn't it feasible to do the
> > calculation only there?
>=20
> Oh sh*t! Right we should check bytes_used from all space_infos in=20
> btrfs_zoned_should_reclaim() and compare that to the disk total bytes.

You mean device->bytes_used? space_info->bytes_used does not count free
space and zone_unusable in BGs, so using that changes the behavior. Even,
it won't kick the thread if there are many zone_unusable but small used
space.

> =