On Tue, Nov 28, 2023 at 06:20:19AM -0800, Christoph Hellwig wrote: > On Fri, Nov 24, 2023 at 03:53:41PM -0800, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@xxxxxxxxxx> > > > > There are a couple of conditions that userspace can set to force repairs > > of metadata. These really belong in the repair code and not open-coded > > into the check code, so refactor them into a helper. > > Just ramblings from someone who is trying to get into the scrub and > repair code: > > I find this code organization where the check helpers are in foo.c, > repair helpers in foo_repair.c and then both are used in scrub.c > to fill out ops really annoying to follow. My normal taste would > expect a single file that has all the methods, and which then > registers the ops vector. But it's probably too late for that now.. Not really, in theory I could respin the whole series to move FOO_repair.c into FOO.c surrounded by a giant #ifdef CONFIG_XFS_ONLINE_REPAIR block; and change agheader_repair.c. OTOH I thought it was cleaner to elide the repair code via Makefiles instead of preprocessor directives that we could get lost in. Longer term I've struggled with whether or not (for example) the alloc.c/alloc_repair.c declarations should go in alloc.h. That's cleaner IMHO but explodes the number of files for usually not that much gain. --D
