Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1497578
| From | Dave Chinner <david@fromorbit.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths |
| Date | 2016-10-08 01:00 +0200 |
| Message-ID | <spMhs-749-5@gated-at.bofh.it> (permalink) |
| References | (5 earlier) <soDSV-71V-5@gated-at.bofh.it> <soXyh-3sY-11@gated-at.bofh.it> <spkuS-3vj-33@gated-at.bofh.it> <spoRP-6HK-9@gated-at.bofh.it> <spGYp-3AS-5@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Fri, Oct 07, 2016 at 07:15:18PM +0200, Oleg Nesterov wrote:
> On 10/07, Dave Chinner wrote:
> >
> > On Thu, Oct 06, 2016 at 07:17:58PM +0200, Oleg Nesterov wrote:
> > > Probably false positive? Although when I look at the comment above xfs_sync_sb()
> > > I think that may be sometging like below makes sense, but I know absolutely nothing
> > > about fs/ and XFS in particular.
> > >
> > > Oleg.
> > >
> > >
> > > --- x/fs/xfs/xfs_trans.c
> > > +++ x/fs/xfs/xfs_trans.c
> > > @@ -245,7 +245,8 @@ xfs_trans_alloc(
> > > atomic_inc(&mp->m_active_trans);
> > >
> > > tp = kmem_zone_zalloc(xfs_trans_zone,
> > > - (flags & XFS_TRANS_NOFS) ? KM_NOFS : KM_SLEEP);
> > > + (flags & (XFS_TRANS_NOFS | XFS_TRANS_NO_WRITECOUNT))
> > > + ? KM_NOFS : KM_SLEEP);
> > > tp->t_magic = XFS_TRANS_HEADER_MAGIC;
> > > tp->t_flags = flags;
> > > tp->t_mountp = mp;
> >
> > Brief examination says caller should set XFS_TRANS_NOFS, not change
> > the implementation to make XFS_TRANS_NO_WRITECOUNT flag to also mean
> > XFS_TRANS_NOFS.
>
> I didn't mean the change above can fix the problem, and I don't really
> understand your suggestion.
xfs_syncsb() does:
tp = xfs_trans_alloc(... , XFS_TRANS_NO_WRITECOUNT, ....);
but it's running in a GFP_NOFS context when a freeze is being
finalised. SO, rather than changing what XFS_TRANS_NO_WRITECOUNT
does in xfs_trans_alloc(), we should tell it to do a GFP_NOFS
allocation. i.e.
tp = xfs_trans_alloc(... , XFS_TRANS_NOFS | XFS_TRANS_NO_WRITECOUNT, ....);
and nothing inside xfs_trans_alloc() changes at all.
> Obviously any GFP_FS allocation in xfs_fs_freeze()
> paths will trigger the same warning.
Of which there should be none except for that xfs_trans_alloc()
call.
> I added this hack
>
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1333,10 +1333,15 @@ xfs_fs_freeze(
> struct super_block *sb)
> {
> struct xfs_mount *mp = XFS_M(sb);
> + int ret;
>
> + current->flags |= PF_FSTRANS; // tell kmem_flags_convert() to remove GFP_FS
> xfs_save_resvblks(mp);
> xfs_quiesce_attr(mp);
> - return xfs_sync_sb(mp, true);
> + ret = xfs_sync_sb(mp, true);
> + current->flags &= ~PF_FSTRANS;
> +
> + return ret;
> }
/me shudders
> just for testing purposes and after that I got another warning below. I didn't
> read it carefully yet, but _at first glance_ it looks like the lock inversion
> uncovered by 2/2, although I can be easily wrong. cancel_delayed_work_sync(l_work)
> under sb_internal can hang if xfs_log_worker() waits for this rwsem?`
Actually: I *can't read it*. I've got no fucking clue what lockdep
is trying to say here. This /looks/ like a lockdep is getting
confused between memory reclaim contexts (which /aren't locks/ but
overload interrupt levels) and freeze contexts (which /aren't locks/)
and workqueue locks which /aren't nested/ inside transactions or
freeze contexts. But, really, I can't follow it because I have to
guess at what "lock contexts" are not locks but are something else.
cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH 0/2] (Was: BUG_ON in rcu_sync_func triggered) Oleg Nesterov <oleg@redhat.com> - 2016-09-26 18:10 +0200
[PATCH 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Oleg Nesterov <oleg@redhat.com> - 2016-09-26 18:10 +0200
Re: [PATCH 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Jan Kara <jack@suse.cz> - 2016-09-26 18:20 +0200
[PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Oleg Nesterov <oleg@redhat.com> - 2016-09-26 19:00 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Jan Kara <jack@suse.cz> - 2016-09-27 09:00 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Dave Chinner <david@fromorbit.com> - 2016-09-27 09:20 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Oleg Nesterov <oleg@redhat.com> - 2016-09-27 19:30 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Oleg Nesterov <oleg@redhat.com> - 2016-09-30 19:20 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Dave Chinner <david@fromorbit.com> - 2016-10-02 23:50 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Oleg Nesterov <oleg@redhat.com> - 2016-10-03 18:50 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Oleg Nesterov <oleg@redhat.com> - 2016-10-04 19:00 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Dave Chinner <david@fromorbit.com> - 2016-10-04 22:10 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Oleg Nesterov <oleg@redhat.com> - 2016-10-05 18:40 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Dave Chinner <david@fromorbit.com> - 2016-10-04 21:50 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Oleg Nesterov <oleg@redhat.com> - 2016-10-05 18:50 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Jan Kara <jack@suse.cz> - 2016-10-06 09:30 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Oleg Nesterov <oleg@redhat.com> - 2016-10-06 19:20 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Dave Chinner <david@fromorbit.com> - 2016-10-07 00:00 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Oleg Nesterov <oleg@redhat.com> - 2016-10-07 19:20 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Dave Chinner <david@fromorbit.com> - 2016-10-08 01:00 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Oleg Nesterov <oleg@redhat.com> - 2016-10-09 18:20 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Dave Chinner <david@fromorbit.com> - 2016-10-10 03:20 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Johannes Weiner <hannes@cmpxchg.org> - 2016-10-06 15:50 +0200
Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Oleg Nesterov <oleg@redhat.com> - 2016-10-07 19:00 +0200
[PATCH 1/2] fs/super.c: fix race between freeze_super() and thaw_super() Oleg Nesterov <oleg@redhat.com> - 2016-09-26 18:10 +0200
Re: [PATCH 1/2] fs/super.c: fix race between freeze_super() and thaw_super() Jan Kara <jack@suse.cz> - 2016-09-26 18:20 +0200
csiph-web