Re: [PATCH 10/13] iomap: only call mapping_set_error once for each failed bio

Christoph Hellwig
2023-11-29T05:41:42Z

On Tue, Nov 28, 2023 at 09:03:22PM -0800, Darrick J. Wong wrote:
> > + if (error) {
> > + mapping_set_error(inode->i_mapping, error);
> > + if (!bio_flagged(bio, BIO_QUIET)) {
> > + pr_err_ratelimited(
> > +"%s: writeback error on inode %lu, offset %lld, sector %llu",
> > + inode->i_sb->s_id, inode->i_ino,
> > + ioend->io_offset, ioend->io_sector);
>
> Something that's always bothered me: Why don't we log the *amount* of
> data that just failed writeback?
>
> "XFS: writeback error on inode 63, offset 4096, length 8192, sector 27"
>
> Now we'd actually have some way to work out that we've lost 8k worth of
> data. OFC maybe the better solution is to wire up that inotify error
> reporting interface that ext4 added a while back...?

Just adding the amount sounds sane to me and I can add a patch to do
that. I think this message originated in buffer.c, where it was
printed once for each block before rate limiting got added..