From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 1/9] xfs: log item flags are racy
Date: Wed, 9 May 2018 07:51:42 -0700 [thread overview]
Message-ID: <20180509145142.GI11261@magnolia> (raw)
In-Reply-To: <20180508034202.10136-2-david@fromorbit.com>
On Tue, May 08, 2018 at 01:41:54PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> The log item flags contain a field that is protected by the AIL
> lock - the XFS_LI_IN_AIL flag. We use non-atomic RMW operations to
> set and clear these flags, but most of the updates and checks are
> not done with the AIL lock held and so are susceptible to update
> races.
>
> Fix this by changing the log item flags to use atomic bitops rather
> than be reliant on the AIL lock for update serialisation.
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Brian Foster <bfoster@redhat.com>
Looks ok, will test,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/xfs/xfs_bmap_item.c | 4 ++--
> fs/xfs/xfs_buf_item.c | 4 +++-
> fs/xfs/xfs_dquot.c | 7 +++----
> fs/xfs/xfs_dquot_item.c | 2 +-
> fs/xfs/xfs_extfree_item.c | 4 ++--
> fs/xfs/xfs_icache.c | 3 ++-
> fs/xfs/xfs_icreate_item.c | 2 +-
> fs/xfs/xfs_inode.c | 4 ++--
> fs/xfs/xfs_inode_item.c | 8 ++++----
> fs/xfs/xfs_log.c | 2 +-
> fs/xfs/xfs_qm.c | 2 +-
> fs/xfs/xfs_refcount_item.c | 4 ++--
> fs/xfs/xfs_rmap_item.c | 4 ++--
> fs/xfs/xfs_trace.h | 6 +++---
> fs/xfs/xfs_trans.c | 4 ++--
> fs/xfs/xfs_trans.h | 19 ++++++++++++-------
> fs/xfs/xfs_trans_ail.c | 9 ++++-----
> fs/xfs/xfs_trans_buf.c | 2 +-
> fs/xfs/xfs_trans_priv.h | 10 ++++------
> 19 files changed, 52 insertions(+), 48 deletions(-)
>
> diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
> index 2203465e63ea..618bb71535c8 100644
> --- a/fs/xfs/xfs_bmap_item.c
> +++ b/fs/xfs/xfs_bmap_item.c
> @@ -160,7 +160,7 @@ STATIC void
> xfs_bui_item_unlock(
> struct xfs_log_item *lip)
> {
> - if (lip->li_flags & XFS_LI_ABORTED)
> + if (test_bit(XFS_LI_ABORTED, &lip->li_flags))
> xfs_bui_release(BUI_ITEM(lip));
> }
>
> @@ -305,7 +305,7 @@ xfs_bud_item_unlock(
> {
> struct xfs_bud_log_item *budp = BUD_ITEM(lip);
>
> - if (lip->li_flags & XFS_LI_ABORTED) {
> + if (test_bit(XFS_LI_ABORTED, &lip->li_flags)) {
> xfs_bui_release(budp->bud_buip);
> kmem_zone_free(xfs_bud_zone, budp);
> }
> diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
> index 82ad270e390e..df62082f2204 100644
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
> @@ -568,13 +568,15 @@ xfs_buf_item_unlock(
> {
> struct xfs_buf_log_item *bip = BUF_ITEM(lip);
> struct xfs_buf *bp = bip->bli_buf;
> - bool aborted = !!(lip->li_flags & XFS_LI_ABORTED);
> + bool aborted;
> bool hold = !!(bip->bli_flags & XFS_BLI_HOLD);
> bool dirty = !!(bip->bli_flags & XFS_BLI_DIRTY);
> #if defined(DEBUG) || defined(XFS_WARN)
> bool ordered = !!(bip->bli_flags & XFS_BLI_ORDERED);
> #endif
>
> + aborted = test_bit(XFS_LI_ABORTED, &lip->li_flags);
> +
> /* Clear the buffer's association with this transaction. */
> bp->b_transp = NULL;
>
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index a7daef9e16bf..d0b65679baae 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -913,9 +913,9 @@ xfs_qm_dqflush_done(
> * since it's cheaper, and then we recheck while
> * holding the lock before removing the dquot from the AIL.
> */
> - if ((lip->li_flags & XFS_LI_IN_AIL) &&
> + if (test_bit(XFS_LI_IN_AIL, &lip->li_flags) &&
> ((lip->li_lsn == qip->qli_flush_lsn) ||
> - (lip->li_flags & XFS_LI_FAILED))) {
> + test_bit(XFS_LI_FAILED, &lip->li_flags))) {
>
> /* xfs_trans_ail_delete() drops the AIL lock. */
> spin_lock(&ailp->ail_lock);
> @@ -926,8 +926,7 @@ xfs_qm_dqflush_done(
> * Clear the failed state since we are about to drop the
> * flush lock
> */
> - if (lip->li_flags & XFS_LI_FAILED)
> - xfs_clear_li_failed(lip);
> + xfs_clear_li_failed(lip);
> spin_unlock(&ailp->ail_lock);
> }
> }
> diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
> index 4b331e354da7..57df98122156 100644
> --- a/fs/xfs/xfs_dquot_item.c
> +++ b/fs/xfs/xfs_dquot_item.c
> @@ -173,7 +173,7 @@ xfs_qm_dquot_logitem_push(
> * The buffer containing this item failed to be written back
> * previously. Resubmit the buffer for IO
> */
> - if (lip->li_flags & XFS_LI_FAILED) {
> + if (test_bit(XFS_LI_FAILED, &lip->li_flags)) {
> if (!xfs_buf_trylock(bp))
> return XFS_ITEM_LOCKED;
>
> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> index b5b1e567b9f4..70b7d48af6d6 100644
> --- a/fs/xfs/xfs_extfree_item.c
> +++ b/fs/xfs/xfs_extfree_item.c
> @@ -168,7 +168,7 @@ STATIC void
> xfs_efi_item_unlock(
> struct xfs_log_item *lip)
> {
> - if (lip->li_flags & XFS_LI_ABORTED)
> + if (test_bit(XFS_LI_ABORTED, &lip->li_flags))
> xfs_efi_release(EFI_ITEM(lip));
> }
>
> @@ -402,7 +402,7 @@ xfs_efd_item_unlock(
> {
> struct xfs_efd_log_item *efdp = EFD_ITEM(lip);
>
> - if (lip->li_flags & XFS_LI_ABORTED) {
> + if (test_bit(XFS_LI_ABORTED, &lip->li_flags)) {
> xfs_efi_release(efdp->efd_efip);
> xfs_efd_item_free(efdp);
> }
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 9a18f69f6e96..98b7a4ae15e4 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -107,7 +107,8 @@ xfs_inode_free_callback(
> xfs_idestroy_fork(ip, XFS_COW_FORK);
>
> if (ip->i_itemp) {
> - ASSERT(!(ip->i_itemp->ili_item.li_flags & XFS_LI_IN_AIL));
> + ASSERT(!test_bit(XFS_LI_IN_AIL,
> + &ip->i_itemp->ili_item.li_flags));
> xfs_inode_item_destroy(ip);
> ip->i_itemp = NULL;
> }
> diff --git a/fs/xfs/xfs_icreate_item.c b/fs/xfs/xfs_icreate_item.c
> index 865ad1373e5e..a99a0f8aa528 100644
> --- a/fs/xfs/xfs_icreate_item.c
> +++ b/fs/xfs/xfs_icreate_item.c
> @@ -91,7 +91,7 @@ xfs_icreate_item_unlock(
> {
> struct xfs_icreate_item *icp = ICR_ITEM(lip);
>
> - if (icp->ic_item.li_flags & XFS_LI_ABORTED)
> + if (test_bit(XFS_LI_ABORTED, &lip->li_flags))
> kmem_zone_free(xfs_icreate_zone, icp);
> return;
> }
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 2b70c8b4cee2..16a43ba884cc 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -498,7 +498,7 @@ xfs_lock_inodes(
> if (!try_lock) {
> for (j = (i - 1); j >= 0 && !try_lock; j--) {
> lp = (xfs_log_item_t *)ips[j]->i_itemp;
> - if (lp && (lp->li_flags & XFS_LI_IN_AIL))
> + if (lp && test_bit(XFS_LI_IN_AIL, &lp->li_flags))
> try_lock++;
> }
> }
> @@ -598,7 +598,7 @@ xfs_lock_two_inodes(
> * and try again.
> */
> lp = (xfs_log_item_t *)ip0->i_itemp;
> - if (lp && (lp->li_flags & XFS_LI_IN_AIL)) {
> + if (lp && test_bit(XFS_LI_IN_AIL, &lp->li_flags)) {
> if (!xfs_ilock_nowait(ip1, xfs_lock_inumorder(ip1_mode, 1))) {
> xfs_iunlock(ip0, ip0_mode);
> if ((++attempts % 5) == 0)
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index 34b91b789702..3e5b8574818e 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -518,7 +518,7 @@ xfs_inode_item_push(
> * The buffer containing this item failed to be written back
> * previously. Resubmit the buffer for IO.
> */
> - if (lip->li_flags & XFS_LI_FAILED) {
> + if (test_bit(XFS_LI_FAILED, &lip->li_flags)) {
> if (!xfs_buf_trylock(bp))
> return XFS_ITEM_LOCKED;
>
> @@ -729,14 +729,14 @@ xfs_iflush_done(
> */
> iip = INODE_ITEM(blip);
> if ((iip->ili_logged && blip->li_lsn == iip->ili_flush_lsn) ||
> - (blip->li_flags & XFS_LI_FAILED))
> + test_bit(XFS_LI_FAILED, &blip->li_flags))
> need_ail++;
> }
>
> /* make sure we capture the state of the initial inode. */
> iip = INODE_ITEM(lip);
> if ((iip->ili_logged && lip->li_lsn == iip->ili_flush_lsn) ||
> - lip->li_flags & XFS_LI_FAILED)
> + test_bit(XFS_LI_FAILED, &lip->li_flags))
> need_ail++;
>
> /*
> @@ -803,7 +803,7 @@ xfs_iflush_abort(
> xfs_inode_log_item_t *iip = ip->i_itemp;
>
> if (iip) {
> - if (iip->ili_item.li_flags & XFS_LI_IN_AIL) {
> + if (test_bit(XFS_LI_IN_AIL, &iip->ili_item.li_flags)) {
> xfs_trans_ail_remove(&iip->ili_item,
> stale ? SHUTDOWN_LOG_IO_ERROR :
> SHUTDOWN_CORRUPT_INCORE);
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 2fcd9ed5d075..e427864434c1 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -2132,7 +2132,7 @@ xlog_print_trans(
>
> xfs_warn(mp, "log item: ");
> xfs_warn(mp, " type = 0x%x", lip->li_type);
> - xfs_warn(mp, " flags = 0x%x", lip->li_flags);
> + xfs_warn(mp, " flags = 0x%lx", lip->li_flags);
> if (!lv)
> continue;
> xfs_warn(mp, " niovecs = %d", lv->lv_niovecs);
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index ec39ae274c78..4438fc446d18 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -173,7 +173,7 @@ xfs_qm_dqpurge(
>
> ASSERT(atomic_read(&dqp->q_pincount) == 0);
> ASSERT(XFS_FORCED_SHUTDOWN(mp) ||
> - !(dqp->q_logitem.qli_item.li_flags & XFS_LI_IN_AIL));
> + !test_bit(XFS_LI_IN_AIL, &dqp->q_logitem.qli_item.li_flags));
>
> xfs_dqfunlock(dqp);
> xfs_dqunlock(dqp);
> diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
> index 15c9393dd7a7..e5866b714d5f 100644
> --- a/fs/xfs/xfs_refcount_item.c
> +++ b/fs/xfs/xfs_refcount_item.c
> @@ -159,7 +159,7 @@ STATIC void
> xfs_cui_item_unlock(
> struct xfs_log_item *lip)
> {
> - if (lip->li_flags & XFS_LI_ABORTED)
> + if (test_bit(XFS_LI_ABORTED, &lip->li_flags))
> xfs_cui_release(CUI_ITEM(lip));
> }
>
> @@ -310,7 +310,7 @@ xfs_cud_item_unlock(
> {
> struct xfs_cud_log_item *cudp = CUD_ITEM(lip);
>
> - if (lip->li_flags & XFS_LI_ABORTED) {
> + if (test_bit(XFS_LI_ABORTED, &lip->li_flags)) {
> xfs_cui_release(cudp->cud_cuip);
> kmem_zone_free(xfs_cud_zone, cudp);
> }
> diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
> index 06a07846c9b3..e5b5b3e7ef82 100644
> --- a/fs/xfs/xfs_rmap_item.c
> +++ b/fs/xfs/xfs_rmap_item.c
> @@ -158,7 +158,7 @@ STATIC void
> xfs_rui_item_unlock(
> struct xfs_log_item *lip)
> {
> - if (lip->li_flags & XFS_LI_ABORTED)
> + if (test_bit(XFS_LI_ABORTED, &lip->li_flags))
> xfs_rui_release(RUI_ITEM(lip));
> }
>
> @@ -331,7 +331,7 @@ xfs_rud_item_unlock(
> {
> struct xfs_rud_log_item *rudp = RUD_ITEM(lip);
>
> - if (lip->li_flags & XFS_LI_ABORTED) {
> + if (test_bit(XFS_LI_ABORTED, &lip->li_flags)) {
> xfs_rui_release(rudp->rud_ruip);
> kmem_zone_free(xfs_rud_zone, rudp);
> }
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 8955254b900e..7f4d961fae12 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -442,7 +442,7 @@ DECLARE_EVENT_CLASS(xfs_buf_item_class,
> __field(int, bli_refcount)
> __field(unsigned, bli_flags)
> __field(void *, li_desc)
> - __field(unsigned, li_flags)
> + __field(unsigned long, li_flags)
> ),
> TP_fast_assign(
> __entry->dev = bip->bli_buf->b_target->bt_dev;
> @@ -1018,7 +1018,7 @@ DECLARE_EVENT_CLASS(xfs_log_item_class,
> __field(dev_t, dev)
> __field(void *, lip)
> __field(uint, type)
> - __field(uint, flags)
> + __field(unsigned long, flags)
> __field(xfs_lsn_t, lsn)
> ),
> TP_fast_assign(
> @@ -1070,7 +1070,7 @@ DECLARE_EVENT_CLASS(xfs_ail_class,
> __field(dev_t, dev)
> __field(void *, lip)
> __field(uint, type)
> - __field(uint, flags)
> + __field(unsigned long, flags)
> __field(xfs_lsn_t, old_lsn)
> __field(xfs_lsn_t, new_lsn)
> ),
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index d6d8f9d129a7..c6a2a3cb9df5 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -790,7 +790,7 @@ xfs_trans_free_items(
> if (commit_lsn != NULLCOMMITLSN)
> lip->li_ops->iop_committing(lip, commit_lsn);
> if (abort)
> - lip->li_flags |= XFS_LI_ABORTED;
> + set_bit(XFS_LI_ABORTED, &lip->li_flags);
> lip->li_ops->iop_unlock(lip);
>
> xfs_trans_free_item_desc(lidp);
> @@ -861,7 +861,7 @@ xfs_trans_committed_bulk(
> xfs_lsn_t item_lsn;
>
> if (aborted)
> - lip->li_flags |= XFS_LI_ABORTED;
> + set_bit(XFS_LI_ABORTED, &lip->li_flags);
> item_lsn = lip->li_ops->iop_committed(lip, commit_lsn);
>
> /* item_lsn of -1 means the item needs no further processing */
> diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
> index 9d542dfe0052..51145ddf7e5b 100644
> --- a/fs/xfs/xfs_trans.h
> +++ b/fs/xfs/xfs_trans.h
> @@ -48,7 +48,7 @@ typedef struct xfs_log_item {
> struct xfs_mount *li_mountp; /* ptr to fs mount */
> struct xfs_ail *li_ailp; /* ptr to AIL */
> uint li_type; /* item type */
> - uint li_flags; /* misc flags */
> + unsigned long li_flags; /* misc flags */
> struct xfs_buf *li_buf; /* real buffer pointer */
> struct list_head li_bio_list; /* buffer item list */
> void (*li_cb)(struct xfs_buf *,
> @@ -64,14 +64,19 @@ typedef struct xfs_log_item {
> xfs_lsn_t li_seq; /* CIL commit seq */
> } xfs_log_item_t;
>
> -#define XFS_LI_IN_AIL 0x1
> -#define XFS_LI_ABORTED 0x2
> -#define XFS_LI_FAILED 0x4
> +/*
> + * li_flags use the (set/test/clear)_bit atomic interfaces because updates can
> + * race with each other and we don't want to have to use the AIL lock to
> + * serialise all updates.
> + */
> +#define XFS_LI_IN_AIL 0
> +#define XFS_LI_ABORTED 1
> +#define XFS_LI_FAILED 2
>
> #define XFS_LI_FLAGS \
> - { XFS_LI_IN_AIL, "IN_AIL" }, \
> - { XFS_LI_ABORTED, "ABORTED" }, \
> - { XFS_LI_FAILED, "FAILED" }
> + { (1 << XFS_LI_IN_AIL), "IN_AIL" }, \
> + { (1 << XFS_LI_ABORTED), "ABORTED" }, \
> + { (1 << XFS_LI_FAILED), "FAILED" }
>
> struct xfs_item_ops {
> void (*iop_size)(xfs_log_item_t *, int *, int *);
> diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
> index d4a2445215e6..50611d2bcbc2 100644
> --- a/fs/xfs/xfs_trans_ail.c
> +++ b/fs/xfs/xfs_trans_ail.c
> @@ -46,7 +46,7 @@ xfs_ail_check(
> /*
> * Check the next and previous entries are valid.
> */
> - ASSERT((lip->li_flags & XFS_LI_IN_AIL) != 0);
> + ASSERT(test_bit(XFS_LI_IN_AIL, &lip->li_flags));
> prev_lip = list_entry(lip->li_ail.prev, xfs_log_item_t, li_ail);
> if (&prev_lip->li_ail != &ailp->ail_head)
> ASSERT(XFS_LSN_CMP(prev_lip->li_lsn, lip->li_lsn) <= 0);
> @@ -684,7 +684,7 @@ xfs_trans_ail_update_bulk(
>
> for (i = 0; i < nr_items; i++) {
> struct xfs_log_item *lip = log_items[i];
> - if (lip->li_flags & XFS_LI_IN_AIL) {
> + if (test_and_set_bit(XFS_LI_IN_AIL, &lip->li_flags)) {
> /* check if we really need to move the item */
> if (XFS_LSN_CMP(lsn, lip->li_lsn) <= 0)
> continue;
> @@ -694,7 +694,6 @@ xfs_trans_ail_update_bulk(
> if (mlip == lip)
> mlip_changed = 1;
> } else {
> - lip->li_flags |= XFS_LI_IN_AIL;
> trace_xfs_ail_insert(lip, 0, lsn);
> }
> lip->li_lsn = lsn;
> @@ -725,7 +724,7 @@ xfs_ail_delete_one(
> trace_xfs_ail_delete(lip, mlip->li_lsn, lip->li_lsn);
> xfs_ail_delete(ailp, lip);
> xfs_clear_li_failed(lip);
> - lip->li_flags &= ~XFS_LI_IN_AIL;
> + clear_bit(XFS_LI_IN_AIL, &lip->li_flags);
> lip->li_lsn = 0;
>
> return mlip == lip;
> @@ -761,7 +760,7 @@ xfs_trans_ail_delete(
> struct xfs_mount *mp = ailp->ail_mount;
> bool mlip_changed;
>
> - if (!(lip->li_flags & XFS_LI_IN_AIL)) {
> + if (!test_bit(XFS_LI_IN_AIL, &lip->li_flags)) {
> spin_unlock(&ailp->ail_lock);
> if (!XFS_FORCED_SHUTDOWN(mp)) {
> xfs_alert_tag(mp, XFS_PTAG_AILDELETE,
> diff --git a/fs/xfs/xfs_trans_buf.c b/fs/xfs/xfs_trans_buf.c
> index a5d9dfc45d98..0081e9b3decf 100644
> --- a/fs/xfs/xfs_trans_buf.c
> +++ b/fs/xfs/xfs_trans_buf.c
> @@ -442,7 +442,7 @@ xfs_trans_brelse(
> ASSERT(bp->b_pincount == 0);
> ***/
> ASSERT(atomic_read(&bip->bli_refcount) == 0);
> - ASSERT(!(bip->bli_item.li_flags & XFS_LI_IN_AIL));
> + ASSERT(!test_bit(XFS_LI_IN_AIL, &bip->bli_item.li_flags));
> ASSERT(!(bip->bli_flags & XFS_BLI_INODE_ALLOC_BUF));
> xfs_buf_item_relse(bp);
> }
> diff --git a/fs/xfs/xfs_trans_priv.h b/fs/xfs/xfs_trans_priv.h
> index be24b0c8a332..43f773297b9d 100644
> --- a/fs/xfs/xfs_trans_priv.h
> +++ b/fs/xfs/xfs_trans_priv.h
> @@ -119,7 +119,7 @@ xfs_trans_ail_remove(
>
> spin_lock(&ailp->ail_lock);
> /* xfs_trans_ail_delete() drops the AIL lock */
> - if (lip->li_flags & XFS_LI_IN_AIL)
> + if (test_bit(XFS_LI_IN_AIL, &lip->li_flags))
> xfs_trans_ail_delete(ailp, lip, shutdown_type);
> else
> spin_unlock(&ailp->ail_lock);
> @@ -171,11 +171,10 @@ xfs_clear_li_failed(
> {
> struct xfs_buf *bp = lip->li_buf;
>
> - ASSERT(lip->li_flags & XFS_LI_IN_AIL);
> + ASSERT(test_bit(XFS_LI_IN_AIL, &lip->li_flags));
> lockdep_assert_held(&lip->li_ailp->ail_lock);
>
> - if (lip->li_flags & XFS_LI_FAILED) {
> - lip->li_flags &= ~XFS_LI_FAILED;
> + if (test_and_clear_bit(XFS_LI_FAILED, &lip->li_flags)) {
> lip->li_buf = NULL;
> xfs_buf_rele(bp);
> }
> @@ -188,9 +187,8 @@ xfs_set_li_failed(
> {
> lockdep_assert_held(&lip->li_ailp->ail_lock);
>
> - if (!(lip->li_flags & XFS_LI_FAILED)) {
> + if (!test_and_set_bit(XFS_LI_FAILED, &lip->li_flags)) {
> xfs_buf_hold(bp);
> - lip->li_flags |= XFS_LI_FAILED;
> lip->li_buf = bp;
> }
> }
> --
> 2.17.0
>
> --
> 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
next prev parent reply other threads:[~2018-05-09 14:51 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-08 3:41 [PATCH 0/9 v2] xfs: log item and transaction cleanups Dave Chinner
2018-05-08 3:41 ` [PATCH 1/9] xfs: log item flags are racy Dave Chinner
2018-05-09 14:51 ` Darrick J. Wong [this message]
2018-05-08 3:41 ` [PATCH 2/9] xfs: add tracing to high level transaction operations Dave Chinner
2018-05-09 14:51 ` Darrick J. Wong
2018-05-08 3:41 ` [PATCH 3/9] xfs: adder caller IP to xfs_defer* tracepoints Dave Chinner
2018-05-09 14:52 ` Darrick J. Wong
2018-05-08 3:41 ` [PATCH 4/9] xfs: don't assert fail with AIL lock held Dave Chinner
2018-05-08 14:18 ` Brian Foster
2018-05-09 6:13 ` Christoph Hellwig
2018-05-09 14:52 ` Darrick J. Wong
2018-05-08 3:41 ` [PATCH 5/9] xfs: fix double ijoin in xfs_inactive_symlink_rmt() Dave Chinner
2018-05-08 14:18 ` Brian Foster
2018-05-09 0:24 ` Dave Chinner
2018-05-09 10:10 ` Brian Foster
2018-05-09 15:02 ` Darrick J. Wong
2018-05-11 2:04 ` Dave Chinner
2018-05-11 13:24 ` Brian Foster
2018-05-12 2:00 ` Dave Chinner
2018-05-12 14:17 ` Brian Foster
2018-05-08 3:41 ` [PATCH 6/9] xfs: fix double ijoin in xfs_reflink_cancel_cow_range Dave Chinner
2018-05-08 14:18 ` Brian Foster
2018-05-09 15:17 ` Darrick J. Wong
2018-05-08 3:42 ` [PATCH 7/9] xfs: fix double ijoin in xfs_reflink_clear_inode_flag() Dave Chinner
2018-05-08 14:18 ` Brian Foster
2018-05-09 0:40 ` Dave Chinner
2018-05-09 10:12 ` Brian Foster
2018-05-09 15:19 ` Darrick J. Wong
2018-05-08 3:42 ` [PATCH 8/9] xfs: add some more debug checks to buffer log item reuse Dave Chinner
2018-05-08 14:18 ` Brian Foster
2018-05-09 15:19 ` Darrick J. Wong
2018-05-08 3:42 ` [PATCH 9/9] xfs: get rid of the log item descriptor Dave Chinner
2018-05-08 14:18 ` Brian Foster
2018-05-09 6:27 ` Christoph Hellwig
2018-05-09 15:19 ` Darrick J. Wong
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=20180509145142.GI11261@magnolia \
--to=darrick.wong@oracle.com \
--cc=david@fromorbit.com \
--cc=linux-xfs@vger.kernel.org \
/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.