* mbind() fails to fail with EIO @ 2019-03-15 16:01 ` Cyril Hrubis 0 siblings, 0 replies; 24+ messages in thread From: Cyril Hrubis @ 2019-03-15 16:01 UTC (permalink / raw) To: linux-mm, linux-api; +Cc: ltp, Vlastimil Babka Hi! I've started to write tests for mbind() and found out that mbind() does not work as described in manual page in a case that page has been faulted on different node that we are asking it to bind to. Looks like this is working fine on older kernels. On my testing machine with 3.0 mbind() fails correctly with EIO but succeeds unexpectedly on newer kernels such as 4.12. What the test does is: * mmap() private mapping * fault it * find out on which node it is faulted on * mbind() it to a different node with MPOL_BIND and MPOL_MF_STRICT and expects to get EIO The test code can be seen and compiled from: https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/mbind/mbind02.c -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 24+ messages in thread
* [LTP] mbind() fails to fail with EIO @ 2019-03-15 16:01 ` Cyril Hrubis 0 siblings, 0 replies; 24+ messages in thread From: Cyril Hrubis @ 2019-03-15 16:01 UTC (permalink / raw) To: ltp Hi! I've started to write tests for mbind() and found out that mbind() does not work as described in manual page in a case that page has been faulted on different node that we are asking it to bind to. Looks like this is working fine on older kernels. On my testing machine with 3.0 mbind() fails correctly with EIO but succeeds unexpectedly on newer kernels such as 4.12. What the test does is: * mmap() private mapping * fault it * find out on which node it is faulted on * mbind() it to a different node with MPOL_BIND and MPOL_MF_STRICT and expects to get EIO The test code can be seen and compiled from: https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/mbind/mbind02.c -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: mbind() fails to fail with EIO 2019-03-15 16:01 ` [LTP] " Cyril Hrubis @ 2019-03-18 16:08 ` Qian Cai -1 siblings, 0 replies; 24+ messages in thread From: Qian Cai @ 2019-03-18 16:08 UTC (permalink / raw) To: Cyril Hrubis, linux-mm, linux-api; +Cc: ltp, Vlastimil Babka On Fri, 2019-03-15 at 17:01 +0100, Cyril Hrubis wrote: > Hi! > I've started to write tests for mbind() and found out that mbind() does > not work as described in manual page in a case that page has been > faulted on different node that we are asking it to bind to. Looks like > this is working fine on older kernels. On my testing machine with 3.0 > mbind() fails correctly with EIO but succeeds unexpectedly on newer > kernels such as 4.12. > > What the test does is: > > * mmap() private mapping > * fault it > * find out on which node it is faulted on > * mbind() it to a different node with MPOL_BIND and MPOL_MF_STRICT and > expects to get EIO > > The test code can be seen and compiled from: > > https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/mbind/m > bind02.c > I am too lazy to checkout the repository and compile the whole thing just to be able to reproduce. If you can make it a standalone program without LTP markups, I'd be happy to take a look. ^ permalink raw reply [flat|nested] 24+ messages in thread
* [LTP] mbind() fails to fail with EIO @ 2019-03-18 16:08 ` Qian Cai 0 siblings, 0 replies; 24+ messages in thread From: Qian Cai @ 2019-03-18 16:08 UTC (permalink / raw) To: ltp On Fri, 2019-03-15 at 17:01 +0100, Cyril Hrubis wrote: > Hi! > I've started to write tests for mbind() and found out that mbind() does > not work as described in manual page in a case that page has been > faulted on different node that we are asking it to bind to. Looks like > this is working fine on older kernels. On my testing machine with 3.0 > mbind() fails correctly with EIO but succeeds unexpectedly on newer > kernels such as 4.12. > > What the test does is: > > * mmap() private mapping > * fault it > * find out on which node it is faulted on > * mbind() it to a different node with MPOL_BIND and MPOL_MF_STRICT and > Â expects to get EIO > > The test code can be seen and compiled from: > > https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/mbind/m > bind02.c > I am too lazy to checkout the repository and compile the whole thing just to be able to reproduce. If you can make it a standalone program without LTP markups, I'd be happy to take a look. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: mbind() fails to fail with EIO 2019-03-18 16:08 ` [LTP] " Qian Cai @ 2019-03-19 12:59 ` Cyril Hrubis -1 siblings, 0 replies; 24+ messages in thread From: Cyril Hrubis @ 2019-03-19 12:59 UTC (permalink / raw) To: Qian Cai; +Cc: linux-mm, linux-api, ltp, Vlastimil Babka Hi! > I am too lazy to checkout the repository and compile the whole thing > just to be able to reproduce. If you can make it a standalone program > without LTP markups, I'd be happy to take a look. Sigh, I've spend last few years so working on the testsuite so that you can compile and run a single test from the checked out repository, we even have howto for that in the README.md. https://github.com/linux-test-project/ltp/#quick-guide-to-running-the-tests It shuld be really easy, so can you please give it a try? -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 24+ messages in thread
* [LTP] mbind() fails to fail with EIO @ 2019-03-19 12:59 ` Cyril Hrubis 0 siblings, 0 replies; 24+ messages in thread From: Cyril Hrubis @ 2019-03-19 12:59 UTC (permalink / raw) To: ltp Hi! > I am too lazy to checkout the repository and compile the whole thing > just to be able to reproduce. If you can make it a standalone program > without LTP markups, I'd be happy to take a look. Sigh, I've spend last few years so working on the testsuite so that you can compile and run a single test from the checked out repository, we even have howto for that in the README.md. https://github.com/linux-test-project/ltp/#quick-guide-to-running-the-tests It shuld be really easy, so can you please give it a try? -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: mbind() fails to fail with EIO 2019-03-15 16:01 ` [LTP] " Cyril Hrubis @ 2019-03-18 18:12 ` Yang Shi -1 siblings, 0 replies; 24+ messages in thread From: Yang Shi @ 2019-03-18 18:12 UTC (permalink / raw) To: Cyril Hrubis; +Cc: Linux MM, linux-api, ltp, Vlastimil Babka On Fri, Mar 15, 2019 at 9:02 AM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > I've started to write tests for mbind() and found out that mbind() does > not work as described in manual page in a case that page has been > faulted on different node that we are asking it to bind to. Looks like > this is working fine on older kernels. On my testing machine with 3.0 > mbind() fails correctly with EIO but succeeds unexpectedly on newer > kernels such as 4.12. > > What the test does is: > > * mmap() private mapping > * fault it > * find out on which node it is faulted on > * mbind() it to a different node with MPOL_BIND and MPOL_MF_STRICT and > expects to get EIO It looks the behavior was changed since v4.0 by the below commit: 6f4576e3687b mempolicy: apply page table walker on queue_pages_range() The new queue_pages_to_pte_range() doesn't return -EIO anymore. Could you please try the below patch (5.1-rc1 based)? diff --git a/mm/mempolicy.c b/mm/mempolicy.c index abe7a67..6ba45aa 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -521,11 +521,14 @@ static int queue_pages_pte_range(pmd_t *pmd, unsigned long addr, continue; if (!queue_pages_required(page, qp)) continue; - migrate_page_add(page, qp->pagelist, flags); + if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) + migrate_page_add(page, qp->pagelist, flags); + else + break; } pte_unmap_unlock(pte - 1, ptl); cond_resched(); - return 0; + return addr != end ? -EIO : 0; } static int queue_pages_hugetlb(pte_t *pte, unsigned long hmask, Yang > > The test code can be seen and compiled from: > > https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/mbind/mbind02.c > > -- > Cyril Hrubis > chrubis@suse.cz > ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [LTP] mbind() fails to fail with EIO @ 2019-03-18 18:12 ` Yang Shi 0 siblings, 0 replies; 24+ messages in thread From: Yang Shi @ 2019-03-18 18:12 UTC (permalink / raw) To: ltp On Fri, Mar 15, 2019 at 9:02 AM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > I've started to write tests for mbind() and found out that mbind() does > not work as described in manual page in a case that page has been > faulted on different node that we are asking it to bind to. Looks like > this is working fine on older kernels. On my testing machine with 3.0 > mbind() fails correctly with EIO but succeeds unexpectedly on newer > kernels such as 4.12. > > What the test does is: > > * mmap() private mapping > * fault it > * find out on which node it is faulted on > * mbind() it to a different node with MPOL_BIND and MPOL_MF_STRICT and > expects to get EIO It looks the behavior was changed since v4.0 by the below commit: 6f4576e3687b mempolicy: apply page table walker on queue_pages_range() The new queue_pages_to_pte_range() doesn't return -EIO anymore. Could you please try the below patch (5.1-rc1 based)? diff --git a/mm/mempolicy.c b/mm/mempolicy.c index abe7a67..6ba45aa 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -521,11 +521,14 @@ static int queue_pages_pte_range(pmd_t *pmd, unsigned long addr, continue; if (!queue_pages_required(page, qp)) continue; - migrate_page_add(page, qp->pagelist, flags); + if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) + migrate_page_add(page, qp->pagelist, flags); + else + break; } pte_unmap_unlock(pte - 1, ptl); cond_resched(); - return 0; + return addr != end ? -EIO : 0; } static int queue_pages_hugetlb(pte_t *pte, unsigned long hmask, Yang > > The test code can be seen and compiled from: > > https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/mbind/mbind02.c > > -- > Cyril Hrubis > chrubis@suse.cz > ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: mbind() fails to fail with EIO 2019-03-18 18:12 ` [LTP] " Yang Shi @ 2019-03-19 13:27 ` Oscar Salvador -1 siblings, 0 replies; 24+ messages in thread From: Oscar Salvador @ 2019-03-19 13:27 UTC (permalink / raw) To: Yang Shi Cc: Cyril Hrubis, Linux MM, linux-api, ltp, Vlastimil Babka, kirill.shutemov +CC Kirill On Mon, Mar 18, 2019 at 11:12:19AM -0700, Yang Shi wrote: > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index abe7a67..6ba45aa 100644 > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -521,11 +521,14 @@ static int queue_pages_pte_range(pmd_t *pmd, > unsigned long addr, > continue; > if (!queue_pages_required(page, qp)) > continue; > - migrate_page_add(page, qp->pagelist, flags); > + if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) > + migrate_page_add(page, qp->pagelist, flags); > + else > + break; > } > pte_unmap_unlock(pte - 1, ptl); > cond_resched(); > - return 0; > + return addr != end ? -EIO : 0; > } > > static int queue_pages_hugetlb(pte_t *pte, unsigned long hmask, This alone is not going to help. The problem is that we do skip the vma early in queue_pages_test_walk() in case MPOL_MF_MOVE and MPOL_MF_MOVE_ALL are not set. walk_page_range walk_page_test queue_pages_test_walk ... ... /* queue pages from current vma */ if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) return 0; return 1; So, we skip the vma and keep going. Before ("77bf45e78050: mempolicy: do not try to queue pages from !vma_migratable()"), queue_pages_test_walk() would not have skipped the vma in case we had MPOL_MF_STRICT or MPOL_MF_MOVE | MPOL_MF_MOVE_ALL. I did not give it a lot of thought, but it seems to me that we might need to reach queue_pages_to_pte_range() in order to see whether the page is in the required node or not by calling queue_pages_required(), and if it is not, check for MPOL_MF_MOVE | MPOL_MF_MOVE_ALL like the above patch does, so we would be able to return -EIO. That would imply that we would need to re-add MPOL_MF_STRICT in queue_pages_test_walk(). -- Oscar Salvador SUSE L3 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [LTP] mbind() fails to fail with EIO @ 2019-03-19 13:27 ` Oscar Salvador 0 siblings, 0 replies; 24+ messages in thread From: Oscar Salvador @ 2019-03-19 13:27 UTC (permalink / raw) To: ltp +CC Kirill On Mon, Mar 18, 2019 at 11:12:19AM -0700, Yang Shi wrote: > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index abe7a67..6ba45aa 100644 > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -521,11 +521,14 @@ static int queue_pages_pte_range(pmd_t *pmd, > unsigned long addr, > continue; > if (!queue_pages_required(page, qp)) > continue; > - migrate_page_add(page, qp->pagelist, flags); > + if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) > + migrate_page_add(page, qp->pagelist, flags); > + else > + break; > } > pte_unmap_unlock(pte - 1, ptl); > cond_resched(); > - return 0; > + return addr != end ? -EIO : 0; > } > > static int queue_pages_hugetlb(pte_t *pte, unsigned long hmask, This alone is not going to help. The problem is that we do skip the vma early in queue_pages_test_walk() in case MPOL_MF_MOVE and MPOL_MF_MOVE_ALL are not set. walk_page_range walk_page_test queue_pages_test_walk ... ... /* queue pages from current vma */ if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) return 0; return 1; So, we skip the vma and keep going. Before ("77bf45e78050: mempolicy: do not try to queue pages from !vma_migratable()"), queue_pages_test_walk() would not have skipped the vma in case we had MPOL_MF_STRICT or MPOL_MF_MOVE | MPOL_MF_MOVE_ALL. I did not give it a lot of thought, but it seems to me that we might need to reach queue_pages_to_pte_range() in order to see whether the page is in the required node or not by calling queue_pages_required(), and if it is not, check for MPOL_MF_MOVE | MPOL_MF_MOVE_ALL like the above patch does, so we would be able to return -EIO. That would imply that we would need to re-add MPOL_MF_STRICT in queue_pages_test_walk(). -- Oscar Salvador SUSE L3 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: mbind() fails to fail with EIO 2019-03-19 13:27 ` [LTP] " Oscar Salvador @ 2019-03-19 14:26 ` Kirill A. Shutemov -1 siblings, 0 replies; 24+ messages in thread From: Kirill A. Shutemov @ 2019-03-19 14:26 UTC (permalink / raw) To: Oscar Salvador Cc: Yang Shi, Cyril Hrubis, Linux MM, linux-api, ltp, Vlastimil Babka, kirill.shutemov On Tue, Mar 19, 2019 at 02:27:33PM +0100, Oscar Salvador wrote: > +CC Kirill > > On Mon, Mar 18, 2019 at 11:12:19AM -0700, Yang Shi wrote: > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > > index abe7a67..6ba45aa 100644 > > --- a/mm/mempolicy.c > > +++ b/mm/mempolicy.c > > @@ -521,11 +521,14 @@ static int queue_pages_pte_range(pmd_t *pmd, > > unsigned long addr, > > continue; > > if (!queue_pages_required(page, qp)) > > continue; > > - migrate_page_add(page, qp->pagelist, flags); > > + if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) > > + migrate_page_add(page, qp->pagelist, flags); > > + else > > + break; > > } > > pte_unmap_unlock(pte - 1, ptl); > > cond_resched(); > > - return 0; > > + return addr != end ? -EIO : 0; > > } > > > > static int queue_pages_hugetlb(pte_t *pte, unsigned long hmask, > > This alone is not going to help. > > The problem is that we do skip the vma early in queue_pages_test_walk() in > case MPOL_MF_MOVE and MPOL_MF_MOVE_ALL are not set. > > walk_page_range > walk_page_test > queue_pages_test_walk > > ... > ... > /* queue pages from current vma */ > if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) > return 0; > return 1; > > So, we skip the vma and keep going. > > Before ("77bf45e78050: mempolicy: do not try to queue pages from !vma_migratable()"), > queue_pages_test_walk() would not have skipped the vma in case we had MPOL_MF_STRICT > or MPOL_MF_MOVE | MPOL_MF_MOVE_ALL. > > I did not give it a lot of thought, but it seems to me that we might need to reach > queue_pages_to_pte_range() in order to see whether the page is in the required node > or not by calling queue_pages_required(), and if it is not, check for > MPOL_MF_MOVE | MPOL_MF_MOVE_ALL like the above patch does, so we would be able to > return -EIO. > That would imply that we would need to re-add MPOL_MF_STRICT in queue_pages_test_walk(). That's all sounds reasonable. We only need to make sure the bug fixed by 77bf45e78050 will not be re-introduced. -- Kirill A. Shutemov ^ permalink raw reply [flat|nested] 24+ messages in thread
* [LTP] mbind() fails to fail with EIO @ 2019-03-19 14:26 ` Kirill A. Shutemov 0 siblings, 0 replies; 24+ messages in thread From: Kirill A. Shutemov @ 2019-03-19 14:26 UTC (permalink / raw) To: ltp On Tue, Mar 19, 2019 at 02:27:33PM +0100, Oscar Salvador wrote: > +CC Kirill > > On Mon, Mar 18, 2019 at 11:12:19AM -0700, Yang Shi wrote: > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > > index abe7a67..6ba45aa 100644 > > --- a/mm/mempolicy.c > > +++ b/mm/mempolicy.c > > @@ -521,11 +521,14 @@ static int queue_pages_pte_range(pmd_t *pmd, > > unsigned long addr, > > continue; > > if (!queue_pages_required(page, qp)) > > continue; > > - migrate_page_add(page, qp->pagelist, flags); > > + if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) > > + migrate_page_add(page, qp->pagelist, flags); > > + else > > + break; > > } > > pte_unmap_unlock(pte - 1, ptl); > > cond_resched(); > > - return 0; > > + return addr != end ? -EIO : 0; > > } > > > > static int queue_pages_hugetlb(pte_t *pte, unsigned long hmask, > > This alone is not going to help. > > The problem is that we do skip the vma early in queue_pages_test_walk() in > case MPOL_MF_MOVE and MPOL_MF_MOVE_ALL are not set. > > walk_page_range > walk_page_test > queue_pages_test_walk > > ... > ... > /* queue pages from current vma */ > if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) > return 0; > return 1; > > So, we skip the vma and keep going. > > Before ("77bf45e78050: mempolicy: do not try to queue pages from !vma_migratable()"), > queue_pages_test_walk() would not have skipped the vma in case we had MPOL_MF_STRICT > or MPOL_MF_MOVE | MPOL_MF_MOVE_ALL. > > I did not give it a lot of thought, but it seems to me that we might need to reach > queue_pages_to_pte_range() in order to see whether the page is in the required node > or not by calling queue_pages_required(), and if it is not, check for > MPOL_MF_MOVE | MPOL_MF_MOVE_ALL like the above patch does, so we would be able to > return -EIO. > That would imply that we would need to re-add MPOL_MF_STRICT in queue_pages_test_walk(). That's all sounds reasonable. We only need to make sure the bug fixed by 77bf45e78050 will not be re-introduced. -- Kirill A. Shutemov ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: mbind() fails to fail with EIO 2019-03-19 14:26 ` [LTP] " Kirill A. Shutemov @ 2019-03-19 14:30 ` Cyril Hrubis -1 siblings, 0 replies; 24+ messages in thread From: Cyril Hrubis @ 2019-03-19 14:30 UTC (permalink / raw) To: Kirill A. Shutemov Cc: Oscar Salvador, Yang Shi, Linux MM, linux-api, ltp, Vlastimil Babka, kirill.shutemov Hi! > That's all sounds reasonable. > > We only need to make sure the bug fixed by 77bf45e78050 will not be > re-introduced. Well, we can turn the reproducer from that commit into an automated test, that's the only way to make sure bugs are not reintroduced anyways... -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 24+ messages in thread
* [LTP] mbind() fails to fail with EIO @ 2019-03-19 14:30 ` Cyril Hrubis 0 siblings, 0 replies; 24+ messages in thread From: Cyril Hrubis @ 2019-03-19 14:30 UTC (permalink / raw) To: ltp Hi! > That's all sounds reasonable. > > We only need to make sure the bug fixed by 77bf45e78050 will not be > re-introduced. Well, we can turn the reproducer from that commit into an automated test, that's the only way to make sure bugs are not reintroduced anyways... -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: mbind() fails to fail with EIO 2019-03-19 14:26 ` [LTP] " Kirill A. Shutemov @ 2019-03-19 14:41 ` Oscar Salvador -1 siblings, 0 replies; 24+ messages in thread From: Oscar Salvador @ 2019-03-19 14:41 UTC (permalink / raw) To: Kirill A. Shutemov Cc: Yang Shi, Cyril Hrubis, Linux MM, linux-api, ltp, Vlastimil Babka, kirill.shutemov On Tue, Mar 19, 2019 at 05:26:39PM +0300, Kirill A. Shutemov wrote: > That's all sounds reasonable. > > We only need to make sure the bug fixed by 77bf45e78050 will not be > re-introduced. I gave it a spin with the below patch. Your testcase works (so the bug is not re-introduced), and we get -EIO when running the ltp test [1]. So unless I am missing something, it should be enough. diff --git a/mm/mempolicy.c b/mm/mempolicy.c index af171ccb56a2..b192b13460f0 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -502,11 +508,16 @@ static int queue_pages_pte_range(pmd_t *pmd, unsigned long addr, continue; if (!queue_pages_required(page, qp)) continue; - migrate_page_add(page, qp->pagelist, flags); + if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) + migrate_page_add(page, qp->pagelist, flags); + else + break; } pte_unmap_unlock(pte - 1, ptl); cond_resched(); - return 0; + return addr != end ? -EIO : 0; } static int queue_pages_hugetlb(pte_t *pte, unsigned long hmask, @@ -603,7 +614,8 @@ static int queue_pages_test_walk(unsigned long start, unsigned long end, } /* queue pages from current vma */ - if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) + if ((flags & MPOL_MF_STRICT) || + (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) return 0; [1] https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/mbind/mbind02.c -- Oscar Salvador SUSE L3 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [LTP] mbind() fails to fail with EIO @ 2019-03-19 14:41 ` Oscar Salvador 0 siblings, 0 replies; 24+ messages in thread From: Oscar Salvador @ 2019-03-19 14:41 UTC (permalink / raw) To: ltp On Tue, Mar 19, 2019 at 05:26:39PM +0300, Kirill A. Shutemov wrote: > That's all sounds reasonable. > > We only need to make sure the bug fixed by 77bf45e78050 will not be > re-introduced. I gave it a spin with the below patch. Your testcase works (so the bug is not re-introduced), and we get -EIO when running the ltp test [1]. So unless I am missing something, it should be enough. diff --git a/mm/mempolicy.c b/mm/mempolicy.c index af171ccb56a2..b192b13460f0 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -502,11 +508,16 @@ static int queue_pages_pte_range(pmd_t *pmd, unsigned long addr, continue; if (!queue_pages_required(page, qp)) continue; - migrate_page_add(page, qp->pagelist, flags); + if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) + migrate_page_add(page, qp->pagelist, flags); + else + break; } pte_unmap_unlock(pte - 1, ptl); cond_resched(); - return 0; + return addr != end ? -EIO : 0; } static int queue_pages_hugetlb(pte_t *pte, unsigned long hmask, @@ -603,7 +614,8 @@ static int queue_pages_test_walk(unsigned long start, unsigned long end, } /* queue pages from current vma */ - if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) + if ((flags & MPOL_MF_STRICT) || + (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) return 0; [1] https://github.com/metan-ucw/ltp/blob/master/testcases/kernel/syscalls/mbind/mbind02.c -- Oscar Salvador SUSE L3 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: mbind() fails to fail with EIO 2019-03-19 14:41 ` [LTP] " Oscar Salvador @ 2019-03-19 14:52 ` Kirill A. Shutemov -1 siblings, 0 replies; 24+ messages in thread From: Kirill A. Shutemov @ 2019-03-19 14:52 UTC (permalink / raw) To: Oscar Salvador Cc: Yang Shi, Cyril Hrubis, Linux MM, linux-api, ltp, Vlastimil Babka, kirill.shutemov On Tue, Mar 19, 2019 at 03:41:33PM +0100, Oscar Salvador wrote: > On Tue, Mar 19, 2019 at 05:26:39PM +0300, Kirill A. Shutemov wrote: > > That's all sounds reasonable. > > > > We only need to make sure the bug fixed by 77bf45e78050 will not be > > re-introduced. > > I gave it a spin with the below patch. > Your testcase works (so the bug is not re-introduced), and we get -EIO > when running the ltp test [1]. > So unless I am missing something, it should be enough. Don't we need to bypass !vma_migratable(vma) check in queue_pages_test_walk() for MPOL_MF_STRICT? I mean user still might want to check if all pages are on the right not even the vma is not migratable. -- Kirill A. Shutemov ^ permalink raw reply [flat|nested] 24+ messages in thread
* [LTP] mbind() fails to fail with EIO @ 2019-03-19 14:52 ` Kirill A. Shutemov 0 siblings, 0 replies; 24+ messages in thread From: Kirill A. Shutemov @ 2019-03-19 14:52 UTC (permalink / raw) To: ltp On Tue, Mar 19, 2019 at 03:41:33PM +0100, Oscar Salvador wrote: > On Tue, Mar 19, 2019 at 05:26:39PM +0300, Kirill A. Shutemov wrote: > > That's all sounds reasonable. > > > > We only need to make sure the bug fixed by 77bf45e78050 will not be > > re-introduced. > > I gave it a spin with the below patch. > Your testcase works (so the bug is not re-introduced), and we get -EIO > when running the ltp test [1]. > So unless I am missing something, it should be enough. Don't we need to bypass !vma_migratable(vma) check in queue_pages_test_walk() for MPOL_MF_STRICT? I mean user still might want to check if all pages are on the right not even the vma is not migratable. -- Kirill A. Shutemov ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: mbind() fails to fail with EIO 2019-03-19 14:52 ` [LTP] " Kirill A. Shutemov @ 2019-03-19 15:10 ` Oscar Salvador -1 siblings, 0 replies; 24+ messages in thread From: Oscar Salvador @ 2019-03-19 15:10 UTC (permalink / raw) To: Kirill A. Shutemov Cc: Yang Shi, Cyril Hrubis, Linux MM, linux-api, ltp, Vlastimil Babka, kirill.shutemov On Tue, Mar 19, 2019 at 05:52:33PM +0300, Kirill A. Shutemov wrote: > On Tue, Mar 19, 2019 at 03:41:33PM +0100, Oscar Salvador wrote: > > On Tue, Mar 19, 2019 at 05:26:39PM +0300, Kirill A. Shutemov wrote: > > > That's all sounds reasonable. > > > > > > We only need to make sure the bug fixed by 77bf45e78050 will not be > > > re-introduced. > > > > I gave it a spin with the below patch. > > Your testcase works (so the bug is not re-introduced), and we get -EIO > > when running the ltp test [1]. > > So unless I am missing something, it should be enough. > > Don't we need to bypass !vma_migratable(vma) check in > queue_pages_test_walk() for MPOL_MF_STRICT? I mean user still might want > to check if all pages are on the right not even the vma is not migratable. Yeah, I missed that. Then, I guess that we have to put the check into queue_pages_pte_range as well, and place it right before migrate_page_add(). So, if it is not placed in the node and is not migreatable, we return -EIO. -- Oscar Salvador SUSE L3 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [LTP] mbind() fails to fail with EIO @ 2019-03-19 15:10 ` Oscar Salvador 0 siblings, 0 replies; 24+ messages in thread From: Oscar Salvador @ 2019-03-19 15:10 UTC (permalink / raw) To: ltp On Tue, Mar 19, 2019 at 05:52:33PM +0300, Kirill A. Shutemov wrote: > On Tue, Mar 19, 2019 at 03:41:33PM +0100, Oscar Salvador wrote: > > On Tue, Mar 19, 2019 at 05:26:39PM +0300, Kirill A. Shutemov wrote: > > > That's all sounds reasonable. > > > > > > We only need to make sure the bug fixed by 77bf45e78050 will not be > > > re-introduced. > > > > I gave it a spin with the below patch. > > Your testcase works (so the bug is not re-introduced), and we get -EIO > > when running the ltp test [1]. > > So unless I am missing something, it should be enough. > > Don't we need to bypass !vma_migratable(vma) check in > queue_pages_test_walk() for MPOL_MF_STRICT? I mean user still might want > to check if all pages are on the right not even the vma is not migratable. Yeah, I missed that. Then, I guess that we have to put the check into queue_pages_pte_range as well, and place it right before migrate_page_add(). So, if it is not placed in the node and is not migreatable, we return -EIO. -- Oscar Salvador SUSE L3 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: mbind() fails to fail with EIO 2019-03-19 15:10 ` [LTP] " Oscar Salvador @ 2019-03-19 16:29 ` Yang Shi -1 siblings, 0 replies; 24+ messages in thread From: Yang Shi @ 2019-03-19 16:29 UTC (permalink / raw) To: Oscar Salvador Cc: Kirill A. Shutemov, Cyril Hrubis, Linux MM, linux-api, ltp, Vlastimil Babka, kirill.shutemov On Tue, Mar 19, 2019 at 8:10 AM Oscar Salvador <osalvador@suse.de> wrote: > > On Tue, Mar 19, 2019 at 05:52:33PM +0300, Kirill A. Shutemov wrote: > > On Tue, Mar 19, 2019 at 03:41:33PM +0100, Oscar Salvador wrote: > > > On Tue, Mar 19, 2019 at 05:26:39PM +0300, Kirill A. Shutemov wrote: > > > > That's all sounds reasonable. > > > > > > > > We only need to make sure the bug fixed by 77bf45e78050 will not be > > > > re-introduced. > > > > > > I gave it a spin with the below patch. > > > Your testcase works (so the bug is not re-introduced), and we get -EIO > > > when running the ltp test [1]. > > > So unless I am missing something, it should be enough. > > > > Don't we need to bypass !vma_migratable(vma) check in > > queue_pages_test_walk() for MPOL_MF_STRICT? I mean user still might want > > to check if all pages are on the right not even the vma is not migratable. > > Yeah, I missed that. > Then, I guess that we have to put the check into queue_pages_pte_range as well, > and place it right before migrate_page_add(). > So, if it is not placed in the node and is not migreatable, we return -EIO. Sorry, I didn't see this reply before I replied Kirill's email earlier. Yes, I agree, it should return -EIO too. Thanks, Yang > > -- > Oscar Salvador > SUSE L3 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [LTP] mbind() fails to fail with EIO @ 2019-03-19 16:29 ` Yang Shi 0 siblings, 0 replies; 24+ messages in thread From: Yang Shi @ 2019-03-19 16:29 UTC (permalink / raw) To: ltp On Tue, Mar 19, 2019 at 8:10 AM Oscar Salvador <osalvador@suse.de> wrote: > > On Tue, Mar 19, 2019 at 05:52:33PM +0300, Kirill A. Shutemov wrote: > > On Tue, Mar 19, 2019 at 03:41:33PM +0100, Oscar Salvador wrote: > > > On Tue, Mar 19, 2019 at 05:26:39PM +0300, Kirill A. Shutemov wrote: > > > > That's all sounds reasonable. > > > > > > > > We only need to make sure the bug fixed by 77bf45e78050 will not be > > > > re-introduced. > > > > > > I gave it a spin with the below patch. > > > Your testcase works (so the bug is not re-introduced), and we get -EIO > > > when running the ltp test [1]. > > > So unless I am missing something, it should be enough. > > > > Don't we need to bypass !vma_migratable(vma) check in > > queue_pages_test_walk() for MPOL_MF_STRICT? I mean user still might want > > to check if all pages are on the right not even the vma is not migratable. > > Yeah, I missed that. > Then, I guess that we have to put the check into queue_pages_pte_range as well, > and place it right before migrate_page_add(). > So, if it is not placed in the node and is not migreatable, we return -EIO. Sorry, I didn't see this reply before I replied Kirill's email earlier. Yes, I agree, it should return -EIO too. Thanks, Yang > > -- > Oscar Salvador > SUSE L3 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: mbind() fails to fail with EIO 2019-03-19 14:52 ` [LTP] " Kirill A. Shutemov @ 2019-03-19 16:25 ` Yang Shi -1 siblings, 0 replies; 24+ messages in thread From: Yang Shi @ 2019-03-19 16:25 UTC (permalink / raw) To: Kirill A. Shutemov Cc: Oscar Salvador, Cyril Hrubis, Linux MM, linux-api, ltp, Vlastimil Babka, kirill.shutemov On Tue, Mar 19, 2019 at 7:52 AM Kirill A. Shutemov <kirill@shutemov.name> wrote: > > On Tue, Mar 19, 2019 at 03:41:33PM +0100, Oscar Salvador wrote: > > On Tue, Mar 19, 2019 at 05:26:39PM +0300, Kirill A. Shutemov wrote: > > > That's all sounds reasonable. > > > > > > We only need to make sure the bug fixed by 77bf45e78050 will not be > > > re-introduced. > > > > I gave it a spin with the below patch. > > Your testcase works (so the bug is not re-introduced), and we get -EIO > > when running the ltp test [1]. > > So unless I am missing something, it should be enough. Thanks for adding the missing part. > > Don't we need to bypass !vma_migratable(vma) check in > queue_pages_test_walk() for MPOL_MF_STRICT? I mean user still might want > to check if all pages are on the right not even the vma is not migratable. I think we need. As long as there is "existing page was already on a node that does not follow the policy" with MPOL_MF_STRICT, it should return -EIO. So, even though the vma is not migratable it should check if the above condition is true or not. I will wrap all the stuff into a formal patch. Thanks, Yang > > -- > Kirill A. Shutemov ^ permalink raw reply [flat|nested] 24+ messages in thread
* [LTP] mbind() fails to fail with EIO @ 2019-03-19 16:25 ` Yang Shi 0 siblings, 0 replies; 24+ messages in thread From: Yang Shi @ 2019-03-19 16:25 UTC (permalink / raw) To: ltp On Tue, Mar 19, 2019 at 7:52 AM Kirill A. Shutemov <kirill@shutemov.name> wrote: > > On Tue, Mar 19, 2019 at 03:41:33PM +0100, Oscar Salvador wrote: > > On Tue, Mar 19, 2019 at 05:26:39PM +0300, Kirill A. Shutemov wrote: > > > That's all sounds reasonable. > > > > > > We only need to make sure the bug fixed by 77bf45e78050 will not be > > > re-introduced. > > > > I gave it a spin with the below patch. > > Your testcase works (so the bug is not re-introduced), and we get -EIO > > when running the ltp test [1]. > > So unless I am missing something, it should be enough. Thanks for adding the missing part. > > Don't we need to bypass !vma_migratable(vma) check in > queue_pages_test_walk() for MPOL_MF_STRICT? I mean user still might want > to check if all pages are on the right not even the vma is not migratable. I think we need. As long as there is "existing page was already on a node that does not follow the policy" with MPOL_MF_STRICT, it should return -EIO. So, even though the vma is not migratable it should check if the above condition is true or not. I will wrap all the stuff into a formal patch. Thanks, Yang > > -- > Kirill A. Shutemov ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2019-03-19 16:30 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-15 16:01 mbind() fails to fail with EIO Cyril Hrubis 2019-03-15 16:01 ` [LTP] " Cyril Hrubis 2019-03-18 16:08 ` Qian Cai 2019-03-18 16:08 ` [LTP] " Qian Cai 2019-03-19 12:59 ` Cyril Hrubis 2019-03-19 12:59 ` [LTP] " Cyril Hrubis 2019-03-18 18:12 ` Yang Shi 2019-03-18 18:12 ` [LTP] " Yang Shi 2019-03-19 13:27 ` Oscar Salvador 2019-03-19 13:27 ` [LTP] " Oscar Salvador 2019-03-19 14:26 ` Kirill A. Shutemov 2019-03-19 14:26 ` [LTP] " Kirill A. Shutemov 2019-03-19 14:30 ` Cyril Hrubis 2019-03-19 14:30 ` [LTP] " Cyril Hrubis 2019-03-19 14:41 ` Oscar Salvador 2019-03-19 14:41 ` [LTP] " Oscar Salvador 2019-03-19 14:52 ` Kirill A. Shutemov 2019-03-19 14:52 ` [LTP] " Kirill A. Shutemov 2019-03-19 15:10 ` Oscar Salvador 2019-03-19 15:10 ` [LTP] " Oscar Salvador 2019-03-19 16:29 ` Yang Shi 2019-03-19 16:29 ` [LTP] " Yang Shi 2019-03-19 16:25 ` Yang Shi 2019-03-19 16:25 ` [LTP] " Yang Shi
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.