From: Oleg Nesterov <oleg@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: Jan Kara <jack@suse.cz>, Al Viro <viro@zeniv.linux.org.uk>,
Nikolay Borisov <kernel@kyup.com>,
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
fstests@vger.kernel.org
Subject: Re: [PATCH V2 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths
Date: Wed, 5 Oct 2016 18:44:32 +0200 [thread overview]
Message-ID: <20161005164432.GB15121@redhat.com> (raw)
In-Reply-To: <20161004194435.GW9806@dastard>
On 10/05, Dave Chinner wrote:
>
> On Tue, Oct 04, 2016 at 01:43:43PM +0200, Oleg Nesterov wrote:
>
> > plus the following warnings:
> >
> > [ 1894.500040] run fstests generic/070 at 2016-10-04 05:03:39
> > [ 1895.076655] =================================
> > [ 1895.077136] [ INFO: inconsistent lock state ]
> > [ 1895.077574] 4.8.0 #1 Not tainted
> > [ 1895.077900] ---------------------------------
> > [ 1895.078330] inconsistent {IN-RECLAIM_FS-W} -> {RECLAIM_FS-ON-W} usage.
> > [ 1895.078993] fsstress/18239 [HC0[0]:SC0[0]:HE1:SE1] takes:
> > [ 1895.079522] (&xfs_nondir_ilock_class){++++?-}, at: [<ffffffffc049ad45>] xfs_ilock+0x165/0x210 [xfs]
> > [ 1895.080529] {IN-RECLAIM_FS-W} state was registered at:
>
> And that is a bug in the lockdep annotations for memory allocation because it
> fails to take into account the current task flags that are set via
> memalloc_noio_save() to prevent vmalloc from doing GFP_KERNEL allocations. i.e.
> in _xfs_buf_map_pages():
OK, I see...
I'll re-test with the following change:
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2867,7 +2867,7 @@ static void __lockdep_trace_alloc(gfp_t gfp_mask, unsigned long flags)
return;
/* We're only interested __GFP_FS allocations for now */
- if (!(gfp_mask & __GFP_FS))
+ if ((curr->flags & PF_MEMALLOC_NOIO) || !(gfp_mask & __GFP_FS))
return;
Hmm. This is off-topic and most probably I missed something... but at
first glance we can simplify/improve the reclaim-fs lockdep annotations:
1. add the global "struct lockdep_map reclaim_fs_map"
2. change __lockdep_trace_alloc
- mark_held_locks(curr, RECLAIM_FS);
+ lock_map_acquire(&reclaim_fs_map);
+ lock_map_release(&reclaim_fs_map);
3. turn lockdep_set/clear_current_reclaim_state() into
void lockdep_set_current_reclaim_state(gfp_t gfp_mask)
{
if (gfp_mask & __GFP_FS)
lock_map_acquire(&reclaim_fs_map);
}
void lockdep_clear_current_reclaim_state(gfp_t gfp_mask)
{
if (gfp_mask & __GFP_FS)
lock_map_release(&reclaim_fs_map);
}
and now we can remove task_struct->lockdep_reclaim_gfp and all other
RECLAIM_FS hacks in lockdep.c. Plus we can easily extend this logic to
check more GFP_ flags.
No?
Oleg.
next prev parent reply other threads:[~2016-10-05 16:44 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-26 16:07 [PATCH 0/2] (Was: BUG_ON in rcu_sync_func triggered) Oleg Nesterov
2016-09-26 16:07 ` [PATCH 1/2] fs/super.c: fix race between freeze_super() and thaw_super() Oleg Nesterov
2016-09-26 16:11 ` Jan Kara
2016-09-26 16:08 ` [PATCH 2/2] fs/super.c: don't fool lockdep in freeze_super() and thaw_super() paths Oleg Nesterov
2016-09-26 16:18 ` Jan Kara
2016-09-26 16:55 ` [PATCH V2 " Oleg Nesterov
2016-09-27 6:51 ` Jan Kara
2016-09-27 7:14 ` Dave Chinner
2016-09-27 17:29 ` Oleg Nesterov
2016-09-30 17:14 ` Oleg Nesterov
2016-10-02 21:42 ` Dave Chinner
2016-10-03 16:44 ` Oleg Nesterov
2016-10-04 11:43 ` Oleg Nesterov
2016-10-04 11:48 ` Michal Hocko
2016-10-06 13:44 ` Johannes Weiner
2016-10-07 16:52 ` Oleg Nesterov
2016-10-04 16:58 ` Oleg Nesterov
2016-10-04 20:03 ` Dave Chinner
2016-10-05 16:33 ` Oleg Nesterov
2016-10-04 19:44 ` Dave Chinner
2016-10-05 16:44 ` Oleg Nesterov [this message]
2016-10-06 7:27 ` Jan Kara
2016-10-06 17:17 ` Oleg Nesterov
2016-10-06 21:59 ` Dave Chinner
2016-10-07 17:15 ` Oleg Nesterov
2016-10-07 22:52 ` Dave Chinner
2016-10-09 16:14 ` Oleg Nesterov
2016-10-10 1:02 ` Dave Chinner
2016-10-13 16:58 ` Oleg Nesterov
2016-10-13 17:10 ` [PATCH 0/2] (Was: BUG_ON in rcu_sync_func triggered) Oleg Nesterov
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=20161005164432.GB15121@redhat.com \
--to=oleg@redhat.com \
--cc=david@fromorbit.com \
--cc=fstests@vger.kernel.org \
--cc=jack@suse.cz \
--cc=kernel@kyup.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=paulmck@linux.vnet.ibm.com \
--cc=viro@zeniv.linux.org.uk \
/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.