Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1410798 > unrolled thread

linux-next: Tree for Jun 1

Started byStephen Rothwell <sfr@canb.auug.org.au>
First post2016-06-01 05:20 +0200
Last post2016-06-09 06:00 +0200
Articles 8 on this page of 28 — 7 participants

Back to article view | Back to linux.kernel

Contents

  linux-next: Tree for Jun 1 Stephen Rothwell <sfr@canb.auug.org.au> - 2016-06-01 05:20 +0200
    [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-06-02 04:00 +0200
      Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Michal Hocko <mhocko@kernel.org> - 2016-06-02 11:30 +0200
        Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-06-02 14:10 +0200
          Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Michal Hocko <mhocko@kernel.org> - 2016-06-02 14:30 +0200
            Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Andrea Arcangeli <aarcange@redhat.com> - 2016-06-03 16:00 +0200
              Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Michal Hocko <mhocko@kernel.org> - 2016-06-03 16:50 +0200
                Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Andrea Arcangeli <aarcange@redhat.com> - 2016-06-03 17:20 +0200
                  Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Michal Hocko <mhocko@kernel.org> - 2016-06-07 09:40 +0200
                  Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Vlastimil Babka <vbabka@suse.cz> - 2016-06-08 10:20 +0200
        Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-06-03 09:20 +0200
          Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Michal Hocko <mhocko@kernel.org> - 2016-06-03 09:30 +0200
            Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-06-03 10:50 +0200
              Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Michal Hocko <mhocko@kernel.org> - 2016-06-03 12:00 +0200
                Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Michal Hocko <mhocko@kernel.org> - 2016-06-03 12:10 +0200
                  Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-06-03 15:40 +0200
                    Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Michal Hocko <mhocko@kernel.org> - 2016-06-03 15:50 +0200
                      Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Michal Hocko <mhocko@kernel.org> - 2016-06-03 15:50 +0200
                        Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-06-04 10:00 +0200
                          Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Michal Hocko <mhocko@kernel.org> - 2016-06-06 10:40 +0200
      Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Vlastimil Babka <vbabka@suse.cz> - 2016-06-02 15:30 +0200
        Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Ebru Akagunduz <ebru.akagunduz@gmail.com> - 2016-06-02 21:00 +0200
          Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-06-03 03:10 +0200
            Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-06-03 03:30 +0200
              Re: [linux-next: Tree for Jun 1] __khugepaged_exit  rwsem_down_write_failed lockup Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-06-03 06:20 +0200
        [PATCH] mm, thp: fix locking inconsistency in collapse_huge_page Ebru Akagunduz <ebru.akagunduz@gmail.com> - 2016-06-03 14:30 +0200
          Re: [PATCH] mm, thp: fix locking inconsistency in collapse_huge_page Vlastimil Babka <vbabka@suse.cz> - 2016-06-06 15:10 +0200
            Re: [PATCH] mm, thp: fix locking inconsistency in collapse_huge_page Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-06-09 06:00 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1412227 — Re: [linux-next: Tree for Jun 1] __khugepaged_exit rwsem_down_write_failed lockup

FromVlastimil Babka <vbabka@suse.cz>
Date2016-06-02 15:30 +0200
SubjectRe: [linux-next: Tree for Jun 1] __khugepaged_exit rwsem_down_write_failed lockup
Message-ID<rFARb-6rl-1@gated-at.bofh.it>
In reply to#1411760
[+CC's]

On 06/02/2016 03:48 AM, Sergey Senozhatsky wrote:
> On (06/01/16 13:11), Stephen Rothwell wrote:
>> Hi all,
>>
>> Changes since 20160531:
>>
>> My fixes tree contains:
>>
>>   of: silence warnings due to max() usage
>>
>> The arm tree gained a conflict against Linus' tree.
>>
>> Non-merge commits (relative to Linus' tree): 1100
>>  936 files changed, 38159 insertions(+), 17475 deletions(-)
>
> Hello,
>
> the cc1 process ended up in DN state during kernel -j4 compilation.
>
> ...
> [ 2856.323052] INFO: task cc1:4582 blocked for more than 21 seconds.
> [ 2856.323055]       Not tainted 4.7.0-rc1-next-20160601-dbg-00012-g52c180e-dirty #453
> [ 2856.323056] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 2856.323059] cc1             D ffff880057e9fd78     0  4582   4575 0x00000000
> [ 2856.323062]  ffff880057e9fd78 ffff880057e08000 ffff880057e9fd90 ffff880057ea0000
> [ 2856.323065]  ffff88005dc3dc68 ffffffff00000001 ffff880057e09500 ffff88005dc3dc80
> [ 2856.323067]  ffff880057e9fd90 ffffffff81441e33 ffff88005dc3dc68 ffff880057e9fe00
> [ 2856.323068] Call Trace:
> [ 2856.323074]  [<ffffffff81441e33>] schedule+0x83/0x98
> [ 2856.323077]  [<ffffffff81443d9b>] rwsem_down_write_failed+0x18e/0x1d3
> [ 2856.323080]  [<ffffffff810a87cf>] ? unlock_page+0x2b/0x2d
> [ 2856.323083]  [<ffffffff811bdb77>] call_rwsem_down_write_failed+0x17/0x30
> [ 2856.323084]  [<ffffffff811bdb77>] ? call_rwsem_down_write_failed+0x17/0x30
> [ 2856.323086]  [<ffffffff81443630>] down_write+0x1f/0x2e
> [ 2856.323089]  [<ffffffff810ea4f3>] __khugepaged_exit+0x104/0x11a
> [ 2856.323091]  [<ffffffff8103702a>] mmput+0x29/0xc5
> [ 2856.323093]  [<ffffffff8103bbd8>] do_exit+0x34c/0x894
> [ 2856.323095]  [<ffffffff8102f9e0>] ? __do_page_fault+0x2f7/0x399
> [ 2856.323097]  [<ffffffff8103c188>] do_group_exit+0x3c/0x98
> [ 2856.323099]  [<ffffffff8103c1f3>] SyS_exit_group+0xf/0xf
> [ 2856.323101]  [<ffffffff81444cdb>] entry_SYSCALL_64_fastpath+0x13/0x8f
>
> [ 2877.322853] INFO: task cc1:4582 blocked for more than 21 seconds.
> [ 2877.322858]       Not tainted 4.7.0-rc1-next-20160601-dbg-00012-g52c180e-dirty #453
> [ 2877.322858] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 2877.322861] cc1             D ffff880057e9fd78     0  4582   4575 0x00000000
> [ 2877.322865]  ffff880057e9fd78 ffff880057e08000 ffff880057e9fd90 ffff880057ea0000
> [ 2877.322867]  ffff88005dc3dc68 ffffffff00000001 ffff880057e09500 ffff88005dc3dc80
> [ 2877.322867]  ffff880057e9fd90 ffffffff81441e33 ffff88005dc3dc68 ffff880057e9fe00
> [ 2877.322870] Call Trace:
> [ 2877.322875]  [<ffffffff81441e33>] schedule+0x83/0x98
> [ 2877.322878]  [<ffffffff81443d9b>] rwsem_down_write_failed+0x18e/0x1d3
> [ 2877.322881]  [<ffffffff810a87cf>] ? unlock_page+0x2b/0x2d
> [ 2877.322884]  [<ffffffff811bdb77>] call_rwsem_down_write_failed+0x17/0x30
> [ 2877.322885]  [<ffffffff811bdb77>] ? call_rwsem_down_write_failed+0x17/0x30
> [ 2877.322887]  [<ffffffff81443630>] down_write+0x1f/0x2e
> [ 2877.322890]  [<ffffffff810ea4f3>] __khugepaged_exit+0x104/0x11a
> [ 2877.322892]  [<ffffffff8103702a>] mmput+0x29/0xc5
> [ 2877.322894]  [<ffffffff8103bbd8>] do_exit+0x34c/0x894
> [ 2877.322896]  [<ffffffff8102f9e0>] ? __do_page_fault+0x2f7/0x399
> [ 2877.322898]  [<ffffffff8103c188>] do_group_exit+0x3c/0x98
> [ 2877.322900]  [<ffffffff8103c1f3>] SyS_exit_group+0xf/0xf
> [ 2877.322902]  [<ffffffff81444cdb>] entry_SYSCALL_64_fastpath+0x13/0x8f

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?

[toc] | [prev] | [next] | [standalone]

#1412501 — Re: [linux-next: Tree for Jun 1] __khugepaged_exit rwsem_down_write_failed lockup

FromEbru Akagunduz <ebru.akagunduz@gmail.com>
Date2016-06-02 21:00 +0200
SubjectRe: [linux-next: Tree for Jun 1] __khugepaged_exit rwsem_down_write_failed lockup
Message-ID<rFG0x-1gi-11@gated-at.bofh.it>
In reply to#1412227
On Thu, Jun 02, 2016 at 03:24:05PM +0200, Vlastimil Babka wrote:
> [+CC's]
> 
> On 06/02/2016 03:48 AM, Sergey Senozhatsky wrote:
> >On (06/01/16 13:11), Stephen Rothwell wrote:
> >>Hi all,
> >>
> >>Changes since 20160531:
> >>
> >>My fixes tree contains:
> >>
> >>  of: silence warnings due to max() usage
> >>
> >>The arm tree gained a conflict against Linus' tree.
> >>
> >>Non-merge commits (relative to Linus' tree): 1100
> >> 936 files changed, 38159 insertions(+), 17475 deletions(-)
> >
> >Hello,
> >
> >the cc1 process ended up in DN state during kernel -j4 compilation.
> >
> >...
> >[ 2856.323052] INFO: task cc1:4582 blocked for more than 21 seconds.
> >[ 2856.323055]       Not tainted 4.7.0-rc1-next-20160601-dbg-00012-g52c180e-dirty #453
> >[ 2856.323056] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> >[ 2856.323059] cc1             D ffff880057e9fd78     0  4582   4575 0x00000000
> >[ 2856.323062]  ffff880057e9fd78 ffff880057e08000 ffff880057e9fd90 ffff880057ea0000
> >[ 2856.323065]  ffff88005dc3dc68 ffffffff00000001 ffff880057e09500 ffff88005dc3dc80
> >[ 2856.323067]  ffff880057e9fd90 ffffffff81441e33 ffff88005dc3dc68 ffff880057e9fe00
> >[ 2856.323068] Call Trace:
> >[ 2856.323074]  [<ffffffff81441e33>] schedule+0x83/0x98
> >[ 2856.323077]  [<ffffffff81443d9b>] rwsem_down_write_failed+0x18e/0x1d3
> >[ 2856.323080]  [<ffffffff810a87cf>] ? unlock_page+0x2b/0x2d
> >[ 2856.323083]  [<ffffffff811bdb77>] call_rwsem_down_write_failed+0x17/0x30
> >[ 2856.323084]  [<ffffffff811bdb77>] ? call_rwsem_down_write_failed+0x17/0x30
> >[ 2856.323086]  [<ffffffff81443630>] down_write+0x1f/0x2e
> >[ 2856.323089]  [<ffffffff810ea4f3>] __khugepaged_exit+0x104/0x11a
> >[ 2856.323091]  [<ffffffff8103702a>] mmput+0x29/0xc5
> >[ 2856.323093]  [<ffffffff8103bbd8>] do_exit+0x34c/0x894
> >[ 2856.323095]  [<ffffffff8102f9e0>] ? __do_page_fault+0x2f7/0x399
> >[ 2856.323097]  [<ffffffff8103c188>] do_group_exit+0x3c/0x98
> >[ 2856.323099]  [<ffffffff8103c1f3>] SyS_exit_group+0xf/0xf
> >[ 2856.323101]  [<ffffffff81444cdb>] entry_SYSCALL_64_fastpath+0x13/0x8f
> >
> >[ 2877.322853] INFO: task cc1:4582 blocked for more than 21 seconds.
> >[ 2877.322858]       Not tainted 4.7.0-rc1-next-20160601-dbg-00012-g52c180e-dirty #453
> >[ 2877.322858] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> >[ 2877.322861] cc1             D ffff880057e9fd78     0  4582   4575 0x00000000
> >[ 2877.322865]  ffff880057e9fd78 ffff880057e08000 ffff880057e9fd90 ffff880057ea0000
> >[ 2877.322867]  ffff88005dc3dc68 ffffffff00000001 ffff880057e09500 ffff88005dc3dc80
> >[ 2877.322867]  ffff880057e9fd90 ffffffff81441e33 ffff88005dc3dc68 ffff880057e9fe00
> >[ 2877.322870] Call Trace:
> >[ 2877.322875]  [<ffffffff81441e33>] schedule+0x83/0x98
> >[ 2877.322878]  [<ffffffff81443d9b>] rwsem_down_write_failed+0x18e/0x1d3
> >[ 2877.322881]  [<ffffffff810a87cf>] ? unlock_page+0x2b/0x2d
> >[ 2877.322884]  [<ffffffff811bdb77>] call_rwsem_down_write_failed+0x17/0x30
> >[ 2877.322885]  [<ffffffff811bdb77>] ? call_rwsem_down_write_failed+0x17/0x30
> >[ 2877.322887]  [<ffffffff81443630>] down_write+0x1f/0x2e
> >[ 2877.322890]  [<ffffffff810ea4f3>] __khugepaged_exit+0x104/0x11a
> >[ 2877.322892]  [<ffffffff8103702a>] mmput+0x29/0xc5
> >[ 2877.322894]  [<ffffffff8103bbd8>] do_exit+0x34c/0x894
> >[ 2877.322896]  [<ffffffff8102f9e0>] ? __do_page_fault+0x2f7/0x399
> >[ 2877.322898]  [<ffffffff8103c188>] do_group_exit+0x3c/0x98
> >[ 2877.322900]  [<ffffffff8103c1f3>] SyS_exit_group+0xf/0xf
> >[ 2877.322902]  [<ffffffff81444cdb>] entry_SYSCALL_64_fastpath+0x13/0x8f
> 
> 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.

Kind regards.

[toc] | [prev] | [next] | [standalone]

#1412667 — Re: [linux-next: Tree for Jun 1] __khugepaged_exit rwsem_down_write_failed lockup

FromSergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Date2016-06-03 03:10 +0200
SubjectRe: [linux-next: Tree for Jun 1] __khugepaged_exit rwsem_down_write_failed lockup
Message-ID<rFLMC-4Zt-9@gated-at.bofh.it>
In reply to#1412501
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

[toc] | [prev] | [next] | [standalone]

#1412679 — Re: [linux-next: Tree for Jun 1] __khugepaged_exit rwsem_down_write_failed lockup

FromSergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Date2016-06-03 03:30 +0200
SubjectRe: [linux-next: Tree for Jun 1] __khugepaged_exit rwsem_down_write_failed lockup
Message-ID<rFM5X-55O-11@gated-at.bofh.it>
In reply to#1412667
On (06/03/16 10:00), Sergey Senozhatsky wrote:
> 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);
> 			:		^^^^^^^^^

oh... it's in a loop

		for (_address = address; _address < address + HPAGE_PMD_NR*PAGE_SIZE;
						pte++, _address += PAGE_SIZE) {
			ret = do_swap_page()
			if (ret & VM_FAULT_RETRY) {
				down_read(&mm->mmap_sem);
				^^^^^^^^^
				...
			}
		}

so there can be multiple sem->count++ in __collapse_huge_page_swapin(),
and you don't know how many sem->count-- you need to do later? is this
correct or am I hallucinating?

	-ss

> 			:		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
>

[toc] | [prev] | [next] | [standalone]

#1412779 — Re: [linux-next: Tree for Jun 1] __khugepaged_exit rwsem_down_write_failed lockup

FromSergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Date2016-06-03 06:20 +0200
SubjectRe: [linux-next: Tree for Jun 1] __khugepaged_exit rwsem_down_write_failed lockup
Message-ID<rFOKt-6Pk-3@gated-at.bofh.it>
In reply to#1412679
On (06/03/16 10:29), Sergey Senozhatsky wrote:
> > 	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);
> > 			:		^^^^^^^^^
> 
> oh... it's in a loop
> 
> 		for (_address = address; _address < address + HPAGE_PMD_NR*PAGE_SIZE;
> 						pte++, _address += PAGE_SIZE) {
> 			ret = do_swap_page()
> 			if (ret & VM_FAULT_RETRY) {
> 				down_read(&mm->mmap_sem);
> 				^^^^^^^^^
> 				...
> 			}
> 		}
> 
> so there can be multiple sem->count++ in __collapse_huge_page_swapin(),
> and you don't know how many sem->count-- you need to do later? is this
> correct or am I hallucinating?

No, I was wrong, sorry for the noise.

it's getting unlocked in

__collapse_huge_page_swapin()
	do_swap_page()
		lock_page_or_retry()
			if (flags & FAULT_FLAG_ALLOW_RETRY)
				up_read(&mm->mmap_sem);
	return VM_FAULT_RETRY

	-ss

[toc] | [prev] | [next] | [standalone]

#1413187 — [PATCH] mm, thp: fix locking inconsistency in collapse_huge_page

FromEbru Akagunduz <ebru.akagunduz@gmail.com>
Date2016-06-03 14:30 +0200
Subject[PATCH] mm, thp: fix locking inconsistency in collapse_huge_page
Message-ID<rFWoG-36Q-31@gated-at.bofh.it>
In reply to#1412227
After creating revalidate vma function, locking inconsistency occured
due to directing the code path to wrong label. This patch directs
to correct label and fix the inconsistency.

Related commit that caused inconsistency:
http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=da4360877094368f6dfe75bbe804b0f0a5d575b0

Signed-off-by: Ebru Akagunduz <ebru.akagunduz@gmail.com>
---
 mm/huge_memory.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 292cedd..8043d91 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2493,13 +2493,18 @@ static void collapse_huge_page(struct mm_struct *mm,
 	curr_allocstall = sum_vm_event(ALLOCSTALL);
 	down_read(&mm->mmap_sem);
 	result = hugepage_vma_revalidate(mm, vma, address);
-	if (result)
-		goto out;
+	if (result) {
+		mem_cgroup_cancel_charge(new_page, memcg, true);
+		up_read(&mm->mmap_sem);
+		goto out_nolock;
+	}
 
 	pmd = mm_find_pmd(mm, address);
 	if (!pmd) {
 		result = SCAN_PMD_NULL;
-		goto out;
+		mem_cgroup_cancel_charge(new_page, memcg, true);
+		up_read(&mm->mmap_sem);
+		goto out_nolock;
 	}
 
 	/*
@@ -2513,8 +2518,9 @@ static void collapse_huge_page(struct mm_struct *mm,
 		 * label out. Continuing to collapse causes inconsistency.
 		 */
 		if (!__collapse_huge_page_swapin(mm, vma, address, pmd)) {
+			mem_cgroup_cancel_charge(new_page, memcg, true);
 			up_read(&mm->mmap_sem);
-			goto out;
+			goto out_nolock;
 		}
 	}
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1415011 — Re: [PATCH] mm, thp: fix locking inconsistency in collapse_huge_page

FromVlastimil Babka <vbabka@suse.cz>
Date2016-06-06 15:10 +0200
SubjectRe: [PATCH] mm, thp: fix locking inconsistency in collapse_huge_page
Message-ID<rH2s1-5nS-5@gated-at.bofh.it>
In reply to#1413187
On 06/03/2016 02:28 PM, Ebru Akagunduz wrote:
> After creating revalidate vma function, locking inconsistency occured
> due to directing the code path to wrong label. This patch directs
> to correct label and fix the inconsistency.
>
> Related commit that caused inconsistency:
> http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=da4360877094368f6dfe75bbe804b0f0a5d575b0
>
> Signed-off-by: Ebru Akagunduz <ebru.akagunduz@gmail.com>

I think this does fix the inconsistency, thanks.

But looking at collapse_huge_page() as of latest -next, I wonder if 
there's another problem:

pmd = mm_find_pmd(mm, address);
...
up_read(&mm->mmap_sem);
down_write(&mm->mmap_sem);
hugepage_vma_revalidate(mm, address);
...
pte = pte_offset_map(pmd, address);

What guarantees that 'pmd' is still valid?

Vlastimil

[toc] | [prev] | [next] | [standalone]

#1418049 — Re: [PATCH] mm, thp: fix locking inconsistency in collapse_huge_page

FromSergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Date2016-06-09 06:00 +0200
SubjectRe: [PATCH] mm, thp: fix locking inconsistency in collapse_huge_page
Message-ID<rHZip-RC-3@gated-at.bofh.it>
In reply to#1415011
On (06/06/16 15:05), Vlastimil Babka wrote:
[..]
> I think this does fix the inconsistency, thanks.
> 
> But looking at collapse_huge_page() as of latest -next, I wonder if there's
> another problem:
> 
> pmd = mm_find_pmd(mm, address);
> ...
> up_read(&mm->mmap_sem);
> down_write(&mm->mmap_sem);
> hugepage_vma_revalidate(mm, address);
> ...
> pte = pte_offset_map(pmd, address);
> 
> What guarantees that 'pmd' is still valid?

the same question applied to __collapse_huge_page_swapin(), I think.

__collapse_huge_page_swapin(pmd)
	pte = pte_offset_map(pmd, address);
	do_swap_page(mm, vma, _address, pte, pmd...)
		up_read(&mm->mmap_sem);
	down_read(&mm->mmap_sem);
	pte = pte_offset_map(pmd, _address);

	-ss

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web