| Message ID | 146907698690.25461.7316591529865921141.stgit@birch.djwong.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, Jul 20, 2016 at 09:56:26PM -0700, Darrick J. Wong wrote: > When we're deleting realtime extents, we need to lock the summary > inode in case we need to update the summary info to prevent an assert > on the rsumip inode lock on a debug kernel. While we're at it, fix > the locking annotations so that we avoid triggering lockdep warnings. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- I haven't tried the rt stuff in quite some time (and even then never really played with it much). What's the assert that fails? Brian > fs/xfs/libxfs/xfs_bmap.c | 4 +++- > fs/xfs/xfs_bmap_util.c | 4 ++-- > 2 files changed, 5 insertions(+), 3 deletions(-) > > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > index 2f2c85c..c5981f4 100644 > --- a/fs/xfs/libxfs/xfs_bmap.c > +++ b/fs/xfs/libxfs/xfs_bmap.c > @@ -5179,8 +5179,10 @@ xfs_bunmapi( > /* > * Synchronize by locking the bitmap inode. > */ > - xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL); > + xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL|XFS_ILOCK_RTBITMAP); > xfs_trans_ijoin(tp, mp->m_rbmip, XFS_ILOCK_EXCL); > + xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL|XFS_ILOCK_RTSUM); > + xfs_trans_ijoin(tp, mp->m_rsumip, XFS_ILOCK_EXCL); > } > > extno = 0; > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c > index cd4a850..998c3e6 100644 > --- a/fs/xfs/xfs_bmap_util.c > +++ b/fs/xfs/xfs_bmap_util.c > @@ -214,9 +214,9 @@ xfs_bmap_rtalloc( > /* > * Lock out modifications to both the RT bitmap and summary inodes > */ > - xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL); > + xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL|XFS_ILOCK_RTBITMAP); > xfs_trans_ijoin(ap->tp, mp->m_rbmip, XFS_ILOCK_EXCL); > - xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL); > + xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL|XFS_ILOCK_RTSUM); > xfs_trans_ijoin(ap->tp, mp->m_rsumip, XFS_ILOCK_EXCL); > > /* > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs
On Tue, Jul 26, 2016 at 12:36:07PM -0400, Brian Foster wrote: > On Wed, Jul 20, 2016 at 09:56:26PM -0700, Darrick J. Wong wrote: > > When we're deleting realtime extents, we need to lock the summary > > inode in case we need to update the summary info to prevent an assert > > on the rsumip inode lock on a debug kernel. While we're at it, fix > > the locking annotations so that we avoid triggering lockdep warnings. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > I haven't tried the rt stuff in quite some time (and even then never > really played with it much). What's the assert that fails? The first assert that I noticed is in xfs_rtfree_extent(): ASSERT(xfs_isilocked(mp->m_rbmip, XFS_ILOCK_EXCL)); I then noticed that we weren't locking the summary inode either, which triggers this assert in xfs_bmapi_read(): ASSERT(xfs_isilocked(ip, XFS_ILOCK_SHARED|XFS_ILOCK_EXCL)); which has this call stack: xfs_rtfree_extent -> xfs_rtmodify_summary -> xfs_rtmodify_summary_int -> xfs_rtbuf_get -> xfs_bmapi_read. --D > > Brian > > > fs/xfs/libxfs/xfs_bmap.c | 4 +++- > > fs/xfs/xfs_bmap_util.c | 4 ++-- > > 2 files changed, 5 insertions(+), 3 deletions(-) > > > > > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > > index 2f2c85c..c5981f4 100644 > > --- a/fs/xfs/libxfs/xfs_bmap.c > > +++ b/fs/xfs/libxfs/xfs_bmap.c > > @@ -5179,8 +5179,10 @@ xfs_bunmapi( > > /* > > * Synchronize by locking the bitmap inode. > > */ > > - xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL); > > + xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL|XFS_ILOCK_RTBITMAP); > > xfs_trans_ijoin(tp, mp->m_rbmip, XFS_ILOCK_EXCL); > > + xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL|XFS_ILOCK_RTSUM); > > + xfs_trans_ijoin(tp, mp->m_rsumip, XFS_ILOCK_EXCL); > > } > > > > extno = 0; > > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c > > index cd4a850..998c3e6 100644 > > --- a/fs/xfs/xfs_bmap_util.c > > +++ b/fs/xfs/xfs_bmap_util.c > > @@ -214,9 +214,9 @@ xfs_bmap_rtalloc( > > /* > > * Lock out modifications to both the RT bitmap and summary inodes > > */ > > - xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL); > > + xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL|XFS_ILOCK_RTBITMAP); > > xfs_trans_ijoin(ap->tp, mp->m_rbmip, XFS_ILOCK_EXCL); > > - xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL); > > + xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL|XFS_ILOCK_RTSUM); > > xfs_trans_ijoin(ap->tp, mp->m_rsumip, XFS_ILOCK_EXCL); > > > > /* > > > > _______________________________________________ > > xfs mailing list > > xfs@oss.sgi.com > > http://oss.sgi.com/mailman/listinfo/xfs
On Wed, Jul 20, 2016 at 09:56:26PM -0700, Darrick J. Wong wrote: > When we're deleting realtime extents, we need to lock the summary > inode in case we need to update the summary info to prevent an assert > on the rsumip inode lock on a debug kernel. While we're at it, fix > the locking annotations so that we avoid triggering lockdep warnings. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Looks fine, Reviewed-by: Christoph Hellwig <hch@lst.de>
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index 2f2c85c..c5981f4 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -5179,8 +5179,10 @@ xfs_bunmapi( /* * Synchronize by locking the bitmap inode. */ - xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL); + xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL|XFS_ILOCK_RTBITMAP); xfs_trans_ijoin(tp, mp->m_rbmip, XFS_ILOCK_EXCL); + xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL|XFS_ILOCK_RTSUM); + xfs_trans_ijoin(tp, mp->m_rsumip, XFS_ILOCK_EXCL); } extno = 0; diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c index cd4a850..998c3e6 100644 --- a/fs/xfs/xfs_bmap_util.c +++ b/fs/xfs/xfs_bmap_util.c @@ -214,9 +214,9 @@ xfs_bmap_rtalloc( /* * Lock out modifications to both the RT bitmap and summary inodes */ - xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL); + xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL|XFS_ILOCK_RTBITMAP); xfs_trans_ijoin(ap->tp, mp->m_rbmip, XFS_ILOCK_EXCL); - xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL); + xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL|XFS_ILOCK_RTSUM); xfs_trans_ijoin(ap->tp, mp->m_rsumip, XFS_ILOCK_EXCL); /*
When we're deleting realtime extents, we need to lock the summary inode in case we need to update the summary info to prevent an assert on the rsumip inode lock on a debug kernel. While we're at it, fix the locking annotations so that we avoid triggering lockdep warnings. Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- fs/xfs/libxfs/xfs_bmap.c | 4 +++- fs/xfs/xfs_bmap_util.c | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-)