* [PATCH] xfs: fix infinite loop by detaching the group/project hints from user dquot @ 2013-11-19 8:42 Jeff Liu 2013-11-19 11:12 ` Dave Chinner 0 siblings, 1 reply; 3+ messages in thread From: Jeff Liu @ 2013-11-19 8:42 UTC (permalink / raw) To: xfs From: Jie Liu <jeff.liu@oracle.com> xfs_quota(8) will hang up if trying to turn group quota or project quota before the user quota is off, this could be 100% reproduced by: # mount -ouquota,gquota /dev/sda7 /xfs # mkdir /xfs/test # xfs_quota -xc 'off -g' /xfs # echo w > /proc/sysrq-trigger SysRq : Show Blocked State task PC stack pid father xfs_quota D 0000000000000000 0 27574 2551 0x00000000 [snip] Call Trace: [<ffffffff81aaa21d>] schedule+0xad/0xc0 [<ffffffff81aa327e>] schedule_timeout+0x35e/0x3c0 [<ffffffff8114b506>] ? mark_held_locks+0x176/0x1c0 [<ffffffff810ad6c0>] ? call_timer_fn+0x2c0/0x2c0 [<ffffffffa0c25380>] ? xfs_qm_shrink_count+0x30/0x30 [xfs] [<ffffffff81aa3306>] schedule_timeout_uninterruptible+0x26/0x30 [<ffffffffa0c26155>] xfs_qm_dquot_walk+0x235/0x260 [xfs] [<ffffffffa0c059d8>] ? xfs_perag_get+0x1d8/0x2d0 [xfs] [<ffffffffa0c05805>] ? xfs_perag_get+0x5/0x2d0 [xfs] [<ffffffffa0b7707e>] ? xfs_inode_ag_iterator+0xae/0xf0 [xfs] [<ffffffffa0c22280>] ? xfs_trans_free_dqinfo+0x50/0x50 [xfs] [<ffffffffa0b7709f>] ? xfs_inode_ag_iterator+0xcf/0xf0 [xfs] [<ffffffffa0c261e6>] xfs_qm_dqpurge_all+0x66/0xb0 [xfs] [<ffffffffa0c2497a>] xfs_qm_scall_quotaoff+0x20a/0x5f0 [xfs] [<ffffffffa0c2b8f6>] xfs_fs_set_xstate+0x136/0x180 [xfs] [<ffffffff8136cf7a>] do_quotactl+0x53a/0x6b0 [<ffffffff812fba4b>] ? iput+0x5b/0x90 [<ffffffff8136d257>] SyS_quotactl+0x167/0x1d0 [<ffffffff814cf2ee>] ? trace_hardirqs_on_thunk+0x3a/0x3f [<ffffffff81abcd19>] system_call_fastpath+0x16/0x1b It's fine if we turn user quota off at first, then turn off other kind of quotas if they are enabled since the group/project dquot refcount is decreased to zero once the user quota if off. Otherwse, those dquots refcount is non-zero due to the user dquot maybe refer to them as hint(s). Hence, above operation hit an infinite loop at xfs_qm_dquot_walk() to purge dquot cache. This problem has been around since Linux 3.4, it was introduced by: b84a3a96751f93071c1863f2962273973c8b8f5e xfs: remove the per-filesystem list of dquots Originally we will release the group dquot pointers because the user dquots maybe carrying around as a hint via xfs_qm_detach_gdquots(). However, with this change, there is no such work to be done before purge group/project dquot cache. This fix introduce a similar routine to the old xfs_qm_detach_gdquots(), it will detach the group/project hints by searching the user dquot radix tree and release those hints if they are there. Signed-off-by: Jie Liu <jeff.liu@oracle.com> --- fs/xfs/xfs_qm.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 77 insertions(+), 2 deletions(-) diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c index 14a4996..410adf4 100644 --- a/fs/xfs/xfs_qm.c +++ b/fs/xfs/xfs_qm.c @@ -60,6 +60,77 @@ STATIC void xfs_qm_dqfree_one(struct xfs_dquot *dqp); */ #define XFS_DQ_LOOKUP_BATCH 32 +/* + * Release the group or project dquot pointers the user dquots may be + * carrying around as a hint. + */ +STATIC void +xfs_qm_dqdetach_hint( + struct xfs_mount *mp, + int type) +{ + struct xfs_quotainfo *qi = mp->m_quotainfo; + struct radix_tree_root *tree = xfs_dquot_tree(qi, XFS_DQ_USER); + uint32_t next_index; + int skipped; + int nr_found; + + ASSERT(type == XFS_DQ_GROUP || type == XFS_DQ_PROJ); + +restart: + next_index = 0; + skipped = 0; + nr_found = 0; + + while (1) { + struct xfs_dquot *batch[XFS_DQ_LOOKUP_BATCH]; + int i; + + mutex_lock(&qi->qi_tree_lock); + nr_found = radix_tree_gang_lookup(tree, (void **)batch, + next_index, XFS_DQ_LOOKUP_BATCH); + if (!nr_found) { + mutex_unlock(&qi->qi_tree_lock); + break; + } + + for (i = 0; i < nr_found; i++) { + struct xfs_dquot *dqp = batch[i]; + struct xfs_dquot *dqhintp; + + next_index = be32_to_cpu(dqp->q_core.d_id) + 1; + + xfs_dqlock(dqp); + if (dqp->dq_flags & XFS_DQ_FREEING) { + xfs_dqunlock(dqp); + skipped++; + continue; + } + + if (type == XFS_DQ_GROUP) { + dqhintp = dqp->q_gdquot; + if (dqhintp) + dqp->q_gdquot = NULL; + } else { + dqhintp = dqp->q_pdquot; + if (dqhintp) + dqp->q_pdquot = NULL; + } + xfs_dqunlock(dqp); + + if (dqhintp) + xfs_qm_dqrele(dqhintp); + } + + mutex_unlock(&qi->qi_tree_lock); + } + + if (skipped) { + delay(1); + goto restart; + } +} + STATIC int xfs_qm_dquot_walk( struct xfs_mount *mp, @@ -224,10 +295,14 @@ xfs_qm_dqpurge_all( { if (flags & XFS_QMOPT_UQUOTA) xfs_qm_dquot_walk(mp, XFS_DQ_USER, xfs_qm_dqpurge, NULL); - if (flags & XFS_QMOPT_GQUOTA) + if (flags & XFS_QMOPT_GQUOTA) { + xfs_qm_dqdetach_hint(mp, XFS_DQ_GROUP); xfs_qm_dquot_walk(mp, XFS_DQ_GROUP, xfs_qm_dqpurge, NULL); - if (flags & XFS_QMOPT_PQUOTA) + } + if (flags & XFS_QMOPT_PQUOTA) { + xfs_qm_dqdetach_hint(mp, XFS_DQ_PROJ); xfs_qm_dquot_walk(mp, XFS_DQ_PROJ, xfs_qm_dqpurge, NULL); + } } /* -- 1.7.9.5 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] xfs: fix infinite loop by detaching the group/project hints from user dquot 2013-11-19 8:42 [PATCH] xfs: fix infinite loop by detaching the group/project hints from user dquot Jeff Liu @ 2013-11-19 11:12 ` Dave Chinner 2013-11-20 5:16 ` Jeff Liu 0 siblings, 1 reply; 3+ messages in thread From: Dave Chinner @ 2013-11-19 11:12 UTC (permalink / raw) To: Jeff Liu; +Cc: xfs On Tue, Nov 19, 2013 at 04:42:30PM +0800, Jeff Liu wrote: > From: Jie Liu <jeff.liu@oracle.com> > > xfs_quota(8) will hang up if trying to turn group quota or project > quota before the user quota is off, this could be 100% reproduced by: > # mount -ouquota,gquota /dev/sda7 /xfs > # mkdir /xfs/test > # xfs_quota -xc 'off -g' /xfs > # echo w > /proc/sysrq-trigger > SysRq : Show Blocked State > task PC stack pid father > xfs_quota D 0000000000000000 0 27574 2551 0x00000000 > [snip] > Call Trace: > [<ffffffff81aaa21d>] schedule+0xad/0xc0 > [<ffffffff81aa327e>] schedule_timeout+0x35e/0x3c0 > [<ffffffff8114b506>] ? mark_held_locks+0x176/0x1c0 > [<ffffffff810ad6c0>] ? call_timer_fn+0x2c0/0x2c0 > [<ffffffffa0c25380>] ? xfs_qm_shrink_count+0x30/0x30 [xfs] > [<ffffffff81aa3306>] schedule_timeout_uninterruptible+0x26/0x30 > [<ffffffffa0c26155>] xfs_qm_dquot_walk+0x235/0x260 [xfs] > [<ffffffffa0c059d8>] ? xfs_perag_get+0x1d8/0x2d0 [xfs] > [<ffffffffa0c05805>] ? xfs_perag_get+0x5/0x2d0 [xfs] > [<ffffffffa0b7707e>] ? xfs_inode_ag_iterator+0xae/0xf0 [xfs] > [<ffffffffa0c22280>] ? xfs_trans_free_dqinfo+0x50/0x50 [xfs] > [<ffffffffa0b7709f>] ? xfs_inode_ag_iterator+0xcf/0xf0 [xfs] > [<ffffffffa0c261e6>] xfs_qm_dqpurge_all+0x66/0xb0 [xfs] > [<ffffffffa0c2497a>] xfs_qm_scall_quotaoff+0x20a/0x5f0 [xfs] > [<ffffffffa0c2b8f6>] xfs_fs_set_xstate+0x136/0x180 [xfs] > [<ffffffff8136cf7a>] do_quotactl+0x53a/0x6b0 > [<ffffffff812fba4b>] ? iput+0x5b/0x90 > [<ffffffff8136d257>] SyS_quotactl+0x167/0x1d0 > [<ffffffff814cf2ee>] ? trace_hardirqs_on_thunk+0x3a/0x3f > [<ffffffff81abcd19>] system_call_fastpath+0x16/0x1b > > It's fine if we turn user quota off at first, then turn off other > kind of quotas if they are enabled since the group/project dquot > refcount is decreased to zero once the user quota if off. Otherwse, > those dquots refcount is non-zero due to the user dquot maybe refer > to them as hint(s). Hence, above operation hit an infinite loop at > xfs_qm_dquot_walk() to purge dquot cache. > > This problem has been around since Linux 3.4, it was introduced by: > b84a3a96751f93071c1863f2962273973c8b8f5e > xfs: remove the per-filesystem list of dquots > > Originally we will release the group dquot pointers because the user > dquots maybe carrying around as a hint via xfs_qm_detach_gdquots(). > However, with this change, there is no such work to be done before > purge group/project dquot cache. > > This fix introduce a similar routine to the old xfs_qm_detach_gdquots(), > it will detach the group/project hints by searching the user dquot radix > tree and release those hints if they are there. Ok, we do get stuck in a loop in this case. We need an xfstest to exercise this - removing group and project quotas while user quotas are left on and fsstress is active - should be pretty easy to do. More comments below. > Signed-off-by: Jie Liu <jeff.liu@oracle.com> > --- > fs/xfs/xfs_qm.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 77 insertions(+), 2 deletions(-) > > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c > index 14a4996..410adf4 100644 > --- a/fs/xfs/xfs_qm.c > +++ b/fs/xfs/xfs_qm.c > @@ -60,6 +60,77 @@ STATIC void xfs_qm_dqfree_one(struct xfs_dquot *dqp); > */ > #define XFS_DQ_LOOKUP_BATCH 32 > > +/* > + * Release the group or project dquot pointers the user dquots may be > + * carrying around as a hint. > + */ > +STATIC void > +xfs_qm_dqdetach_hint( > + struct xfs_mount *mp, > + int type) > +{ > + struct xfs_quotainfo *qi = mp->m_quotainfo; > + struct radix_tree_root *tree = xfs_dquot_tree(qi, XFS_DQ_USER); > + uint32_t next_index; > + int skipped; > + int nr_found; > + > + ASSERT(type == XFS_DQ_GROUP || type == XFS_DQ_PROJ); > + > +restart: > + next_index = 0; > + skipped = 0; > + nr_found = 0; > + > + while (1) { > + struct xfs_dquot *batch[XFS_DQ_LOOKUP_BATCH]; > + int i; > + > + mutex_lock(&qi->qi_tree_lock); > + nr_found = radix_tree_gang_lookup(tree, (void **)batch, > + next_index, XFS_DQ_LOOKUP_BATCH); > + if (!nr_found) { > + mutex_unlock(&qi->qi_tree_lock); > + break; > + } Why reimplement xfs_qm_dquot_walk? >From a quick scan of the code, this could be done with xfs_qm_dquot_walk(XFS_DQ_USER) and an execute function that looks like the hint handling at the start of xfs_qm_dqpurge().... > STATIC int > xfs_qm_dquot_walk( > struct xfs_mount *mp, > @@ -224,10 +295,14 @@ xfs_qm_dqpurge_all( > { > if (flags & XFS_QMOPT_UQUOTA) > xfs_qm_dquot_walk(mp, XFS_DQ_USER, xfs_qm_dqpurge, NULL); > - if (flags & XFS_QMOPT_GQUOTA) > + if (flags & XFS_QMOPT_GQUOTA) { > + xfs_qm_dqdetach_hint(mp, XFS_DQ_GROUP); > xfs_qm_dquot_walk(mp, XFS_DQ_GROUP, xfs_qm_dqpurge, NULL); and called here like: if (flags & XFS_QMOPT_GQUOTA) { xfs_qm_dquot_walk(mp, XFS_DQ_USER, xfs_qm_detach_group_hint, NULL); xfs_qm_dquot_walk(mp, XFS_DQ_GROUP, xfs_qm_dqpurge, NULL); } In effect, this means we always need to walk the user quota tree, so we could do all the hint freeing and purging in a single pass by passing the flags as an argument to a special execution function for the user dquot tree walk that handles releasing hints and purging user dquots accordingly. i.e: xfs_qm_dquot_walk(mp, XFS_DQ_USER, xfs_qm_dqpurge_hints, &flags); if (flags & XFS_QMOPT_GQUOTA) xfs_qm_dquot_walk(mp, XFS_DQ_GROUP, xfs_qm_dqpurge, NULL); if (flags & XFS_QMOPT_PQUOTA) xfs_qm_dquot_walk(mp, XFS_DQ_PROJ, xfs_qm_dqpurge, NULL); But either way, we don't need to duplicate the radix tree walk code to release the hints... Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] xfs: fix infinite loop by detaching the group/project hints from user dquot 2013-11-19 11:12 ` Dave Chinner @ 2013-11-20 5:16 ` Jeff Liu 0 siblings, 0 replies; 3+ messages in thread From: Jeff Liu @ 2013-11-20 5:16 UTC (permalink / raw) To: Dave Chinner; +Cc: xfs On 11/19 2013 19:12 PM, Dave Chinner wrote: > On Tue, Nov 19, 2013 at 04:42:30PM +0800, Jeff Liu wrote: >> From: Jie Liu <jeff.liu@oracle.com> >> >> xfs_quota(8) will hang up if trying to turn group quota or project >> quota before the user quota is off, this could be 100% reproduced by: >> # mount -ouquota,gquota /dev/sda7 /xfs >> # mkdir /xfs/test >> # xfs_quota -xc 'off -g' /xfs >> # echo w > /proc/sysrq-trigger >> SysRq : Show Blocked State >> task PC stack pid father >> xfs_quota D 0000000000000000 0 27574 2551 0x00000000 >> [snip] >> Call Trace: >> [<ffffffff81aaa21d>] schedule+0xad/0xc0 >> [<ffffffff81aa327e>] schedule_timeout+0x35e/0x3c0 >> [<ffffffff8114b506>] ? mark_held_locks+0x176/0x1c0 >> [<ffffffff810ad6c0>] ? call_timer_fn+0x2c0/0x2c0 >> [<ffffffffa0c25380>] ? xfs_qm_shrink_count+0x30/0x30 [xfs] >> [<ffffffff81aa3306>] schedule_timeout_uninterruptible+0x26/0x30 >> [<ffffffffa0c26155>] xfs_qm_dquot_walk+0x235/0x260 [xfs] >> [<ffffffffa0c059d8>] ? xfs_perag_get+0x1d8/0x2d0 [xfs] >> [<ffffffffa0c05805>] ? xfs_perag_get+0x5/0x2d0 [xfs] >> [<ffffffffa0b7707e>] ? xfs_inode_ag_iterator+0xae/0xf0 [xfs] >> [<ffffffffa0c22280>] ? xfs_trans_free_dqinfo+0x50/0x50 [xfs] >> [<ffffffffa0b7709f>] ? xfs_inode_ag_iterator+0xcf/0xf0 [xfs] >> [<ffffffffa0c261e6>] xfs_qm_dqpurge_all+0x66/0xb0 [xfs] >> [<ffffffffa0c2497a>] xfs_qm_scall_quotaoff+0x20a/0x5f0 [xfs] >> [<ffffffffa0c2b8f6>] xfs_fs_set_xstate+0x136/0x180 [xfs] >> [<ffffffff8136cf7a>] do_quotactl+0x53a/0x6b0 >> [<ffffffff812fba4b>] ? iput+0x5b/0x90 >> [<ffffffff8136d257>] SyS_quotactl+0x167/0x1d0 >> [<ffffffff814cf2ee>] ? trace_hardirqs_on_thunk+0x3a/0x3f >> [<ffffffff81abcd19>] system_call_fastpath+0x16/0x1b >> >> It's fine if we turn user quota off at first, then turn off other >> kind of quotas if they are enabled since the group/project dquot >> refcount is decreased to zero once the user quota if off. Otherwse, >> those dquots refcount is non-zero due to the user dquot maybe refer >> to them as hint(s). Hence, above operation hit an infinite loop at >> xfs_qm_dquot_walk() to purge dquot cache. >> >> This problem has been around since Linux 3.4, it was introduced by: >> b84a3a96751f93071c1863f2962273973c8b8f5e >> xfs: remove the per-filesystem list of dquots >> >> Originally we will release the group dquot pointers because the user >> dquots maybe carrying around as a hint via xfs_qm_detach_gdquots(). >> However, with this change, there is no such work to be done before >> purge group/project dquot cache. >> >> This fix introduce a similar routine to the old xfs_qm_detach_gdquots(), >> it will detach the group/project hints by searching the user dquot radix >> tree and release those hints if they are there. > > Ok, we do get stuck in a loop in this case. We need an xfstest to > exercise this - removing group and project quotas while user quotas > are left on and fsstress is active - should be pretty easy to do. > > More comments below. > >> Signed-off-by: Jie Liu <jeff.liu@oracle.com> >> --- >> fs/xfs/xfs_qm.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 77 insertions(+), 2 deletions(-) >> >> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c >> index 14a4996..410adf4 100644 >> --- a/fs/xfs/xfs_qm.c >> +++ b/fs/xfs/xfs_qm.c >> @@ -60,6 +60,77 @@ STATIC void xfs_qm_dqfree_one(struct xfs_dquot *dqp); >> */ >> #define XFS_DQ_LOOKUP_BATCH 32 >> >> +/* >> + * Release the group or project dquot pointers the user dquots may be >> + * carrying around as a hint. >> + */ >> +STATIC void >> +xfs_qm_dqdetach_hint( >> + struct xfs_mount *mp, >> + int type) >> +{ >> + struct xfs_quotainfo *qi = mp->m_quotainfo; >> + struct radix_tree_root *tree = xfs_dquot_tree(qi, XFS_DQ_USER); >> + uint32_t next_index; >> + int skipped; >> + int nr_found; >> + >> + ASSERT(type == XFS_DQ_GROUP || type == XFS_DQ_PROJ); >> + >> +restart: >> + next_index = 0; >> + skipped = 0; >> + nr_found = 0; >> + >> + while (1) { >> + struct xfs_dquot *batch[XFS_DQ_LOOKUP_BATCH]; >> + int i; >> + >> + mutex_lock(&qi->qi_tree_lock); >> + nr_found = radix_tree_gang_lookup(tree, (void **)batch, >> + next_index, XFS_DQ_LOOKUP_BATCH); >> + if (!nr_found) { >> + mutex_unlock(&qi->qi_tree_lock); >> + break; >> + } > > Why reimplement xfs_qm_dquot_walk? > > From a quick scan of the code, this could be done with > xfs_qm_dquot_walk(XFS_DQ_USER) and an execute function that looks > like the hint handling at the start of xfs_qm_dqpurge().... > >> STATIC int >> xfs_qm_dquot_walk( >> struct xfs_mount *mp, >> @@ -224,10 +295,14 @@ xfs_qm_dqpurge_all( >> { >> if (flags & XFS_QMOPT_UQUOTA) >> xfs_qm_dquot_walk(mp, XFS_DQ_USER, xfs_qm_dqpurge, NULL); >> - if (flags & XFS_QMOPT_GQUOTA) >> + if (flags & XFS_QMOPT_GQUOTA) { >> + xfs_qm_dqdetach_hint(mp, XFS_DQ_GROUP); >> xfs_qm_dquot_walk(mp, XFS_DQ_GROUP, xfs_qm_dqpurge, NULL); > > and called here like: > > if (flags & XFS_QMOPT_GQUOTA) { > xfs_qm_dquot_walk(mp, XFS_DQ_USER, xfs_qm_detach_group_hint, NULL); > xfs_qm_dquot_walk(mp, XFS_DQ_GROUP, xfs_qm_dqpurge, NULL); > } > > In effect, this means we always need to walk the user quota tree, > so we could do all the hint freeing and purging in a single pass by > passing the flags as an argument to a special execution function for > the user dquot tree walk that handles releasing hints and purging > user dquots accordingly. i.e: > > xfs_qm_dquot_walk(mp, XFS_DQ_USER, xfs_qm_dqpurge_hints, &flags); > if (flags & XFS_QMOPT_GQUOTA) > xfs_qm_dquot_walk(mp, XFS_DQ_GROUP, xfs_qm_dqpurge, NULL); > if (flags & XFS_QMOPT_PQUOTA) > xfs_qm_dquot_walk(mp, XFS_DQ_PROJ, xfs_qm_dqpurge, NULL); > > But either way, we don't need to duplicate the radix tree walk code > to release the hints... Ok, I'll post a revised fix as well as a test after finishing another internal task. Thanks, -Jeff _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-11-20 5:16 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-11-19 8:42 [PATCH] xfs: fix infinite loop by detaching the group/project hints from user dquot Jeff Liu 2013-11-19 11:12 ` Dave Chinner 2013-11-20 5:16 ` Jeff Liu
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.