| Message ID | 1496833017-63787-1-git-send-email-bfoster@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Jun 07, 2017 at 06:56:57AM -0400, Brian Foster wrote: > The 0-day kernel test robot reports assertion failures on > !CONFIG_SMP kernels due to failed spin_is_locked() checks. As it > turns out, spin_is_locked() is hardcoded to return zero on > !CONFIG_SMP kernels and so this function cannot be relied on to > verify spinlock state in this configuration. > > To avoid this problem, update the associated asserts to fail only > when CONFIG_SMP is enabled in the kernel. Note that this is not > necessary for one assert that expects a zero return from > spin_is_locked(). Update this assert anyways for consistency and > future proofing. Just switch to lockdep_assert_held instead of this mess.. -- 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
On Wed, Jun 07, 2017 at 04:21:40AM -0700, Christoph Hellwig wrote: > On Wed, Jun 07, 2017 at 06:56:57AM -0400, Brian Foster wrote: > > The 0-day kernel test robot reports assertion failures on > > !CONFIG_SMP kernels due to failed spin_is_locked() checks. As it > > turns out, spin_is_locked() is hardcoded to return zero on > > !CONFIG_SMP kernels and so this function cannot be relied on to > > verify spinlock state in this configuration. > > > > To avoid this problem, update the associated asserts to fail only > > when CONFIG_SMP is enabled in the kernel. Note that this is not > > necessary for one assert that expects a zero return from > > spin_is_locked(). Update this assert anyways for consistency and > > future proofing. > > Just switch to lockdep_assert_held instead of this mess.. Seems reasonable, I wasn't aware of that. What about the !spin_is_locked() case? Do you want to drop it? Brian > -- > 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 -- 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
On Wed, Jun 07, 2017 at 07:54:54AM -0400, Brian Foster wrote: > > Just switch to lockdep_assert_held instead of this mess.. > > Seems reasonable, I wasn't aware of that. What about the > !spin_is_locked() case? Do you want to drop it? Yes, I would just drop it. -- 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
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c index 07b77b7..d60f917 100644 --- a/fs/xfs/xfs_buf.c +++ b/fs/xfs/xfs_buf.c @@ -117,7 +117,7 @@ static inline void __xfs_buf_ioacct_dec( struct xfs_buf *bp) { - ASSERT(spin_is_locked(&bp->b_lock)); + ASSERT(spin_is_locked(&bp->b_lock) || !IS_ENABLED(CONFIG_SMP)); if (bp->b_state & XFS_BSTATE_IN_FLIGHT) { bp->b_state &= ~XFS_BSTATE_IN_FLIGHT; diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c index f61c84f8..5cecb36 100644 --- a/fs/xfs/xfs_icache.c +++ b/fs/xfs/xfs_icache.c @@ -66,7 +66,7 @@ xfs_inode_alloc( XFS_STATS_INC(mp, vn_active); ASSERT(atomic_read(&ip->i_pincount) == 0); - ASSERT(!spin_is_locked(&ip->i_flags_lock)); + ASSERT(!spin_is_locked(&ip->i_flags_lock) || !IS_ENABLED(CONFIG_SMP)); ASSERT(!xfs_isiflocked(ip)); ASSERT(ip->i_ino == 0); @@ -190,7 +190,7 @@ xfs_perag_set_reclaim_tag( { struct xfs_mount *mp = pag->pag_mount; - ASSERT(spin_is_locked(&pag->pag_ici_lock)); + ASSERT(spin_is_locked(&pag->pag_ici_lock) || !IS_ENABLED(CONFIG_SMP)); if (pag->pag_ici_reclaimable++) return; @@ -212,7 +212,7 @@ xfs_perag_clear_reclaim_tag( { struct xfs_mount *mp = pag->pag_mount; - ASSERT(spin_is_locked(&pag->pag_ici_lock)); + ASSERT(spin_is_locked(&pag->pag_ici_lock) || !IS_ENABLED(CONFIG_SMP)); if (--pag->pag_ici_reclaimable) return;
The 0-day kernel test robot reports assertion failures on !CONFIG_SMP kernels due to failed spin_is_locked() checks. As it turns out, spin_is_locked() is hardcoded to return zero on !CONFIG_SMP kernels and so this function cannot be relied on to verify spinlock state in this configuration. To avoid this problem, update the associated asserts to fail only when CONFIG_SMP is enabled in the kernel. Note that this is not necessary for one assert that expects a zero return from spin_is_locked(). Update this assert anyways for consistency and future proofing. Reported-by: kbuild test robot <fengguang.wu@intel.com> Signed-off-by: Brian Foster <bfoster@redhat.com> --- Darrick, FYI, this one applies on top of Linus' latest 4.12.0-rc4 tree as that is where the particular assert that the 0-day test tripped over is. Otherwise this suppresses these assert failures on some quick !SMP tests and survives some overnight sanity testing on my normal (SMP) test box. Thanks! Brian fs/xfs/xfs_buf.c | 2 +- fs/xfs/xfs_icache.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)