* Re: [patch] hugetlb: fix lockdep error
2008-05-29 3:19 ` Andrew Morton
@ 2008-05-29 3:26 ` Nick Piggin
2008-05-29 3:35 ` Andrew Morton
2008-05-29 3:48 ` Nish Aravamudan
2008-05-29 12:35 ` Adam Litke
2 siblings, 1 reply; 9+ messages in thread
From: Nick Piggin @ 2008-05-29 3:26 UTC (permalink / raw)
To: Andrew Morton; +Cc: agl, nacc, Linux Memory Management List, kosaki.motohiro
On Wed, May 28, 2008 at 08:19:29PM -0700, Andrew Morton wrote:
> On Thu, 29 May 2008 05:07:45 +0200 Nick Piggin <npiggin@suse.de> wrote:
>
> > > > mm/hugetlb.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > Index: linux-2.6/mm/hugetlb.c
> > > > ===================================================================
> > > > --- linux-2.6.orig/mm/hugetlb.c
> > > > +++ linux-2.6/mm/hugetlb.c
> > > > @@ -785,7 +785,7 @@ int copy_hugetlb_page_range(struct mm_st
> > > > continue;
> > > >
> > > > spin_lock(&dst->page_table_lock);
> > > > - spin_lock(&src->page_table_lock);
> > > > + spin_lock_nested(&src->page_table_lock, SINGLE_DEPTH_NESTING);
> > > > if (!huge_pte_none(huge_ptep_get(src_pte))) {
> > > > if (cow)
> > > > huge_ptep_set_wrprotect(src, addr, src_pte);
> > >
> > > Confused. This code has been there since October 2005. Why are we
> > > only seeing lockdep warnings now?
> >
> > Can't say. Haven't looked at hugetlb code or tested it much until now.
> > I am using a recent libhugetlbfs test suite, FWIW.
>
> I don't believe that it's possible that nobody has run that test suite
> with lockdep enabled at any time in the past three years.
Could be it was ignored as a false positive?
> If that's really the case then perhaps we should make the ability to
> recite Documentation/SubmitChecklist in three languages a prerequisite
> for MM development.
It does seem unusual, I don't know...
Would it help to have a big button in kconfig called "test your kernel
patches with this", which then selects various other things?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [patch] hugetlb: fix lockdep error
2008-05-29 3:26 ` Nick Piggin
@ 2008-05-29 3:35 ` Andrew Morton
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Morton @ 2008-05-29 3:35 UTC (permalink / raw)
To: Nick Piggin; +Cc: agl, nacc, Linux Memory Management List, kosaki.motohiro
On Thu, 29 May 2008 05:26:58 +0200 Nick Piggin <npiggin@suse.de> wrote:
> Would it help to have a big button in kconfig called "test your kernel
> patches with this", which then selects various other things?
Sigh. Maybe. A big stick to whack people with would be nice too.
It would be good to have some mechanism to detect the kernel version
within Kconfig. So we could at least do things in Kconfig which make
it really really really hard to disable debug features if
CONFIG_RC_KERNEL=y.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] hugetlb: fix lockdep error
2008-05-29 3:19 ` Andrew Morton
2008-05-29 3:26 ` Nick Piggin
@ 2008-05-29 3:48 ` Nish Aravamudan
2008-05-29 8:58 ` Nick Piggin
2008-05-29 12:35 ` Adam Litke
2 siblings, 1 reply; 9+ messages in thread
From: Nish Aravamudan @ 2008-05-29 3:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Nick Piggin, agl, nacc, Linux Memory Management List,
kosaki.motohiro
On 5/28/08, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Thu, 29 May 2008 05:07:45 +0200 Nick Piggin <npiggin@suse.de> wrote:
>
> > > > mm/hugetlb.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > Index: linux-2.6/mm/hugetlb.c
> > > > ===================================================================
> > > > --- linux-2.6.orig/mm/hugetlb.c
> > > > +++ linux-2.6/mm/hugetlb.c
> > > > @@ -785,7 +785,7 @@ int copy_hugetlb_page_range(struct mm_st
> > > > continue;
> > > >
> > > > spin_lock(&dst->page_table_lock);
> > > > - spin_lock(&src->page_table_lock);
> > > > + spin_lock_nested(&src->page_table_lock, SINGLE_DEPTH_NESTING);
> > > > if (!huge_pte_none(huge_ptep_get(src_pte))) {
> > > > if (cow)
> > > > huge_ptep_set_wrprotect(src, addr, src_pte);
> > >
> > > Confused. This code has been there since October 2005. Why are we
> > > only seeing lockdep warnings now?
> >
> > Can't say. Haven't looked at hugetlb code or tested it much until now.
> > I am using a recent libhugetlbfs test suite, FWIW.
>
>
> I don't believe that it's possible that nobody has run that test suite
> with lockdep enabled at any time in the past three years.
I can't tell from Nick's mail if the lockdep error is specific to this
particular testcase or not, but if so, that would make it the past two
(almost three) months, as that was when this particular testcase was
added. And I'm not sure when we released the first development
snapshot that would have included it (for non-git users, that is). In
any case, I also don't know how we wouldn't have seen this issue on
our systems and that's a problem. I will make a concerted effort to
work with the other libhugetlbfs developers to make sure all possible
debug options are on (as you've pointed out to me elsewhere/when).
Thanks,
Nish
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [patch] hugetlb: fix lockdep error
2008-05-29 3:48 ` Nish Aravamudan
@ 2008-05-29 8:58 ` Nick Piggin
0 siblings, 0 replies; 9+ messages in thread
From: Nick Piggin @ 2008-05-29 8:58 UTC (permalink / raw)
To: Nish Aravamudan
Cc: Andrew Morton, agl, nacc, Linux Memory Management List,
kosaki.motohiro
On Wed, May 28, 2008 at 08:48:36PM -0700, Nish Aravamudan wrote:
> On 5/28/08, Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Thu, 29 May 2008 05:07:45 +0200 Nick Piggin <npiggin@suse.de> wrote:
> >
> > > > > mm/hugetlb.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > Index: linux-2.6/mm/hugetlb.c
> > > > > ===================================================================
> > > > > --- linux-2.6.orig/mm/hugetlb.c
> > > > > +++ linux-2.6/mm/hugetlb.c
> > > > > @@ -785,7 +785,7 @@ int copy_hugetlb_page_range(struct mm_st
> > > > > continue;
> > > > >
> > > > > spin_lock(&dst->page_table_lock);
> > > > > - spin_lock(&src->page_table_lock);
> > > > > + spin_lock_nested(&src->page_table_lock, SINGLE_DEPTH_NESTING);
> > > > > if (!huge_pte_none(huge_ptep_get(src_pte))) {
> > > > > if (cow)
> > > > > huge_ptep_set_wrprotect(src, addr, src_pte);
> > > >
> > > > Confused. This code has been there since October 2005. Why are we
> > > > only seeing lockdep warnings now?
> > >
> > > Can't say. Haven't looked at hugetlb code or tested it much until now.
> > > I am using a recent libhugetlbfs test suite, FWIW.
> >
> >
> > I don't believe that it's possible that nobody has run that test suite
> > with lockdep enabled at any time in the past three years.
>
> I can't tell from Nick's mail if the lockdep error is specific to this
> particular testcase or not, but if so, that would make it the past two
> (almost three) months, as that was when this particular testcase was
> added. And I'm not sure when we released the first development
> snapshot that would have included it (for non-git users, that is). In
> any case, I also don't know how we wouldn't have seen this issue on
> our systems and that's a problem. I will make a concerted effort to
I didn't check if other tests trigger it (lockdep I think turns off after
the first message...). But I guess anything which calls fork and has a
non shared, populated hugepage pagetable entry should trigger it...
Anyway the good news is that no other lockdep problems ever came up
after this false positive was fixed, so not much harm done ;)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] hugetlb: fix lockdep error
2008-05-29 3:19 ` Andrew Morton
2008-05-29 3:26 ` Nick Piggin
2008-05-29 3:48 ` Nish Aravamudan
@ 2008-05-29 12:35 ` Adam Litke
2 siblings, 0 replies; 9+ messages in thread
From: Adam Litke @ 2008-05-29 12:35 UTC (permalink / raw)
To: Andrew Morton
Cc: Nick Piggin, nacc, Linux Memory Management List, kosaki.motohiro
On Wed, 2008-05-28 at 20:19 -0700, Andrew Morton wrote:
> On Thu, 29 May 2008 05:07:45 +0200 Nick Piggin <npiggin@suse.de> wrote:
>
> > > > mm/hugetlb.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > Index: linux-2.6/mm/hugetlb.c
> > > > ===================================================================
> > > > --- linux-2.6.orig/mm/hugetlb.c
> > > > +++ linux-2.6/mm/hugetlb.c
> > > > @@ -785,7 +785,7 @@ int copy_hugetlb_page_range(struct mm_st
> > > > continue;
> > > >
> > > > spin_lock(&dst->page_table_lock);
> > > > - spin_lock(&src->page_table_lock);
> > > > + spin_lock_nested(&src->page_table_lock, SINGLE_DEPTH_NESTING);
> > > > if (!huge_pte_none(huge_ptep_get(src_pte))) {
> > > > if (cow)
> > > > huge_ptep_set_wrprotect(src, addr, src_pte);
> > >
> > > Confused. This code has been there since October 2005. Why are we
> > > only seeing lockdep warnings now?
> >
> > Can't say. Haven't looked at hugetlb code or tested it much until now.
> > I am using a recent libhugetlbfs test suite, FWIW.
>
> I don't believe that it's possible that nobody has run that test suite
> with lockdep enabled at any time in the past three years.
I have to confess that I have seen this from time to time. Since it was
clearly a false positive, it was easy to get distracted by other things.
I'll go and update all my default kernel configs to turn on everything
all the time so things like this annoy me until I fix them.
--
Adam Litke - (agl at us.ibm.com)
IBM Linux Technology Center
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 9+ messages in thread