[PATCH v4.9.y] Fixes: Commit 3c226c637b69 ("mm: numa: avoid waiting on freed migrated pages")

[PATCH v4.9.y] Fixes: Commit 3c226c637b69 ("mm: numa: avoid waiting on freed migrated pages")

[Chas Williams]

From: Chas Williams <chas3@att.com>

Commit 3c226c637b69 ("mm: numa: avoid waiting on freed migrated pages")
was an incomplete backport of the upstream commit.  It is necessary to
always reset page_nid before attempting any early exit.
---
 mm/huge_memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 9efe88ef9702..e4c6c3edaf6a 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1259,12 +1259,12 @@ int do_huge_pmd_numa_page(struct fault_env *fe, pmd_t pmd)
 
 	/* Migration could have started since the pmd_trans_migrating check */
 	if (!page_locked) {
+		page_nid = -1;
 		if (!get_page_unless_zero(page))
 			goto out_unlock;
 		spin_unlock(fe->ptl);
 		wait_on_page_locked(page);
 		put_page(page);
-		page_nid = -1;
 		goto out;
 	}
 
-- 
2.14.4

Re: [PATCH v4.9.y] Fixes: Commit 3c226c637b69 ("mm: numa: avoid waiting on freed migrated pages")

[Greg KH]

On Wed, Sep 05, 2018 at 04:58:52AM -0400, Chas Williams wrote:
> From: Chas Williams <chas3@att.com>
> 
> Commit 3c226c637b69 ("mm: numa: avoid waiting on freed migrated pages")
> was an incomplete backport of the upstream commit.  It is necessary to
> always reset page_nid before attempting any early exit.
> ---
>  mm/huge_memory.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: [PATCH v4.9.y] Fixes: Commit 3c226c637b69 ("mm: numa: avoid waiting on freed migrated pages")

[Nathan Chancellor]

On Wed, Sep 05, 2018 at 11:05:15AM +0200, Greg KH wrote:
> On Wed, Sep 05, 2018 at 04:58:52AM -0400, Chas Williams wrote:
> > From: Chas Williams <chas3@att.com>
> > 
> > Commit 3c226c637b69 ("mm: numa: avoid waiting on freed migrated pages")
> > was an incomplete backport of the upstream commit.  It is necessary to
> > always reset page_nid before attempting any early exit.
> > ---
> >  mm/huge_memory.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> 
> <formletter>
> 
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
>     https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
> 
> </formletter>

This is an issue with the 4.9 tree, not mainline. The hash is
incorrect but the problem appears valid.

Compare:

2aa6d036b716 ("mm: numa: avoid waiting on freed migrated pages")

from v4.9.36 with the upstream commit from 4.12:

3c226c637b69 ("mm: numa: avoid waiting on freed migrated pages")

It is missing this diff.

The original commit conflicted due to lack of commit 82b0f8c39a38 ("mm:
join struct fault_env and vm_fault") in 4.9 so it wasn't a clean
application, must have just gotten lost in the noise.

Cheers,
Nathan

Re: [PATCH v4.9.y] Fixes: Commit 3c226c637b69 ("mm: numa: avoid waiting on freed migrated pages")

[Greg Kroah-Hartman]

On Wed, Sep 05, 2018 at 05:23:28AM -0400, Chas Williams wrote:
> On Wed, Sep 5, 2018 at 5:18 AM Nathan Chancellor <natechancellor@gmail.com>
> wrote:
> 
> > On Wed, Sep 05, 2018 at 11:05:15AM +0200, Greg KH wrote:
> > > On Wed, Sep 05, 2018 at 04:58:52AM -0400, Chas Williams wrote:
> > > > From: Chas Williams <chas3@att.com>
> > > >
> > > > Commit 3c226c637b69 ("mm: numa: avoid waiting on freed migrated pages")
> > > > was an incomplete backport of the upstream commit.  It is necessary to
> > > > always reset page_nid before attempting any early exit.
> > > > ---
> > > >  mm/huge_memory.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > >
> > > <formletter>
> > >
> > > This is not the correct way to submit patches for inclusion in the
> > > stable kernel tree.  Please read:
> > >
> > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > > for how to do this properly.
> > >
> > > </formletter>
> >
> > This is an issue with the 4.9 tree, not mainline. The hash is
> > incorrect but the problem appears valid.
> >
> 
> Unfortunately, this issue doesn't fit into option 1, 2 or 3.  I could
> only do option 4.

Ok, but if so, you need to at least sign-off on your patch, right?  :)

Also, call it out explicitly what you are trying to do here, and why the
patch is not upstream, so I know what is going on.

thanks,

greg k-h

Re: [PATCH v4.9.y] Fixes: Commit 3c226c637b69 ("mm: numa: avoid waiting on freed migrated pages")

[Chas Williams]

On Wed, Sep 5, 2018 at 5:33 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Sep 05, 2018 at 05:23:28AM -0400, Chas Williams wrote:
> > On Wed, Sep 5, 2018 at 5:18 AM Nathan Chancellor <natechancellor@gmail.com>
> > wrote:
> >
> > > On Wed, Sep 05, 2018 at 11:05:15AM +0200, Greg KH wrote:
> > > > On Wed, Sep 05, 2018 at 04:58:52AM -0400, Chas Williams wrote:
> > > > > From: Chas Williams <chas3@att.com>
> > > > >
> > > > > Commit 3c226c637b69 ("mm: numa: avoid waiting on freed migrated pages")
> > > > > was an incomplete backport of the upstream commit.  It is necessary to
> > > > > always reset page_nid before attempting any early exit.
> > > > > ---
> > > > >  mm/huge_memory.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > >
> > > > <formletter>
> > > >
> > > > This is not the correct way to submit patches for inclusion in the
> > > > stable kernel tree.  Please read:
> > > >
> > > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > > > for how to do this properly.
> > > >
> > > > </formletter>
> > >
> > > This is an issue with the 4.9 tree, not mainline. The hash is
> > > incorrect but the problem appears valid.
> > >
> >
> > Unfortunately, this issue doesn't fit into option 1, 2 or 3.  I could
> > only do option 4.
>
> Ok, but if so, you need to at least sign-off on your patch, right?  :)

I can.  I wasn't clear to me since I am just fixing what the
original author wrote.

>
>
> Also, call it out explicitly what you are trying to do here, and why the
> patch is not upstream, so I know what is going on.


How would you like this done?  I thought the body of the
commit message was fairly clear (with the exception of the
hash).


>
>
> thanks,
>
> greg k-h

Re: [PATCH v4.9.y] Fixes: Commit 3c226c637b69 ("mm: numa: avoid waiting on freed migrated pages")

[Greg Kroah-Hartman]

On Wed, Sep 05, 2018 at 05:41:40AM -0400, Chas Williams wrote:
> On Wed, Sep 5, 2018 at 5:33 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Sep 05, 2018 at 05:23:28AM -0400, Chas Williams wrote:
> > > On Wed, Sep 5, 2018 at 5:18 AM Nathan Chancellor <natechancellor@gmail.com>
> > > wrote:
> > >
> > > > On Wed, Sep 05, 2018 at 11:05:15AM +0200, Greg KH wrote:
> > > > > On Wed, Sep 05, 2018 at 04:58:52AM -0400, Chas Williams wrote:
> > > > > > From: Chas Williams <chas3@att.com>
> > > > > >
> > > > > > Commit 3c226c637b69 ("mm: numa: avoid waiting on freed migrated pages")
> > > > > > was an incomplete backport of the upstream commit.  It is necessary to
> > > > > > always reset page_nid before attempting any early exit.
> > > > > > ---
> > > > > >  mm/huge_memory.c | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > >
> > > > > <formletter>
> > > > >
> > > > > This is not the correct way to submit patches for inclusion in the
> > > > > stable kernel tree.  Please read:
> > > > >
> > > > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > > > > for how to do this properly.
> > > > >
> > > > > </formletter>
> > > >
> > > > This is an issue with the 4.9 tree, not mainline. The hash is
> > > > incorrect but the problem appears valid.
> > > >
> > >
> > > Unfortunately, this issue doesn't fit into option 1, 2 or 3.  I could
> > > only do option 4.
> >
> > Ok, but if so, you need to at least sign-off on your patch, right?  :)
> 
> I can.  I wasn't clear to me since I am just fixing what the
> original author wrote.

We can not take a patch without a signed-off-by, that's for any kernel
patch, nothing special about stable trees here.

> > Also, call it out explicitly what you are trying to do here, and why the
> > patch is not upstream, so I know what is going on.
> 
> How would you like this done?  I thought the body of the
> commit message was fairly clear (with the exception of the
> hash).

Summarize the other info in this thread there as well.

thanks,

greg k-h

[PATCH] Fixes: Commit 2aa6d036b716 ("mm: numa: avoid waiting on freed migrated pages")

[Chas Williams]

From: Chas Williams <chas3@att.com>

Commit 2aa6d036b716 ("mm: numa: avoid waiting on freed migrated pages")
was an incomplete backport of the upstream commit.  It is necessary to
always reset page_nid before attempting any early exit.

The original commit conflicted due to lack of commit 82b0f8c39a38
("mm: join struct fault_env and vm_fault") in 4.9 so it wasn't a clean
application, and the change must have just gotten lost in the noise.

Signed-off-by: Chas Williams <chas3@att.com>
---
 mm/huge_memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 9efe88ef9702..e4c6c3edaf6a 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1259,12 +1259,12 @@ int do_huge_pmd_numa_page(struct fault_env *fe, pmd_t pmd)
 
 	/* Migration could have started since the pmd_trans_migrating check */
 	if (!page_locked) {
+		page_nid = -1;
 		if (!get_page_unless_zero(page))
 			goto out_unlock;
 		spin_unlock(fe->ptl);
 		wait_on_page_locked(page);
 		put_page(page);
-		page_nid = -1;
 		goto out;
 	}
 
-- 
2.14.4

Re: [PATCH] Fixes: Commit 2aa6d036b716 ("mm: numa: avoid waiting on freed migrated pages")

[Greg KH]

On Wed, Sep 05, 2018 at 10:43:17AM -0400, Chas Williams wrote:
> From: Chas Williams <chas3@att.com>
> 
> Commit 2aa6d036b716 ("mm: numa: avoid waiting on freed migrated pages")
> was an incomplete backport of the upstream commit.  It is necessary to
> always reset page_nid before attempting any early exit.
> 
> The original commit conflicted due to lack of commit 82b0f8c39a38
> ("mm: join struct fault_env and vm_fault") in 4.9 so it wasn't a clean
> application, and the change must have just gotten lost in the noise.
> 
> Signed-off-by: Chas Williams <chas3@att.com>
> ---
>  mm/huge_memory.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Much better, thanks!

But can you cc: the people involved in that first patch to get their
ack?

Also, what about 4.4.y and 3.18.y which had the original commit
backported as well?  Do they also need this fix?

thanks,

greg k-h

Re: [PATCH] Fixes: Commit 2aa6d036b716 ("mm: numa: avoid waiting on freed migrated pages")

[Chas Williams]

On Wed, Sep 5, 2018 at 11:03 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Sep 05, 2018 at 10:43:17AM -0400, Chas Williams wrote:
> > From: Chas Williams <chas3@att.com>
> >
> > Commit 2aa6d036b716 ("mm: numa: avoid waiting on freed migrated pages")
> > was an incomplete backport of the upstream commit.  It is necessary to
> > always reset page_nid before attempting any early exit.
> >
> > The original commit conflicted due to lack of commit 82b0f8c39a38
> > ("mm: join struct fault_env and vm_fault") in 4.9 so it wasn't a clean
> > application, and the change must have just gotten lost in the noise.
> >
> > Signed-off-by: Chas Williams <chas3@att.com>
> > ---
> >  mm/huge_memory.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Much better, thanks!
>
> But can you cc: the people involved in that first patch to get their
> ack?
>
> Also, what about 4.4.y and 3.18.y which had the original commit
> backported as well?  Do they also need this fix?

Yes, both are broken.

> thanks,
>
> greg k-h

[PATCH][4.9.y] Fixes: Commit 2aa6d036b716 ("mm: numa: avoid waiting on freed migrated pages")

[Chas Williams]

From: Chas Williams <chas3@att.com>

Commit 2aa6d036b716 ("mm: numa: avoid waiting on freed migrated pages")
was an incomplete backport of the upstream commit.  It is necessary to
always reset page_nid before attempting any early exit.

The original commit conflicted due to lack of commit 82b0f8c39a38
("mm: join struct fault_env and vm_fault") in 4.9 so it wasn't a clean
application, and the change must have just gotten lost in the noise.

Signed-off-by: Chas Williams <chas3@att.com>
---
 mm/huge_memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 9efe88ef9702..e4c6c3edaf6a 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1259,12 +1259,12 @@ int do_huge_pmd_numa_page(struct fault_env *fe, pmd_t pmd)
 
 	/* Migration could have started since the pmd_trans_migrating check */
 	if (!page_locked) {
+		page_nid = -1;
 		if (!get_page_unless_zero(page))
 			goto out_unlock;
 		spin_unlock(fe->ptl);
 		wait_on_page_locked(page);
 		put_page(page);
-		page_nid = -1;
 		goto out;
 	}
 
-- 
2.14.4

[PATCH][v3.18.y] Fixes: Commit 86af955d02bb ("mm: numa: avoid waiting on freed migrated pages")

[Chas Williams]

From: Chas Williams <chas3@att.com>

Commit 86af955d02bb ("mm: numa: avoid waiting on freed migrated pages")
was an incomplete backport of the upstream commit.  It is necessary to
always reset page_nid before attempting any early exit.

The original commit conflicted due to lack of commit 82b0f8c39a38
("mm: join struct fault_env and vm_fault") in 4.9 so it wasn't a clean
application, and the change must have just gotten lost in the noise.

Signed-off-by: Chas Williams <chas3@att.com>
---
 mm/huge_memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 690d172436c4..bc402f39ac48 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1329,12 +1329,12 @@ int do_huge_pmd_numa_page(struct mm_struct *mm, struct vm_area_struct *vma,
 
 	/* Migration could have started since the pmd_trans_migrating check */
 	if (!page_locked) {
+		page_nid = -1;
 		if (!get_page_unless_zero(page))
 			goto out_unlock;
 		spin_unlock(ptl);
 		wait_on_page_locked(page);
 		put_page(page);
-		page_nid = -1;
 		goto out;
 	}
 
-- 
2.14.4

[PATCH][v4.4.y] Fixes: Commit cdbf92675fad ("mm: numa: avoid waiting on freed migrated pages")

[Chas Williams]

From: Chas Williams <chas3@att.com>

Commit cdbf92675fad ("mm: numa: avoid waiting on freed migrated pages")
was an incomplete backport of the upstream commit.  It is necessary to
always reset page_nid before attempting any early exit.

The original commit conflicted due to lack of commit 82b0f8c39a38
("mm: join struct fault_env and vm_fault") in 4.9 so it wasn't a clean
application, and the change must have just gotten lost in the noise.

Signed-off-by: Chas Williams <chas3@att.com>
---
 mm/huge_memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 0127b788272f..c4ea57ee2fd1 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1393,12 +1393,12 @@ int do_huge_pmd_numa_page(struct mm_struct *mm, struct vm_area_struct *vma,
 
 	/* Migration could have started since the pmd_trans_migrating check */
 	if (!page_locked) {
+		page_nid = -1;
 		if (!get_page_unless_zero(page))
 			goto out_unlock;
 		spin_unlock(ptl);
 		wait_on_page_locked(page);
 		put_page(page);
-		page_nid = -1;
 		goto out;
 	}
 
-- 
2.14.4