Re: [PATCH v3.1 3/7] btrfs: relocation: Delay reloc tree deletion after merge_reloc_roots()

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1224 bytes --]



On 2019/1/15 上午1:21, David Sterba wrote:
> On Mon, Jan 07, 2019 at 01:41:47PM +0800, Qu Wenruo wrote:
>> +/*
>> + * Helper structure to keep record of a file tree whose reloc
>> + * root needs to be cleaned up.
>> + *
>> + * Since reloc_control is used less frequently than btrfs_root, this should
>> + * prevent us to add another structure in btrfs_root.
>> + */
>> +struct dirty_source_root {
>> +	struct rb_node node;
>> +
>> +	/* Root must be file tree */
> 
> The naming starts here and is used in the following patches too: the
> naming is IMO confusing, here it's still ok but variables like
> 'file_bytenr' or 'file_parent' sound weird in context of relocation.

'file_bytenr' and 'file_parent' are introduced in the 5th patch.

My main concern of using traditional naming like 'source/destination'
is, for swapped blocks, source/destination needs extra context to
determine which is which.

So I tend to use naming which shows directly to which tree the block
belongs.

> 
> The file tree is the subvolume right, or would it be confusing in
> another way?
> 
So did you mean use something like 'subv_bytenr' and 'subv_generation'?

I'm OK for this naming.

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]