Re: [PATCH v3 3/3] xfs: extract xfs_da_buf_copy() helper function

From: Christoph Hellwig
Date: Mon Dec 04 2023 - 02:26:07 EST


> - /* copy leaf to new buffer, update identifiers */
> - xfs_trans_buf_set_type(args->trans, bp2, XFS_BLFT_ATTR_LEAF_BUF);
> - bp2->b_ops = bp1->b_ops;
> - memcpy(bp2->b_addr, bp1->b_addr, args->geo->blksize);
> - if (xfs_has_crc(mp)) {
> - struct xfs_da3_blkinfo *hdr3 = bp2->b_addr;
> - hdr3->blkno = cpu_to_be64(xfs_buf_daddr(bp2));
> - }
> + /*
> + * copy leaf to new buffer and log it.
> + */

Nit: The first word in a sentence should be capitalized.
Alternativalely just keep the old comment format that doesn't
pretence to be a sentence :)

> + /*
> + * Now we could drop the child buffer.
> + */

s/could/can/ ?

> +/*
> + * Copy src directory/xattribute leaf/node buffer to the dst.
> + * If xfs enables crc(IOW, xfs' on-disk format is v5), we have to
> + * make sure that the block specific identifiers are kept intact.
> + */

I'd reword this a bit:

* Copy src directory/attr leaf/node buffer to the dst.
* For v5 file systems make sure the right blkno is stamped in.

Also maybe move this function further up in the file? Even for
non-static functions it's kinda nice if they are implemented before
use to ease the reading flow.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@xxxxxx>