All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: Eric Sandeen <sandeen@redhat.com>
Cc: linux-xfs <linux-xfs@vger.kernel.org>
Subject: Re: [PATCH] xfs: ignore leaf attr ichdr.count in verifier during log replay
Date: Thu, 1 Dec 2016 07:15:39 -0500	[thread overview]
Message-ID: <20161201121538.GA22890@bfoster.bfoster> (raw)
In-Reply-To: <596737a3-fbf8-8712-422c-4705ce36deae@redhat.com>

On Wed, Nov 30, 2016 at 04:33:15PM -0600, Eric Sandeen wrote:
> When we create a new attribute, we first create a shortform
> attribute, and try to fit the new attribute into it.
> If that fails, we copy the (empty) attribute into a leaf attribute,
> and do the copy again.  Thus there can be a transient state where
> we have an empty leaf attribute.
> 
> If we encounter this during log replay, the verifier will fail.
> So add a test to ignore this part of the leaf attr verification
> during log replay.
> 
> Thanks as usual to dchinner for spotting the problem.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 8ea91f3..2852521 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -253,6 +253,7 @@ STATIC void xfs_attr3_leaf_moveents(struct xfs_da_args *args,
>  {
>  	struct xfs_mount	*mp = bp->b_target->bt_mount;
>  	struct xfs_attr_leafblock *leaf = bp->b_addr;
> +	struct xfs_perag *pag = bp->b_pag;
>  	struct xfs_attr3_icleaf_hdr ichdr;
>  
>  	xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &ichdr, leaf);
> @@ -273,7 +274,12 @@ STATIC void xfs_attr3_leaf_moveents(struct xfs_da_args *args,
>  		if (ichdr.magic != XFS_ATTR_LEAF_MAGIC)
>  			return false;
>  	}
> -	if (ichdr.count == 0)
> +	/*
> +	 * In recovery there is a transient state where count == 0 is valid
> +	 * because we may have transitioned an empty shortform attr to a leaf
> +	 * if the attr didn't fit in shortform.
> +	 */
> +	if (pag && pag->pagf_init && ichdr.count == 0)
>  		return false;

Seems fine, but if the idea is to filter out failures during log
recovery, can we detect that state explicitly? E.g., check for some
combination of XLOG_ACTIVE_RECOVERY and/or XLOG_RECOVERY_NEEDED (or just
define and use a new flag/helper if necessary)?

Brian

>  
>  	/* XXX: need to range check rest of attr header values */
> --
> 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

  reply	other threads:[~2016-12-01 12:15 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-30 22:33 [PATCH] xfs: ignore leaf attr ichdr.count in verifier during log replay Eric Sandeen
2016-12-01 12:15 ` Brian Foster [this message]
2016-12-05 20:31   ` Eric Sandeen
2016-12-05 21:33     ` Brian Foster
2016-12-05 21:45       ` Eric Sandeen
2016-12-05 16:21 ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161201121538.GA22890@bfoster.bfoster \
    --to=bfoster@redhat.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=sandeen@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.