On 2018年04月12日 01:15, Goffredo Baroncelli wrote:
> On 04/11/2018 02:32 AM, Qu Wenruo wrote:
> [...]
>>>>> so to get rid  of generate_tab_indent and indent_str
>>>> And we need to call such functions in each helper macros, with
>>>> duplicated codes.
>>> Please look at the asm generated: even if the "source generated" by the 
>>> expansion of the macro is bigger, the binary code is smaller.
>>> E.g. the code below 
>> No, I don't mean asm code, but C code.
> May be that there is some misunderstanding: my code is about 20loc, your one 
> is about 50loc... I am missing something ?

Loc is not the core factor, it's duplicated code.

> [...]
>>>> When passing random stream to dump-super, such reason will make output
>>>> quite nasty.
>>>> So just INVALID to info the user that some of the members don't look
>>>> valid is good enough, as the tool is only to help guys who are going to
>>>> manually patching superblocks.
>>> I think that we should increase the possible target also to who want to 
>>> make some debugging :-)
>> There are several problems here to output the condition
>> 1) Loose condition
>> for basic alignment check it may looks good to output the condition, but
>> the fact is, the condition is not 100% correct for 64K pages system.
>> So when output IS_ALIGN(value, SZ_4K), it's not 100% correct.
> I don't understand your statement: does the alignment is the same for all the 
> system ?.

As already mentioned by Nikolay, no.

> If not, this means that a filesystem created on a x86 might not work on a 
> PPC64 (which IIRC is a 64k page hardware) ?

Yep, btrfs created on x86 will not work on any 64K page size system, and
vice verse.

IBM guy is working on such sub-page size support to allow PPC64 to mount
4K sector size btrfs, but I didn't see their update for a long time.

>> 2) Too long condition for some case.
>> !(is_power_of_2(value) && value >= SZ_4K && value <= SZ_64K)
>> This seems pretty strange in fact.
> I agree that this is quite verbose... however if this is the constraint, why 
> not print it

Because when it's invalid, the constraint doesn't help much for
developer/user to patch the value.


>> 3) C macro usage
>>    Just a minor problem, macro usage such as SZ_4K/SZ_64K may not looks
>>    good for new developers.
> I find it quite clear and intuitive even if this is the first time that I 
> notice these macros
> [...]
>> Thanks,
>> Qu
> BR
> G.Baroncelli
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to