On Thu, Jun 28, 2018 at 09:37:20AM +1000, Dave Chinner wrote:
> On Wed, Jun 27, 2018 at 09:44:53AM -0700, Allison Henderson wrote:
> > On 06/26/2018 07:19 PM, Dave Chinner wrote:
> > >On Sun, Jun 24, 2018 at 12:23:54PM -0700, Darrick J. Wong wrote:
> > >>From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > >>
> > >>Regenerate the AGF and AGFL from the rmap data.
> > >>
> > >>Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > >
> > >[...]
>
> > >>+ /* Record all the OWN_AG blocks. */
> > >>+ if (rec->rm_owner == XFS_RMAP_OWN_AG) {
> > >>+ fsb = XFS_AGB_TO_FSB(cur->bc_mp, cur->bc_private.a.agno,
> > >>+ rec->rm_startblock);
> > >>+ error = xfs_repair_collect_btree_extent(ra->sc,
> > >>+ ra->freesp_list, fsb, rec->rm_blockcount);
> > >>+ if (error)
> > >>+ return error;
> > >>+ }
> > >>+
> > >>+ return xfs_repair_collect_btree_cur_blocks(ra->sc, cur,
> > >>+ xfs_repair_collect_btree_cur_blocks_in_extent_list,
> > >
> > >Urk. The function name lengths is getting out of hand. I'm very
> > >tempted to suggest we should shorten the namespace of all this
> > >like s/xfs_repair_/xr_/ and s/xfs_scrub_/xs_/, etc just to make them
> > >shorter and easier to read.
> > >
> > >Oh, wait, did I say that out loud? :P
> > >
> > >Something to think about, anyway.
> > >
> > Well they are sort of long, but TBH I think i still kind of
> > appreciate the extra verbiage. I have seen other projects do things
> > like adopt a sort of 3 or 4 letter abbreviation (like maybe xfs_scrb
> > or xfs_repr). Helps to cut down on the verbosity while still not
> > loosing too much of what it is supposed to mean. Just another idea
> > to consider. :-)
>
> We've got that in places, too, like "xlog_" prefixes for all the log
> code, so that's not an unreasonable thing to suggest. After all, in
> many cases we're talking about a tradeoff between readabilty and the
> amount of typing necessary.
I propose(d on IRC) to shorten the prefixes to xrep_ and xchk_.
I'll also take a look at condensing the non-prefix parts of the names.
Agreed that they're too long now.
> However, IMO, function names so long they need a line of their own
> indicates we have a structural problem in our code, not a
> readability problem. We should not need names that long to document
> what the function does - it should be obvious from the context, the
> abstraction that is being used and a short name....
>
> e.g. how many of these different "collect extent" operations could
> be abstracted into a common extent list structure and generic
> callbacks? It seems there's a lot of similarity in them, and we're
> really only differentiating them by adding more namespace and
> context specific information into the structure and function names.
I'd /really/ like to convert this into a proper incore bitmap and then
turn the operations into proper bitmasking operations, since
collect_btree_extents is merely setting ranges of bits in two bitmaps
and subtract_extents takes the union of them both.
--D
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
