On Tue, Oct 17, 2023 at 08:54:08AM -0700, Darrick J. Wong wrote: > From: Dave Chinner <dchinner@xxxxxxxxxx> > > Consolidate the arguments passed around the rt allocator into a > struct xfs_rtalloc_arg similar to how the btree allocator arguments > are consolidated in a struct xfs_alloc_arg.... Overall this looks good to me, but a few cosmetic comments: > + struct xfs_rtalloc_args *args, > + xfs_fileoff_t block, /* block number in bitmap or summary */ > + int issum, /* is summary not bitmap */ > + struct xfs_buf **bpp) /* output: buffer for the block */ we should either also document the new args argument, or drop all the other argument comments like we've done in many places. If we want to keep them it would be good to do the trivial reformatting so that they don't extend over the 80 character readability limit. > + struct xfs_mount *mp = args->mount; mount is a bit of an odd name for the member. We usuall calls this mp in most structures like our normal variable name (with tp->t_mountp as one notable odd exception). > + error = xfs_trans_read_buf(mp, args->trans, mp->m_ddev_targp, > XFS_FSB_TO_DADDR(mp, map.br_startblock), > mp->m_bsize, 0, &bp, &xfs_rtbuf_ops); ->trans has some precedence in the dir/attr code, but I think tp would still be more logical.
