Re: [PATCH] xfs: clarify units in the failed metadata io message

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Mon, Jan 08, 2018 at 02:19:58PM -0600, Eric Sandeen wrote:
> On 1/8/18 1:40 PM, Darrick J. Wong wrote:> From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > If a metadata IO error happens, we report the location of the failed IO
> > request in units of daddrs.  However, the printk message misleads people
> > into thinking that the units are fs blocks, so fix the reported units.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/xfs_buf.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> > index 1981ef7..582c64a 100644
> > --- a/fs/xfs/xfs_buf.c
> > +++ b/fs/xfs/xfs_buf.c
> > @@ -1196,7 +1196,7 @@ xfs_buf_ioerror_alert(
> >  	const char		*func)
> >  {
> >  	xfs_alert(bp->b_target->bt_mount,
> > -"metadata I/O error: block 0x%llx (\"%s\") error %d numblks %d",
> > +"metadata I/O error: daddr 0x%llx (\"%s\") error %d numblks %d",
> >  		(uint64_t)XFS_BUF_ADDR(bp), func, -bp->b_error, bp->b_length);
> Ok, so I might have nodded in approval on IRC a little early; if "block"
> was misleading, I think "numblks" is too.  So if you change one to
> daddr, I wonder if the other should be changed as well to, um ...
> 
> 
> "metadata I/O error: daddr 0x%llx (\"%s\") error %d count %d"

"metadata I/O error in ("\%s\") at daddr 0x%llx len %d error %d"

also:
"%s: daddr out of range: block 0x%llx, EOFS 0x%llx ", __func__, b_bn, eofs

"%s: no ops on buf for daddr 0x%llx/0x%x", __func__, b_bn, b_length

"Corruption Alert: Buffer at daddr 0x%llx had permanent write failures!"

So I guess it's also not consistent how we print buffer length either?
I'll work on something to clean all this up...

--D

> ?  That's a bit vague but at least not misleadingly containing a form of
> "blocks" - whatchathink?  Maybe "len?"  Even "sectors" is a bit overloaded
> now.  :(
> 
> -Eric
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html