misc cleanups

misc cleanups

[Christoph Hellwig]

Just a couple random cleanups from invesigating a larger project.

[PATCH 1/3] xfs: add a xfs_valid_startblock helper

[Christoph Hellwig]

Add a helper that validates the startblock is valid.  This checks for a
non-zero block on the main device, but skips that check for blocks on
the realtime device.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_bmap.c | 2 +-
 fs/xfs/libxfs/xfs_bmap.h | 3 +++
 fs/xfs/xfs_iomap.c       | 6 +++---
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 05aedf4a538c..80b25e21e708 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4519,7 +4519,7 @@ xfs_bmapi_convert_delalloc(
 	if (WARN_ON_ONCE(bma.blkno == NULLFSBLOCK))
 		goto out_finish;
 	error = -EFSCORRUPTED;
-	if (WARN_ON_ONCE(!bma.got.br_startblock && !XFS_IS_REALTIME_INODE(ip)))
+	if (WARN_ON_ONCE(!xfs_valid_startblock(ip, bma.got.br_startblock)))
 		goto out_finish;
 
 	XFS_STATS_ADD(mp, xs_xstrat_bytes, XFS_FSB_TO_B(mp, bma.length));
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index c409871a096e..7efa56e8750f 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -171,6 +171,9 @@ static inline bool xfs_bmap_is_real_extent(struct xfs_bmbt_irec *irec)
 		!isnullstartblock(irec->br_startblock);
 }
 
+#define xfs_valid_startblock(ip, startblock) \
+	((startblock) != 0 || XFS_IS_REALTIME_INODE(ip))
+
 void	xfs_trim_extent(struct xfs_bmbt_irec *irec, xfs_fileoff_t bno,
 		xfs_filblks_t len);
 int	xfs_bmap_add_attrfork(struct xfs_inode *ip, int size, int rsvd);
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 3a4310d7cb59..f780e223b118 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -58,7 +58,7 @@ xfs_bmbt_to_iomap(
 {
 	struct xfs_mount	*mp = ip->i_mount;
 
-	if (unlikely(!imap->br_startblock && !XFS_IS_REALTIME_INODE(ip)))
+	if (unlikely(!xfs_valid_startblock(ip, imap->br_startblock)))
 		return xfs_alert_fsblock_zero(ip, imap);
 
 	if (imap->br_startblock == HOLESTARTBLOCK) {
@@ -297,7 +297,7 @@ xfs_iomap_write_direct(
 		goto out_unlock;
 	}
 
-	if (!(imap->br_startblock || XFS_IS_REALTIME_INODE(ip)))
+	if (unlikely(!xfs_valid_startblock(ip, imap->br_startblock)))
 		error = xfs_alert_fsblock_zero(ip, imap);
 
 out_unlock:
@@ -814,7 +814,7 @@ xfs_iomap_write_unwritten(
 		if (error)
 			return error;
 
-		if (!(imap.br_startblock || XFS_IS_REALTIME_INODE(ip)))
+		if (unlikely(!xfs_valid_startblock(ip, imap.br_startblock)))
 			return xfs_alert_fsblock_zero(ip, &imap);
 
 		if ((numblks_fsb = imap.br_blockcount) == 0) {
-- 
2.20.1

[PATCH 2/3] xfs: cleanup xfs_fsb_to_db

[Christoph Hellwig]

This function isn't a macro anymore, so remove various superflous braces,
and explicit cast that is done implicitly due to the return value, use
a normal if statement instead of trying to squeeze everything together.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_bmap_util.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index e12f0ba7f2eb..0910cb75b65d 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -39,9 +39,9 @@
 xfs_daddr_t
 xfs_fsb_to_db(struct xfs_inode *ip, xfs_fsblock_t fsb)
 {
-	return (XFS_IS_REALTIME_INODE(ip) ? \
-		 (xfs_daddr_t)XFS_FSB_TO_BB((ip)->i_mount, (fsb)) : \
-		 XFS_FSB_TO_DADDR((ip)->i_mount, (fsb)));
+	if (XFS_IS_REALTIME_INODE(ip))
+		return XFS_FSB_TO_BB(ip->i_mount, fsb);
+	return XFS_FSB_TO_DADDR(ip->i_mount, fsb);
 }
 
 /*
-- 
2.20.1

[PATCH 3/3] xfs: remove the unused XFS_ALLOC_USERDATA flag

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_alloc.h | 7 +++----
 fs/xfs/libxfs/xfs_bmap.c  | 8 ++------
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
index d6ed5d2c07c2..58fa85cec325 100644
--- a/fs/xfs/libxfs/xfs_alloc.h
+++ b/fs/xfs/libxfs/xfs_alloc.h
@@ -81,10 +81,9 @@ typedef struct xfs_alloc_arg {
 /*
  * Defines for datatype
  */
-#define XFS_ALLOC_USERDATA		(1 << 0)/* allocation is for user data*/
-#define XFS_ALLOC_INITIAL_USER_DATA	(1 << 1)/* special case start of file */
-#define XFS_ALLOC_USERDATA_ZERO		(1 << 2)/* zero extent on allocation */
-#define XFS_ALLOC_NOBUSY		(1 << 3)/* Busy extents not allowed */
+#define XFS_ALLOC_INITIAL_USER_DATA	(1 << 0)/* special case start of file */
+#define XFS_ALLOC_USERDATA_ZERO		(1 << 1)/* zero extent on allocation */
+#define XFS_ALLOC_NOBUSY		(1 << 2)/* Busy extents not allowed */
 
 static inline bool
 xfs_alloc_is_userdata(int datatype)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 80b25e21e708..054b4ce30033 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4042,12 +4042,8 @@ xfs_bmapi_allocate(
 	 */
 	if (!(bma->flags & XFS_BMAPI_METADATA)) {
 		bma->datatype = XFS_ALLOC_NOBUSY;
-		if (whichfork == XFS_DATA_FORK) {
-			if (bma->offset == 0)
-				bma->datatype |= XFS_ALLOC_INITIAL_USER_DATA;
-			else
-				bma->datatype |= XFS_ALLOC_USERDATA;
-		}
+		if (whichfork == XFS_DATA_FORK && bma->offset == 0)
+			bma->datatype |= XFS_ALLOC_INITIAL_USER_DATA;
 		if (bma->flags & XFS_BMAPI_ZERO)
 			bma->datatype |= XFS_ALLOC_USERDATA_ZERO;
 	}
-- 
2.20.1

Re: [PATCH 1/3] xfs: add a xfs_valid_startblock helper

[Darrick J. Wong]

On Fri, Aug 30, 2019 at 12:24:09PM +0200, Christoph Hellwig wrote:
> Add a helper that validates the startblock is valid.  This checks for a
> non-zero block on the main device, but skips that check for blocks on
> the realtime device.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_bmap.c | 2 +-
>  fs/xfs/libxfs/xfs_bmap.h | 3 +++
>  fs/xfs/xfs_iomap.c       | 6 +++---
>  3 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 05aedf4a538c..80b25e21e708 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4519,7 +4519,7 @@ xfs_bmapi_convert_delalloc(
>  	if (WARN_ON_ONCE(bma.blkno == NULLFSBLOCK))
>  		goto out_finish;
>  	error = -EFSCORRUPTED;
> -	if (WARN_ON_ONCE(!bma.got.br_startblock && !XFS_IS_REALTIME_INODE(ip)))
> +	if (WARN_ON_ONCE(!xfs_valid_startblock(ip, bma.got.br_startblock)))
>  		goto out_finish;
>  
>  	XFS_STATS_ADD(mp, xs_xstrat_bytes, XFS_FSB_TO_B(mp, bma.length));
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index c409871a096e..7efa56e8750f 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -171,6 +171,9 @@ static inline bool xfs_bmap_is_real_extent(struct xfs_bmbt_irec *irec)
>  		!isnullstartblock(irec->br_startblock);
>  }
>  
> +#define xfs_valid_startblock(ip, startblock) \
> +	((startblock) != 0 || XFS_IS_REALTIME_INODE(ip))

We have more robust validators for data/rtdev fsblock_t, so why not:

#define xfs_valid_startblock(ip, startblock) \
	(XFS_IS_REALTIME_INODE(ip) ? xfs_verify_rtbno(startblock) : \
				     xfs_verify_fsbno(startblock))

and why not make it a static inline function too?

--D

> +
>  void	xfs_trim_extent(struct xfs_bmbt_irec *irec, xfs_fileoff_t bno,
>  		xfs_filblks_t len);
>  int	xfs_bmap_add_attrfork(struct xfs_inode *ip, int size, int rsvd);
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 3a4310d7cb59..f780e223b118 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -58,7 +58,7 @@ xfs_bmbt_to_iomap(
>  {
>  	struct xfs_mount	*mp = ip->i_mount;
>  
> -	if (unlikely(!imap->br_startblock && !XFS_IS_REALTIME_INODE(ip)))
> +	if (unlikely(!xfs_valid_startblock(ip, imap->br_startblock)))
>  		return xfs_alert_fsblock_zero(ip, imap);
>  
>  	if (imap->br_startblock == HOLESTARTBLOCK) {
> @@ -297,7 +297,7 @@ xfs_iomap_write_direct(
>  		goto out_unlock;
>  	}
>  
> -	if (!(imap->br_startblock || XFS_IS_REALTIME_INODE(ip)))
> +	if (unlikely(!xfs_valid_startblock(ip, imap->br_startblock)))
>  		error = xfs_alert_fsblock_zero(ip, imap);
>  
>  out_unlock:
> @@ -814,7 +814,7 @@ xfs_iomap_write_unwritten(
>  		if (error)
>  			return error;
>  
> -		if (!(imap.br_startblock || XFS_IS_REALTIME_INODE(ip)))
> +		if (unlikely(!xfs_valid_startblock(ip, imap.br_startblock)))
>  			return xfs_alert_fsblock_zero(ip, &imap);
>  
>  		if ((numblks_fsb = imap.br_blockcount) == 0) {
> -- 
> 2.20.1
> 

Re: [PATCH 2/3] xfs: cleanup xfs_fsb_to_db

[Darrick J. Wong]

On Fri, Aug 30, 2019 at 12:24:10PM +0200, Christoph Hellwig wrote:
> This function isn't a macro anymore, so remove various superflous braces,
> and explicit cast that is done implicitly due to the return value, use
> a normal if statement instead of trying to squeeze everything together.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_bmap_util.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index e12f0ba7f2eb..0910cb75b65d 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -39,9 +39,9 @@
>  xfs_daddr_t
>  xfs_fsb_to_db(struct xfs_inode *ip, xfs_fsblock_t fsb)
>  {
> -	return (XFS_IS_REALTIME_INODE(ip) ? \
> -		 (xfs_daddr_t)XFS_FSB_TO_BB((ip)->i_mount, (fsb)) : \
> -		 XFS_FSB_TO_DADDR((ip)->i_mount, (fsb)));
> +	if (XFS_IS_REALTIME_INODE(ip))
> +		return XFS_FSB_TO_BB(ip->i_mount, fsb);
> +	return XFS_FSB_TO_DADDR(ip->i_mount, fsb);
>  }
>  
>  /*
> -- 
> 2.20.1
> 

Re: [PATCH 3/3] xfs: remove the unused XFS_ALLOC_USERDATA flag

[Darrick J. Wong]

On Fri, Aug 30, 2019 at 12:24:11PM +0200, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/libxfs/xfs_alloc.h | 7 +++----
>  fs/xfs/libxfs/xfs_bmap.c  | 8 ++------
>  2 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
> index d6ed5d2c07c2..58fa85cec325 100644
> --- a/fs/xfs/libxfs/xfs_alloc.h
> +++ b/fs/xfs/libxfs/xfs_alloc.h
> @@ -81,10 +81,9 @@ typedef struct xfs_alloc_arg {
>  /*
>   * Defines for datatype
>   */
> -#define XFS_ALLOC_USERDATA		(1 << 0)/* allocation is for user data*/
> -#define XFS_ALLOC_INITIAL_USER_DATA	(1 << 1)/* special case start of file */
> -#define XFS_ALLOC_USERDATA_ZERO		(1 << 2)/* zero extent on allocation */
> -#define XFS_ALLOC_NOBUSY		(1 << 3)/* Busy extents not allowed */
> +#define XFS_ALLOC_INITIAL_USER_DATA	(1 << 0)/* special case start of file */
> +#define XFS_ALLOC_USERDATA_ZERO		(1 << 1)/* zero extent on allocation */
> +#define XFS_ALLOC_NOBUSY		(1 << 2)/* Busy extents not allowed */
>  
>  static inline bool
>  xfs_alloc_is_userdata(int datatype)
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 80b25e21e708..054b4ce30033 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4042,12 +4042,8 @@ xfs_bmapi_allocate(
>  	 */
>  	if (!(bma->flags & XFS_BMAPI_METADATA)) {
>  		bma->datatype = XFS_ALLOC_NOBUSY;
> -		if (whichfork == XFS_DATA_FORK) {
> -			if (bma->offset == 0)
> -				bma->datatype |= XFS_ALLOC_INITIAL_USER_DATA;
> -			else
> -				bma->datatype |= XFS_ALLOC_USERDATA;
> -		}
> +		if (whichfork == XFS_DATA_FORK && bma->offset == 0)
> +			bma->datatype |= XFS_ALLOC_INITIAL_USER_DATA;
>  		if (bma->flags & XFS_BMAPI_ZERO)
>  			bma->datatype |= XFS_ALLOC_USERDATA_ZERO;
>  	}
> -- 
> 2.20.1
> 

Re: [PATCH 1/3] xfs: add a xfs_valid_startblock helper

[Christoph Hellwig]

On Fri, Aug 30, 2019 at 08:06:50AM -0700, Darrick J. Wong wrote:
> > --- a/fs/xfs/libxfs/xfs_bmap.h
> > +++ b/fs/xfs/libxfs/xfs_bmap.h
> > @@ -171,6 +171,9 @@ static inline bool xfs_bmap_is_real_extent(struct xfs_bmbt_irec *irec)
> >  		!isnullstartblock(irec->br_startblock);
> >  }
> >  
> > +#define xfs_valid_startblock(ip, startblock) \
> > +	((startblock) != 0 || XFS_IS_REALTIME_INODE(ip))
> 
> We have more robust validators for data/rtdev fsblock_t, so why not:
> 
> #define xfs_valid_startblock(ip, startblock) \
> 	(XFS_IS_REALTIME_INODE(ip) ? xfs_verify_rtbno(startblock) : \
> 				     xfs_verify_fsbno(startblock))
> 
> and why not make it a static inline function too?

I tried an inline function, but I could not find a header to place
it that would actually easily compile everywhere...  Maybe we should
just make that a xfs_verify_bno(mp, startblock) and move that out of
line such in a way that a smart compiler avoids the function call
overhead for xfs_verify_rtbno / xfs_verify_fsbno.  I'll take another
stab at this.

Re: [PATCH 1/3] xfs: add a xfs_valid_startblock helper

[Christoph Hellwig]

On Fri, Aug 30, 2019 at 05:32:53PM +0200, Christoph Hellwig wrote:
> On Fri, Aug 30, 2019 at 08:06:50AM -0700, Darrick J. Wong wrote:
> > > --- a/fs/xfs/libxfs/xfs_bmap.h
> > > +++ b/fs/xfs/libxfs/xfs_bmap.h
> > > @@ -171,6 +171,9 @@ static inline bool xfs_bmap_is_real_extent(struct xfs_bmbt_irec *irec)
> > >  		!isnullstartblock(irec->br_startblock);
> > >  }
> > >  
> > > +#define xfs_valid_startblock(ip, startblock) \
> > > +	((startblock) != 0 || XFS_IS_REALTIME_INODE(ip))
> > 
> > We have more robust validators for data/rtdev fsblock_t, so why not:
> > 
> > #define xfs_valid_startblock(ip, startblock) \
> > 	(XFS_IS_REALTIME_INODE(ip) ? xfs_verify_rtbno(startblock) : \
> > 				     xfs_verify_fsbno(startblock))
> > 
> > and why not make it a static inline function too?
> 
> I tried an inline function, but I could not find a header to place
> it that would actually easily compile everywhere...  Maybe we should
> just make that a xfs_verify_bno(mp, startblock) and move that out of
> line such in a way that a smart compiler avoids the function call
> overhead for xfs_verify_rtbno / xfs_verify_fsbno.  I'll take another
> stab at this.

So I looked into your suggestion, but xfs_verify_rtbno / xfs_verify_fsbno
do a lot of validity checking, but they don't actually contain the
check that was in the existing code.  The bmap code just checks that
there is a startblock of 0 for non-rt devices, probably this was added
to find some old bug where a irec structure that was zeroed was returned.

So replacing it with xfs_verify_rtbno / xfs_verify_fsbno would not help
in any way.  But the big question is if keeping the 0 check is even
worth it.

Re: [PATCH 1/3] xfs: add a xfs_valid_startblock helper

[Darrick J. Wong]

On Sun, Sep 01, 2019 at 09:36:34AM +0200, Christoph Hellwig wrote:
> On Fri, Aug 30, 2019 at 05:32:53PM +0200, Christoph Hellwig wrote:
> > On Fri, Aug 30, 2019 at 08:06:50AM -0700, Darrick J. Wong wrote:
> > > > --- a/fs/xfs/libxfs/xfs_bmap.h
> > > > +++ b/fs/xfs/libxfs/xfs_bmap.h
> > > > @@ -171,6 +171,9 @@ static inline bool xfs_bmap_is_real_extent(struct xfs_bmbt_irec *irec)
> > > >  		!isnullstartblock(irec->br_startblock);
> > > >  }
> > > >  
> > > > +#define xfs_valid_startblock(ip, startblock) \
> > > > +	((startblock) != 0 || XFS_IS_REALTIME_INODE(ip))
> > > 
> > > We have more robust validators for data/rtdev fsblock_t, so why not:
> > > 
> > > #define xfs_valid_startblock(ip, startblock) \
> > > 	(XFS_IS_REALTIME_INODE(ip) ? xfs_verify_rtbno(startblock) : \
> > > 				     xfs_verify_fsbno(startblock))
> > > 
> > > and why not make it a static inline function too?
> > 
> > I tried an inline function, but I could not find a header to place
> > it that would actually easily compile everywhere...  Maybe we should
> > just make that a xfs_verify_bno(mp, startblock) and move that out of
> > line such in a way that a smart compiler avoids the function call
> > overhead for xfs_verify_rtbno / xfs_verify_fsbno.  I'll take another
> > stab at this.
> 
> So I looked into your suggestion, but xfs_verify_rtbno / xfs_verify_fsbno
> do a lot of validity checking, but they don't actually contain the
> check that was in the existing code.  The bmap code just checks that
> there is a startblock of 0 for non-rt devices, probably this was added
> to find some old bug where a irec structure that was zeroed was returned.
> 
> So replacing it with xfs_verify_rtbno / xfs_verify_fsbno would not help
> in any way.  But the big question is if keeping the 0 check is even
> worth it.

It's been mildly helpful for noticing when my online/offline repair
prototype code totally screws up, but at that point so much magic smoke
is already pouring out everywhere that it's hard not to notice. :)

--D

Re: [PATCH 1/3] xfs: add a xfs_valid_startblock helper

[Christoph Hellwig]

On Sun, Sep 01, 2019 at 01:31:40PM -0700, Darrick J. Wong wrote:
> It's been mildly helpful for noticing when my online/offline repair
> prototype code totally screws up, but at that point so much magic smoke
> is already pouring out everywhere that it's hard not to notice. :)

That suggests to just keep the macro as I submitted it, maybe with
a big fat comment explaining the usage.

Re: [PATCH 1/3] xfs: add a xfs_valid_startblock helper

[Darrick J. Wong]

On Mon, Sep 02, 2019 at 09:59:46AM +0200, Christoph Hellwig wrote:
> On Sun, Sep 01, 2019 at 01:31:40PM -0700, Darrick J. Wong wrote:
> > It's been mildly helpful for noticing when my online/offline repair
> > prototype code totally screws up, but at that point so much magic smoke
> > is already pouring out everywhere that it's hard not to notice. :)
> 
> That suggests to just keep the macro as I submitted it, maybe with
> a big fat comment explaining the usage.

Ok.  Do you want to resubmit with a comment of your choosing, or let me
write in whatever:

/*
 * Check the mapping for obviously garbage allocations that could trash
 * the filesystem immediately.
 */

?

--D

Re: [PATCH 1/3] xfs: add a xfs_valid_startblock helper

[Christoph Hellwig]

On Mon, Sep 02, 2019 at 10:04:40AM -0700, Darrick J. Wong wrote:
> On Mon, Sep 02, 2019 at 09:59:46AM +0200, Christoph Hellwig wrote:
> > On Sun, Sep 01, 2019 at 01:31:40PM -0700, Darrick J. Wong wrote:
> > > It's been mildly helpful for noticing when my online/offline repair
> > > prototype code totally screws up, but at that point so much magic smoke
> > > is already pouring out everywhere that it's hard not to notice. :)
> > 
> > That suggests to just keep the macro as I submitted it, maybe with
> > a big fat comment explaining the usage.
> 
> Ok.  Do you want to resubmit with a comment of your choosing, or let me
> write in whatever:
> 
> /*
>  * Check the mapping for obviously garbage allocations that could trash
>  * the filesystem immediately.
>  */

I was going to resend it, but now that you've written the comment for
me feel free to just apply it with that added.