Messages in this thread Patch in this message | | | Date | Sun, 1 Jun 2014 21:32:40 +0200 | From | Peter Zijlstra <> | Subject | Re: sleeping function warning from __put_anon_vma |
| |
On Sat, May 31, 2014 at 01:33:13PM -0700, Hugh Dickins wrote: > [PATCH] mm: fix sleeping function warning from __put_anon_vma > > Trinity reports BUG: > sleeping function called from invalid context at kernel/locking/rwsem.c:47 > in_atomic(): 0, irqs_disabled(): 0, pid: 5787, name: trinity-c27 > __might_sleep < down_write < __put_anon_vma < page_get_anon_vma < > migrate_pages < compact_zone < compact_zone_order < try_to_compact_pages .. > > Right, since conversion to mutex then rwsem, we should not put_anon_vma() > from inside an rcu_read_lock()ed section: fix the two places that did so. > > Fixes: 88c22088bf23 ("mm: optimize page_lock_anon_vma() fast-path") > Reported-by: Dave Jones <davej@redhat.com> > Signed-off-by: Hugh Dickins <hughd@google.com> > Needs-Ack-from: Peter Zijlstra <peterz@infradead.org> > --- > > mm/rmap.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > --- 3.15-rc7/mm/rmap.c 2014-04-13 17:24:36.680507189 -0700 > +++ linux/mm/rmap.c 2014-05-31 12:02:08.496088637 -0700 > @@ -426,12 +426,14 @@ struct anon_vma *page_get_anon_vma(struc > * above cannot corrupt). > */ > if (!page_mapped(page)) { > + rcu_read_unlock(); > put_anon_vma(anon_vma); > anon_vma = NULL; > + goto outer; > } > out: > rcu_read_unlock(); > - > +outer: > return anon_vma; > }
I think we can do that without the goto if we write something like:
if (!page_mapped(page)) { rcu_read_unlock(); put_anon_vma(anon_vma); return NULL; }
> @@ -477,9 +479,10 @@ struct anon_vma *page_lock_anon_vma_read > } > > if (!page_mapped(page)) { > + rcu_read_unlock(); > put_anon_vma(anon_vma); > anon_vma = NULL; > - goto out; > + goto outer; > } > > /* we pinned the anon_vma, its safe to sleep */ > @@ -501,6 +504,7 @@ struct anon_vma *page_lock_anon_vma_read > > out: > rcu_read_unlock(); > +outer: > return anon_vma; > }
Same here too, I suppose.
Interesting that we never managed to hit this one; it might also make sense to put a might_sleep() in anon_vma_free().
Other than that, I don't see anything really odd, then again, its sunday evening and my thinking cap isn't exactly on proper.
| |