On Wed 14-02-24 16:01:57, Andreas Dilger wrote:
> On Feb 13, 2024, at 3:16 AM, Jan Kara <jack@xxxxxxx> wrote:
> >
> > When ext4 is mounted without journal, with discard mount option, and on
> > a device not supporting trim, we print error for each and every freed
> > extent. This is not only useless but actively harmful. Instead ignore
> > the EOPNOTSUPP error. Trim is only advisory anyway and when the
> > filesystem has journal we silently ignore trim error as well.
> >
>
> > Signed-off-by: Jan Kara <jack@xxxxxxx>
> > ---
> > fs/ext4/mballoc.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> > index e4f7cf9d89c4..aed620cf4d40 100644
> > --- a/fs/ext4/mballoc.c
> > +++ b/fs/ext4/mballoc.c
> > @@ -6488,7 +6488,13 @@ static void ext4_mb_clear_bb(handle_t *handle, struct inode *inode,
> > if (test_opt(sb, DISCARD)) {
> > err = ext4_issue_discard(sb, block_group, bit,
> > count_clusters, NULL);
> > - if (err && err != -EOPNOTSUPP)
> > + /*
> > + * Ignore EOPNOTSUPP error. This is consistent with
> > + * what happens when using journal.
> > + */
> > + if (err == -EOPNOTSUPP)
> > + err = 0;
> > + if (err)
>
> I don't see how this patch is actually changing whether the error message
> is printed? Previously, if "err" was set and err was -EOPNOTSUPP the
> message was skipped. Now it is doing the same thing in a different way?
>
> The "err" value is overwritten 50 lines later on without being used:
>
> err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh);
>
> so the setting "err = 0" doesn't really affect the later code either.
> What am I missing?
Yeah, the code flow is a bit contrived. The error message gets printed by
ext4_std_error() at the end of ext4_mb_clear_bb(). I don't think there's
any ext4_handle_dirty_metadata() call in the current version of
ext4_mb_clear_bb()...
Honza
>
> Cheers, Andreas
>
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
