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

[David Sterba <dsterba@suse.cz>]

On Wed, Apr 16, 2025 at 08:57:40AM +0300, Andy Shevchenko wrote:
> On Wed, Apr 16, 2025 at 2:57 AM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> > 在 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.
> 
> Ah, if it's that simple, of course, round_*() is okay to go.
> My only worries are about explicit castings to "fix" such a warning.

Both ALIGN and round_* seem to be fine with different types, there are
the tricks with masking lower bits and the alignment is explicitly cast
to the target type. Most offten we have u64 as target type and u32 as
the alignment.