On Tue, Jan 16, 2024 at 08:40:23AM +0000, Johannes Thumshirn wrote:
> On 15.01.24 20:39, David Sterba wrote:
> > There's a warning in btrfs_issue_discard() when the range is not aligned
> > to 512 bytes, originally added in 4d89d377bbb0 ("btrfs:
> > btrfs_issue_discard ensure offset/length are aligned to sector
> > boundaries"). We can't do sub-sector writes anyway so the adjustment is
> > the only thing that we can do and the warning is unnecessary.
> >
> > CC: stable@xxxxxxxxxxxxxxx # 4.19+
> > Reported-by: syzbot+4a4f1eba14eb5c3417d1@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Signed-off-by: David Sterba <dsterba@xxxxxxxx>
> > ---
> > fs/btrfs/extent-tree.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> > index 6d680031211a..8e8cc1111277 100644
> > --- a/fs/btrfs/extent-tree.c
> > +++ b/fs/btrfs/extent-tree.c
> > @@ -1260,7 +1260,8 @@ static int btrfs_issue_discard(struct block_device *bdev, u64 start, u64 len,
> > u64 bytes_left, end;
> > u64 aligned_start = ALIGN(start, 1 << SECTOR_SHIFT);
> >
> > - if (WARN_ON(start != aligned_start)) {
> > + /* Adjust the range to be aligned to 512B sectors if necessary. */
> > + if (start != aligned_start) {
> > len -= aligned_start - start;
> > len = round_down(len, 1 << SECTOR_SHIFT);
> > start = aligned_start;
>
> Maybe leave a
> btrfs_warn(fs_info, "adjusting discard range to 512b boundary");
> in there?
>
> Or is info a more appropriate level?
I don't know if we should warn here at all, the warning is there since
2015 and only now we see it triggered, I guess everybody sane using the
discard interface uses proper values. Warning level means that it needs
user attention that something is wrong and take some action or examine
the system.
The discard range could be verified at the beginning of the ioctl, but
printing it here is a bit late. I've checked xfs and it silently rounds
down the start and offset the same way, I find this sufficient thing to
do.
