Re: [PATCH 2/2] btrfs: make btrfs_truncate_block() zero folio range for certain subpage corner cases

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

在 2025/4/16 03:51, Andy Shevchenko 写道:
> On Tue, Apr 15, 2025 at 9:18 PM David Sterba <dsterba@suse.cz> wrote:
>> On Mon, Apr 14, 2025 at 01:40:11PM +0300, Andy Shevchenko wrote:
>>> On Mon, Apr 14, 2025 at 4:20 AM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>>>> 在 2025/4/13 04:05, Andy Shevchenko 写道:
>>>>> Fri, Apr 11, 2025 at 02:44:01PM +0930, Qu Wenruo kirjoitti:
> 
> [...]
> 
>>>>>> +    block_start = round_down(clamp_start, block_size);
>>>>>> +    block_end = round_up(clamp_end + 1, block_size) - 1;
>>>>>
>>>>> LKP rightfully complains, I believe you want to use ALIGN*() macros instead.
>>>>
>>>> Personally speaking I really want to explicitly show whether it's
>>>> rounding up or down.
>>>>
>>>> And unfortunately the ALIGN() itself doesn't show that (meanwhile the
>>>> ALIGN_DOWN() is pretty fine).
>>>>
>>>> Can I just do a forced conversion on the @blocksize to fix the warning?
>>>
>>> ALIGN*() are for pointers, the round_*() are for integers. So, please
>>> use ALIGN*().
>>
>> clamp_start and blocksize are integers and there's a lot of use of ALIGN
>> with integers too. There's no documentation saying it should be used for
>> pointers, I can see PTR_ALIGN that does the explicit cast to unsigned
>> logn and then passes it to ALIGN (as integer).
> 
> Yes, because the unsigned long is natural holder for the addresses and
> due to some APIs use it instead of pointers (for whatever reasons) the
> PTR_ALIGN() does that. But you see the difference? round_*() expect
> _the same_ types of the arguments, while ALIGN*() do not. That is what
> makes it so.
> 
>> Historically in the btrfs code the use of ALIGN and round_* is basically
>> 50/50 so we don't have a consistent style, although we'd like to. As the
>> round_up and round_down are clear I'd rather keep using them in new
>> code.
> 
> And how do you suggest avoiding the warning, please?

By fixing the typo, @block_size -> @blocksize.

The original warning is not about the type difference, but that 
@block_size is a function pointer.

We have tons of round_down()/round_up() usage inside btrfs, with 
different types.

E.g. btrfs_check_data_free_space(), which is calling 
round_down()/round_up() against u64 and u32, and do you got any warnings?

Thanks,
Qu