From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Ebru Akagunduz <ebru.akagunduz@gmail.com>
Cc: Vlastimil Babka <vbabka@suse.cz>,
sergey.senozhatsky.work@gmail.com,
Andrew Morton <akpm@linux-foundation.org>,
Michal Hocko <mhocko@kernel.org>,
"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
Stephen Rothwell <sfr@canb.auug.org.au>,
Andrea Arcangeli <aarcange@redhat.com>,
Rik van Riel <riel@redhat.com>,
linux-mm@kvack.org, linux-next@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [linux-next: Tree for Jun 1] __khugepaged_exit rwsem_down_write_failed lockup
Date: Fri, 3 Jun 2016 10:00:36 +0900 [thread overview]
Message-ID: <20160603010036.GA464@swordfish> (raw)
In-Reply-To: <20160602185856.GA3854@debian>
On (06/02/16 21:58), Ebru Akagunduz wrote:
[..]
> > I think it's this patch:
> >
> > http://ozlabs.org/~akpm/mmots/broken-out/mm-thp-make-swapin-readahead-under-down_read-of-mmap_sem.patch
> >
> > Some parts of the code in collapse_huge_page() that were under
> > down_write(mmap_sem) are under down_read() after the patch. But
> > there's "goto out" which continues via "goto out_up_write" which
> > does up_write(mmap_sem) so there's an imbalance. One path seems to
> > go via both up_read() and up_write(). I can imagine this can cause a
> > stuck down_write() among other things?
> Recently, I realized the same imbalance, it is an obvious
> inconsistency. I don't know, this issue can be related with
> mine. I'll send a fix patch.
a good find by Vlastimil.
Ebru, can you also re-visit __collapse_huge_page_swapin()? it's called
from collapse_huge_page() under the down_read(&mm->mmap_sem), is there
any reason to do the nested down_read(&mm->mmap_sem)?
collapse_huge_page()
...
down_read(&mm->mmap_sem);
result = hugepage_vma_revalidate(mm, vma, address);
if (result)
goto out;
pmd = mm_find_pmd(mm, address);
if (!pmd) {
result = SCAN_PMD_NULL;
goto out;
}
if (allocstall == curr_allocstall && swap != 0) {
if (!__collapse_huge_page_swapin(mm, vma, address, pmd)) {
{
: if (ret & VM_FAULT_RETRY) {
: down_read(&mm->mmap_sem);
: ^^^^^^^^^
: if (hugepage_vma_revalidate(mm, vma, address))
: return false;
: }
}
up_read(&mm->mmap_sem);
goto out;
}
}
up_read(&mm->mmap_sem);
so if __collapse_huge_page_swapin() retruns true we have:
- down_read() twice, up_read() once?
the locking rules here are a bit confusing. (I didn't have my morning coffee yet).
-ss
--
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>
WARNING: multiple messages have this Message-ID (diff)
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Ebru Akagunduz <ebru.akagunduz@gmail.com>
Cc: Vlastimil Babka <vbabka@suse.cz>,
sergey.senozhatsky.work@gmail.com,
Andrew Morton <akpm@linux-foundation.org>,
Michal Hocko <mhocko@kernel.org>,
"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
Stephen Rothwell <sfr@canb.auug.org.au>,
Andrea Arcangeli <aarcange@redhat.com>,
Rik van Riel <riel@redhat.com>,
linux-mm@kvack.org, linux-next@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [linux-next: Tree for Jun 1] __khugepaged_exit rwsem_down_write_failed lockup
Date: Fri, 3 Jun 2016 10:00:36 +0900 [thread overview]
Message-ID: <20160603010036.GA464@swordfish> (raw)
In-Reply-To: <20160602185856.GA3854@debian>
On (06/02/16 21:58), Ebru Akagunduz wrote:
[..]
> > I think it's this patch:
> >
> > http://ozlabs.org/~akpm/mmots/broken-out/mm-thp-make-swapin-readahead-under-down_read-of-mmap_sem.patch
> >
> > Some parts of the code in collapse_huge_page() that were under
> > down_write(mmap_sem) are under down_read() after the patch. But
> > there's "goto out" which continues via "goto out_up_write" which
> > does up_write(mmap_sem) so there's an imbalance. One path seems to
> > go via both up_read() and up_write(). I can imagine this can cause a
> > stuck down_write() among other things?
> Recently, I realized the same imbalance, it is an obvious
> inconsistency. I don't know, this issue can be related with
> mine. I'll send a fix patch.
a good find by Vlastimil.
Ebru, can you also re-visit __collapse_huge_page_swapin()? it's called
from collapse_huge_page() under the down_read(&mm->mmap_sem), is there
any reason to do the nested down_read(&mm->mmap_sem)?
collapse_huge_page()
...
down_read(&mm->mmap_sem);
result = hugepage_vma_revalidate(mm, vma, address);
if (result)
goto out;
pmd = mm_find_pmd(mm, address);
if (!pmd) {
result = SCAN_PMD_NULL;
goto out;
}
if (allocstall == curr_allocstall && swap != 0) {
if (!__collapse_huge_page_swapin(mm, vma, address, pmd)) {
{
: if (ret & VM_FAULT_RETRY) {
: down_read(&mm->mmap_sem);
: ^^^^^^^^^
: if (hugepage_vma_revalidate(mm, vma, address))
: return false;
: }
}
up_read(&mm->mmap_sem);
goto out;
}
}
up_read(&mm->mmap_sem);
so if __collapse_huge_page_swapin() retruns true we have:
- down_read() twice, up_read() once?
the locking rules here are a bit confusing. (I didn't have my morning coffee yet).
-ss
next prev parent reply other threads:[~2016-06-03 1:00 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-01 3:11 linux-next: Tree for Jun 1 Stephen Rothwell
2016-06-02 1:48 ` [linux-next: Tree for Jun 1] __khugepaged_exit rwsem_down_write_failed lockup Sergey Senozhatsky
2016-06-02 1:48 ` Sergey Senozhatsky
2016-06-02 9:21 ` Michal Hocko
2016-06-02 9:21 ` Michal Hocko
2016-06-02 12:08 ` Sergey Senozhatsky
2016-06-02 12:08 ` Sergey Senozhatsky
2016-06-02 12:21 ` Michal Hocko
2016-06-02 12:21 ` Michal Hocko
2016-06-03 13:51 ` Andrea Arcangeli
2016-06-03 13:51 ` Andrea Arcangeli
2016-06-03 14:46 ` Michal Hocko
2016-06-03 14:46 ` Michal Hocko
2016-06-03 15:10 ` Andrea Arcangeli
2016-06-03 15:10 ` Andrea Arcangeli
2016-06-07 7:34 ` Michal Hocko
2016-06-07 7:34 ` Michal Hocko
2016-06-08 8:19 ` Vlastimil Babka
2016-06-08 8:19 ` Vlastimil Babka
2016-06-03 7:15 ` Sergey Senozhatsky
2016-06-03 7:15 ` Sergey Senozhatsky
2016-06-03 7:25 ` Michal Hocko
2016-06-03 7:25 ` Michal Hocko
2016-06-03 8:43 ` Sergey Senozhatsky
2016-06-03 8:43 ` Sergey Senozhatsky
2016-06-03 9:55 ` Michal Hocko
2016-06-03 9:55 ` Michal Hocko
2016-06-03 10:05 ` Michal Hocko
2016-06-03 10:05 ` Michal Hocko
2016-06-03 13:38 ` Sergey Senozhatsky
2016-06-03 13:38 ` Sergey Senozhatsky
2016-06-03 13:45 ` Michal Hocko
2016-06-03 13:45 ` Michal Hocko
2016-06-03 13:49 ` Michal Hocko
2016-06-03 13:49 ` Michal Hocko
2016-06-03 13:49 ` Michal Hocko
2016-06-04 7:51 ` Sergey Senozhatsky
2016-06-04 7:51 ` Sergey Senozhatsky
2016-06-06 8:39 ` Michal Hocko
2016-06-06 8:39 ` Michal Hocko
2016-06-02 13:24 ` Vlastimil Babka
2016-06-02 13:24 ` Vlastimil Babka
2016-06-02 18:58 ` Ebru Akagunduz
2016-06-02 18:58 ` Ebru Akagunduz
2016-06-03 1:00 ` Sergey Senozhatsky [this message]
2016-06-03 1:00 ` Sergey Senozhatsky
2016-06-03 1:29 ` Sergey Senozhatsky
2016-06-03 1:29 ` Sergey Senozhatsky
2016-06-03 4:14 ` Sergey Senozhatsky
2016-06-03 4:14 ` Sergey Senozhatsky
2016-06-03 12:28 ` [PATCH] mm, thp: fix locking inconsistency in collapse_huge_page Ebru Akagunduz
2016-06-03 12:28 ` Ebru Akagunduz
2016-06-06 13:05 ` Vlastimil Babka
2016-06-06 13:05 ` Vlastimil Babka
2016-06-09 3:51 ` Sergey Senozhatsky
2016-06-09 3:51 ` Sergey Senozhatsky
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=20160603010036.GA464@swordfish \
--to=sergey.senozhatsky.work@gmail.com \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=ebru.akagunduz@gmail.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-next@vger.kernel.org \
--cc=mhocko@kernel.org \
--cc=riel@redhat.com \
--cc=sfr@canb.auug.org.au \
--cc=vbabka@suse.cz \
/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.