[PATCH 0/2] Two bug-fixes for HMM

[PATCH 0/2] Two bug-fixes for HMM

[Kuehling, Felix]

These problems were found in AMD-internal testing as we're working on
adopting HMM. They are rebased against glisse/hmm-5.2-v3. We'd like to get
them applied to a mainline Linux kernel as well as drm-next and
amd-staging-drm-next sooner rather than later.

Currently the HMM in amd-staging-drm-next is quite far behind hmm-5.2-v3,
but the driver changes for HMM are expected to land in 5.2 and will need to
be rebased on those HMM changes.

I'd like to work out a flow between Jerome, Dave, Alex and myself that
allows us to test the latest version of HMM on amd-staging-drm-next so
that ideally everything comes together in master without much need for
rebasing and retesting.

Maybe having Jerome's latest HMM changes in drm-next. However, that may
create dependencies where Jerome and Dave need to coordinate their pull-
requests for master.

Felix Kuehling (1):
  mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking

Philip Yang (1):
  mm/hmm: support automatic NUMA balancing

 mm/hmm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.17.1

[PATCH 1/2] mm/hmm: support automatic NUMA balancing

[Kuehling, Felix]

From: Philip Yang <Philip.Yang@amd.com>

While the page is migrating by NUMA balancing, HMM failed to detect this
condition and still return the old page. Application will use the new
page migrated, but driver pass the old page physical address to GPU,
this crash the application later.

Use pte_protnone(pte) to return this condition and then hmm_vma_do_fault
will allocate new page.

Signed-off-by: Philip Yang <Philip.Yang@amd.com>
---
 mm/hmm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/hmm.c b/mm/hmm.c
index 75d2ea906efb..b65c27d5c119 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -554,7 +554,7 @@ static int hmm_vma_handle_pmd(struct mm_walk *walk,
 
 static inline uint64_t pte_to_hmm_pfn_flags(struct hmm_range *range, pte_t pte)
 {
-	if (pte_none(pte) || !pte_present(pte))
+	if (pte_none(pte) || !pte_present(pte) || pte_protnone(pte))
 		return 0;
 	return pte_write(pte) ? range->flags[HMM_PFN_VALID] |
 				range->flags[HMM_PFN_WRITE] :
-- 
2.17.1

[PATCH 2/2] mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking

[Kuehling, Felix]

Don't set this flag by default in hmm_vma_do_fault. It is set
conditionally just a few lines below. Setting it unconditionally
can lead to handle_mm_fault doing a non-blocking fault, returning
-EBUSY and unlocking mmap_sem unexpectedly.

Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 mm/hmm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/hmm.c b/mm/hmm.c
index b65c27d5c119..3c4f1d62202f 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -339,7 +339,7 @@ struct hmm_vma_walk {
 static int hmm_vma_do_fault(struct mm_walk *walk, unsigned long addr,
 			    bool write_fault, uint64_t *pfn)
 {
-	unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_REMOTE;
+	unsigned int flags = FAULT_FLAG_REMOTE;
 	struct hmm_vma_walk *hmm_vma_walk = walk->private;
 	struct hmm_range *range = hmm_vma_walk->range;
 	struct vm_area_struct *vma = walk->vma;
-- 
2.17.1

Re: [PATCH 1/2] mm/hmm: support automatic NUMA balancing

[Jerome Glisse]

On Fri, May 10, 2019 at 07:53:23PM +0000, Kuehling, Felix wrote:
> From: Philip Yang <Philip.Yang@amd.com>
> 
> While the page is migrating by NUMA balancing, HMM failed to detect this
> condition and still return the old page. Application will use the new
> page migrated, but driver pass the old page physical address to GPU,
> this crash the application later.
> 
> Use pte_protnone(pte) to return this condition and then hmm_vma_do_fault
> will allocate new page.
> 
> Signed-off-by: Philip Yang <Philip.Yang@amd.com>

Reviewed-by: Jérôme Glisse <jglisse@redhat.com>

> ---
>  mm/hmm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 75d2ea906efb..b65c27d5c119 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -554,7 +554,7 @@ static int hmm_vma_handle_pmd(struct mm_walk *walk,
>  
>  static inline uint64_t pte_to_hmm_pfn_flags(struct hmm_range *range, pte_t pte)
>  {
> -	if (pte_none(pte) || !pte_present(pte))
> +	if (pte_none(pte) || !pte_present(pte) || pte_protnone(pte))
>  		return 0;
>  	return pte_write(pte) ? range->flags[HMM_PFN_VALID] |
>  				range->flags[HMM_PFN_WRITE] :
> -- 
> 2.17.1
> 

Re: [PATCH 2/2] mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking

[Jerome Glisse]

On Fri, May 10, 2019 at 07:53:24PM +0000, Kuehling, Felix wrote:
> Don't set this flag by default in hmm_vma_do_fault. It is set
> conditionally just a few lines below. Setting it unconditionally
> can lead to handle_mm_fault doing a non-blocking fault, returning
> -EBUSY and unlocking mmap_sem unexpectedly.
> 
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>

Reviewed-by: Jérôme Glisse <jglisse@redhat.com>

> ---
>  mm/hmm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/hmm.c b/mm/hmm.c
> index b65c27d5c119..3c4f1d62202f 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -339,7 +339,7 @@ struct hmm_vma_walk {
>  static int hmm_vma_do_fault(struct mm_walk *walk, unsigned long addr,
>  			    bool write_fault, uint64_t *pfn)
>  {
> -	unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_REMOTE;
> +	unsigned int flags = FAULT_FLAG_REMOTE;
>  	struct hmm_vma_walk *hmm_vma_walk = walk->private;
>  	struct hmm_range *range = hmm_vma_walk->range;
>  	struct vm_area_struct *vma = walk->vma;
> -- 
> 2.17.1
> 

Re: [PATCH 2/2] mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking

[Kuehling, Felix]

Hi Jerome,

Do you want me to push the patches to your branch? Or are you going to 
apply them yourself?

Is your hmm-5.2-v3 branch going to make it into Linux 5.2? If so, do you 
know when? I'd like to coordinate with Dave Airlie so that we can also 
get that update into a drm-next branch soon.

I see that Linus merged Dave's pull request for Linux 5.2, which 
includes the first changes in amdgpu using HMM. They're currently broken 
without these two patches.

Thanks,
   Felix

On 2019-05-10 4:14 p.m., Jerome Glisse wrote:
> [CAUTION: External Email]
>
> On Fri, May 10, 2019 at 07:53:24PM +0000, Kuehling, Felix wrote:
>> Don't set this flag by default in hmm_vma_do_fault. It is set
>> conditionally just a few lines below. Setting it unconditionally
>> can lead to handle_mm_fault doing a non-blocking fault, returning
>> -EBUSY and unlocking mmap_sem unexpectedly.
>>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Reviewed-by: Jérôme Glisse <jglisse@redhat.com>
>
>> ---
>>   mm/hmm.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/hmm.c b/mm/hmm.c
>> index b65c27d5c119..3c4f1d62202f 100644
>> --- a/mm/hmm.c
>> +++ b/mm/hmm.c
>> @@ -339,7 +339,7 @@ struct hmm_vma_walk {
>>   static int hmm_vma_do_fault(struct mm_walk *walk, unsigned long addr,
>>                            bool write_fault, uint64_t *pfn)
>>   {
>> -     unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_REMOTE;
>> +     unsigned int flags = FAULT_FLAG_REMOTE;
>>        struct hmm_vma_walk *hmm_vma_walk = walk->private;
>>        struct hmm_range *range = hmm_vma_walk->range;
>>        struct vm_area_struct *vma = walk->vma;
>> --
>> 2.17.1
>>

Re: [PATCH 2/2] mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking

[Jerome Glisse]

Andrew can we get this 2 fixes line up for 5.2 ?

On Mon, May 13, 2019 at 07:36:44PM +0000, Kuehling, Felix wrote:
> Hi Jerome,
> 
> Do you want me to push the patches to your branch? Or are you going to 
> apply them yourself?
> 
> Is your hmm-5.2-v3 branch going to make it into Linux 5.2? If so, do you 
> know when? I'd like to coordinate with Dave Airlie so that we can also 
> get that update into a drm-next branch soon.
> 
> I see that Linus merged Dave's pull request for Linux 5.2, which 
> includes the first changes in amdgpu using HMM. They're currently broken 
> without these two patches.

HMM patch do not go through any git branch they go through the mmotm
collection. So it is not something you can easily coordinate with drm
branch.

By broken i expect you mean that if numabalance happens it breaks ?
Or it might sleep when you are not expecting it too ?

Cheers,
Jérôme

> 
> Thanks,
>    Felix
> 
> On 2019-05-10 4:14 p.m., Jerome Glisse wrote:
> > [CAUTION: External Email]
> >
> > On Fri, May 10, 2019 at 07:53:24PM +0000, Kuehling, Felix wrote:
> >> Don't set this flag by default in hmm_vma_do_fault. It is set
> >> conditionally just a few lines below. Setting it unconditionally
> >> can lead to handle_mm_fault doing a non-blocking fault, returning
> >> -EBUSY and unlocking mmap_sem unexpectedly.
> >>
> >> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> > Reviewed-by: Jérôme Glisse <jglisse@redhat.com>
> >
> >> ---
> >>   mm/hmm.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/mm/hmm.c b/mm/hmm.c
> >> index b65c27d5c119..3c4f1d62202f 100644
> >> --- a/mm/hmm.c
> >> +++ b/mm/hmm.c
> >> @@ -339,7 +339,7 @@ struct hmm_vma_walk {
> >>   static int hmm_vma_do_fault(struct mm_walk *walk, unsigned long addr,
> >>                            bool write_fault, uint64_t *pfn)
> >>   {
> >> -     unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_REMOTE;
> >> +     unsigned int flags = FAULT_FLAG_REMOTE;
> >>        struct hmm_vma_walk *hmm_vma_walk = walk->private;
> >>        struct hmm_range *range = hmm_vma_walk->range;
> >>        struct vm_area_struct *vma = walk->vma;
> >> --
> >> 2.17.1
> >>

Re: [PATCH 2/2] mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking

[Deucher, Alexander]

[-- Attachment #1: Type: text/plain, Size: 2597 bytes --]

I reverted all the amdgpu HMM patches for 5.2 because they also depended on this patch:
https://cgit.freedesktop.org/~agd5f/linux/commit/?h=drm-next-5.2-wip&id=ce05ef71564f7cbe270cd4337c36ee720ea534db
which did not have a clear line of sight for 5.2 either.

Alex
________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Kuehling, Felix <Felix.Kuehling@amd.com>
Sent: Monday, May 13, 2019 3:36 PM
To: Jerome Glisse
Cc: linux-mm@kvack.org; airlied@gmail.com; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; alex.deucher@amd.com
Subject: Re: [PATCH 2/2] mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking

[CAUTION: External Email]

Hi Jerome,

Do you want me to push the patches to your branch? Or are you going to
apply them yourself?

Is your hmm-5.2-v3 branch going to make it into Linux 5.2? If so, do you
know when? I'd like to coordinate with Dave Airlie so that we can also
get that update into a drm-next branch soon.

I see that Linus merged Dave's pull request for Linux 5.2, which
includes the first changes in amdgpu using HMM. They're currently broken
without these two patches.

Thanks,
   Felix

On 2019-05-10 4:14 p.m., Jerome Glisse wrote:
> [CAUTION: External Email]
>
> On Fri, May 10, 2019 at 07:53:24PM +0000, Kuehling, Felix wrote:
>> Don't set this flag by default in hmm_vma_do_fault. It is set
>> conditionally just a few lines below. Setting it unconditionally
>> can lead to handle_mm_fault doing a non-blocking fault, returning
>> -EBUSY and unlocking mmap_sem unexpectedly.
>>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Reviewed-by: Jérôme Glisse <jglisse@redhat.com>
>
>> ---
>>   mm/hmm.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/hmm.c b/mm/hmm.c
>> index b65c27d5c119..3c4f1d62202f 100644
>> --- a/mm/hmm.c
>> +++ b/mm/hmm.c
>> @@ -339,7 +339,7 @@ struct hmm_vma_walk {
>>   static int hmm_vma_do_fault(struct mm_walk *walk, unsigned long addr,
>>                            bool write_fault, uint64_t *pfn)
>>   {
>> -     unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_REMOTE;
>> +     unsigned int flags = FAULT_FLAG_REMOTE;
>>        struct hmm_vma_walk *hmm_vma_walk = walk->private;
>>        struct hmm_range *range = hmm_vma_walk->range;
>>        struct vm_area_struct *vma = walk->vma;
>> --
>> 2.17.1
>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #2: Type: text/html, Size: 4763 bytes --]

Re: [PATCH 2/2] mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking

[Kuehling, Felix]

[Fixed Alex's email address, sorry for getting it wrong first]

On 2019-05-13 3:49 p.m., Jerome Glisse wrote:
> [CAUTION: External Email]
>
> Andrew can we get this 2 fixes line up for 5.2 ?
>
> On Mon, May 13, 2019 at 07:36:44PM +0000, Kuehling, Felix wrote:
>> Hi Jerome,
>>
>> Do you want me to push the patches to your branch? Or are you going to
>> apply them yourself?
>>
>> Is your hmm-5.2-v3 branch going to make it into Linux 5.2? If so, do you
>> know when? I'd like to coordinate with Dave Airlie so that we can also
>> get that update into a drm-next branch soon.
>>
>> I see that Linus merged Dave's pull request for Linux 5.2, which
>> includes the first changes in amdgpu using HMM. They're currently broken
>> without these two patches.
> HMM patch do not go through any git branch they go through the mmotm
> collection. So it is not something you can easily coordinate with drm
> branch.
>
> By broken i expect you mean that if numabalance happens it breaks ?
> Or it might sleep when you are not expecting it too ?

Without the NUMA fix we'd end up using an outdated physical address in 
the GPU page table. The problem was caught by a test that got incorrect 
computation results using OpenCL on a NUMA system.

Without the FAULT_FLAG_ALLOW_RETRY patch, there can be kernel oopses due 
to incorrect locking/unlocking of mmap_sem. It breaks the promise that 
hmm_range_fault should not unlock the mmap_sem if block==true. It takes 
some memory pressure to trigger this.

Regards,
   Felix


>
> Cheers,
> Jérôme
>
>> Thanks,
>>     Felix
>>
>> On 2019-05-10 4:14 p.m., Jerome Glisse wrote:
>>> [CAUTION: External Email]
>>>
>>> On Fri, May 10, 2019 at 07:53:24PM +0000, Kuehling, Felix wrote:
>>>> Don't set this flag by default in hmm_vma_do_fault. It is set
>>>> conditionally just a few lines below. Setting it unconditionally
>>>> can lead to handle_mm_fault doing a non-blocking fault, returning
>>>> -EBUSY and unlocking mmap_sem unexpectedly.
>>>>
>>>> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
>>> Reviewed-by: Jérôme Glisse <jglisse@redhat.com>
>>>
>>>> ---
>>>>    mm/hmm.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/mm/hmm.c b/mm/hmm.c
>>>> index b65c27d5c119..3c4f1d62202f 100644
>>>> --- a/mm/hmm.c
>>>> +++ b/mm/hmm.c
>>>> @@ -339,7 +339,7 @@ struct hmm_vma_walk {
>>>>    static int hmm_vma_do_fault(struct mm_walk *walk, unsigned long addr,
>>>>                             bool write_fault, uint64_t *pfn)
>>>>    {
>>>> -     unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_REMOTE;
>>>> +     unsigned int flags = FAULT_FLAG_REMOTE;
>>>>         struct hmm_vma_walk *hmm_vma_walk = walk->private;
>>>>         struct hmm_range *range = hmm_vma_walk->range;
>>>>         struct vm_area_struct *vma = walk->vma;
>>>> --
>>>> 2.17.1
>>>>

Re: [PATCH 1/2] mm/hmm: support automatic NUMA balancing

[Andrew Morton]

On Fri, 10 May 2019 19:53:23 +0000 "Kuehling, Felix" <Felix.Kuehling@amd.com> wrote:

> From: Philip Yang <Philip.Yang@amd.com>
> 
> While the page is migrating by NUMA balancing, HMM failed to detect this
> condition and still return the old page. Application will use the new
> page migrated, but driver pass the old page physical address to GPU,
> this crash the application later.
> 
> Use pte_protnone(pte) to return this condition and then hmm_vma_do_fault
> will allocate new page.
> 
> Signed-off-by: Philip Yang <Philip.Yang@amd.com>

This should have included your signed-off-by:, since you were on the
patch delivery path.  I'll make that change to my copy of the patch,
OK?

Re: [PATCH 1/2] mm/hmm: support automatic NUMA balancing

[Jerome Glisse]

On Mon, May 13, 2019 at 02:27:20PM -0700, Andrew Morton wrote:
> On Fri, 10 May 2019 19:53:23 +0000 "Kuehling, Felix" <Felix.Kuehling@amd.com> wrote:
> 
> > From: Philip Yang <Philip.Yang@amd.com>
> > 
> > While the page is migrating by NUMA balancing, HMM failed to detect this
> > condition and still return the old page. Application will use the new
> > page migrated, but driver pass the old page physical address to GPU,
> > this crash the application later.
> > 
> > Use pte_protnone(pte) to return this condition and then hmm_vma_do_fault
> > will allocate new page.
> > 
> > Signed-off-by: Philip Yang <Philip.Yang@amd.com>
> 
> This should have included your signed-off-by:, since you were on the
> patch delivery path.  I'll make that change to my copy of the patch,
> OK?

Yes it should have included that.

Re: [PATCH 2/2] mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking

[Kuehling, Felix]

On 2019-05-13 4:21 p.m., Deucher, Alexander wrote:
> [CAUTION: External Email]
> I reverted all the amdgpu HMM patches for 5.2 because they also 
> depended on this patch:
> https://cgit.freedesktop.org/~agd5f/linux/commit/?h=drm-next-5.2-wip&id=ce05ef71564f7cbe270cd4337c36ee720ea534db
> which did not have a clear line of sight for 5.2 either.

When was that? I saw "Use HMM for userptr" in Dave's 5.2-rc1 pull 
request to Linus.


Regards,
   Felix


>
> Alex
> ------------------------------------------------------------------------
> *From:* amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of 
> Kuehling, Felix <Felix.Kuehling@amd.com>
> *Sent:* Monday, May 13, 2019 3:36 PM
> *To:* Jerome Glisse
> *Cc:* linux-mm@kvack.org; airlied@gmail.com; 
> amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; 
> alex.deucher@amd.com
> *Subject:* Re: [PATCH 2/2] mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for 
> non-blocking
> [CAUTION: External Email]
>
> Hi Jerome,
>
> Do you want me to push the patches to your branch? Or are you going to
> apply them yourself?
>
> Is your hmm-5.2-v3 branch going to make it into Linux 5.2? If so, do you
> know when? I'd like to coordinate with Dave Airlie so that we can also
> get that update into a drm-next branch soon.
>
> I see that Linus merged Dave's pull request for Linux 5.2, which
> includes the first changes in amdgpu using HMM. They're currently broken
> without these two patches.
>
> Thanks,
>    Felix
>
> On 2019-05-10 4:14 p.m., Jerome Glisse wrote:
> > [CAUTION: External Email]
> >
> > On Fri, May 10, 2019 at 07:53:24PM +0000, Kuehling, Felix wrote:
> >> Don't set this flag by default in hmm_vma_do_fault. It is set
> >> conditionally just a few lines below. Setting it unconditionally
> >> can lead to handle_mm_fault doing a non-blocking fault, returning
> >> -EBUSY and unlocking mmap_sem unexpectedly.
> >>
> >> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> > Reviewed-by: Jérôme Glisse <jglisse@redhat.com>
> >
> >> ---
> >>   mm/hmm.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/mm/hmm.c b/mm/hmm.c
> >> index b65c27d5c119..3c4f1d62202f 100644
> >> --- a/mm/hmm.c
> >> +++ b/mm/hmm.c
> >> @@ -339,7 +339,7 @@ struct hmm_vma_walk {
> >>   static int hmm_vma_do_fault(struct mm_walk *walk, unsigned long addr,
> >>                            bool write_fault, uint64_t *pfn)
> >>   {
> >> -     unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_REMOTE;
> >> +     unsigned int flags = FAULT_FLAG_REMOTE;
> >>        struct hmm_vma_walk *hmm_vma_walk = walk->private;
> >>        struct hmm_range *range = hmm_vma_walk->range;
> >>        struct vm_area_struct *vma = walk->vma;
> >> --
> >> 2.17.1
> >>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] mm/hmm: support automatic NUMA balancing

[Kuehling, Felix]

On 2019-05-13 5:27 p.m., Andrew Morton wrote:
> [CAUTION: External Email]
>
> On Fri, 10 May 2019 19:53:23 +0000 "Kuehling, Felix" <Felix.Kuehling@amd.com> wrote:
>
>> From: Philip Yang <Philip.Yang@amd.com>
>>
>> While the page is migrating by NUMA balancing, HMM failed to detect this
>> condition and still return the old page. Application will use the new
>> page migrated, but driver pass the old page physical address to GPU,
>> this crash the application later.
>>
>> Use pte_protnone(pte) to return this condition and then hmm_vma_do_fault
>> will allocate new page.
>>
>> Signed-off-by: Philip Yang <Philip.Yang@amd.com>
> This should have included your signed-off-by:, since you were on the
> patch delivery path.  I'll make that change to my copy of the patch,
> OK?
>
Yes. Thanks!

Re: [PATCH 2/2] mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking

[Alex Deucher]

On Tue, May 14, 2019 at 5:12 PM Kuehling, Felix <Felix.Kuehling@amd.com> wrote:
>
>
> On 2019-05-13 4:21 p.m., Deucher, Alexander wrote:
> > [CAUTION: External Email]
> > I reverted all the amdgpu HMM patches for 5.2 because they also
> > depended on this patch:
> > https://cgit.freedesktop.org/~agd5f/linux/commit/?h=drm-next-5.2-wip&id=ce05ef71564f7cbe270cd4337c36ee720ea534db
> > which did not have a clear line of sight for 5.2 either.
>
> When was that? I saw "Use HMM for userptr" in Dave's 5.2-rc1 pull
> request to Linus.

https://patchwork.kernel.org/patch/10875587/

Alex



>
>
> Regards,
>    Felix
>
>
> >
> > Alex
> > ------------------------------------------------------------------------
> > *From:* amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of
> > Kuehling, Felix <Felix.Kuehling@amd.com>
> > *Sent:* Monday, May 13, 2019 3:36 PM
> > *To:* Jerome Glisse
> > *Cc:* linux-mm@kvack.org; airlied@gmail.com;
> > amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> > alex.deucher@amd.com
> > *Subject:* Re: [PATCH 2/2] mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for
> > non-blocking
> > [CAUTION: External Email]
> >
> > Hi Jerome,
> >
> > Do you want me to push the patches to your branch? Or are you going to
> > apply them yourself?
> >
> > Is your hmm-5.2-v3 branch going to make it into Linux 5.2? If so, do you
> > know when? I'd like to coordinate with Dave Airlie so that we can also
> > get that update into a drm-next branch soon.
> >
> > I see that Linus merged Dave's pull request for Linux 5.2, which
> > includes the first changes in amdgpu using HMM. They're currently broken
> > without these two patches.
> >
> > Thanks,
> >    Felix
> >
> > On 2019-05-10 4:14 p.m., Jerome Glisse wrote:
> > > [CAUTION: External Email]
> > >
> > > On Fri, May 10, 2019 at 07:53:24PM +0000, Kuehling, Felix wrote:
> > >> Don't set this flag by default in hmm_vma_do_fault. It is set
> > >> conditionally just a few lines below. Setting it unconditionally
> > >> can lead to handle_mm_fault doing a non-blocking fault, returning
> > >> -EBUSY and unlocking mmap_sem unexpectedly.
> > >>
> > >> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> > > Reviewed-by: Jérôme Glisse <jglisse@redhat.com>
> > >
> > >> ---
> > >>   mm/hmm.c | 2 +-
> > >>   1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/mm/hmm.c b/mm/hmm.c
> > >> index b65c27d5c119..3c4f1d62202f 100644
> > >> --- a/mm/hmm.c
> > >> +++ b/mm/hmm.c
> > >> @@ -339,7 +339,7 @@ struct hmm_vma_walk {
> > >>   static int hmm_vma_do_fault(struct mm_walk *walk, unsigned long addr,
> > >>                            bool write_fault, uint64_t *pfn)
> > >>   {
> > >> -     unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_REMOTE;
> > >> +     unsigned int flags = FAULT_FLAG_REMOTE;
> > >>        struct hmm_vma_walk *hmm_vma_walk = walk->private;
> > >>        struct hmm_range *range = hmm_vma_walk->range;
> > >>        struct vm_area_struct *vma = walk->vma;
> > >> --
> > >> 2.17.1
> > >>
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 0/2] Two bug-fixes for HMM

[Jason Gunthorpe]

On Fri, May 10, 2019 at 07:53:21PM +0000, Kuehling, Felix wrote:
> These problems were found in AMD-internal testing as we're working on
> adopting HMM. They are rebased against glisse/hmm-5.2-v3. We'd like to get
> them applied to a mainline Linux kernel as well as drm-next and
> amd-staging-drm-next sooner rather than later.
> 
> Currently the HMM in amd-staging-drm-next is quite far behind hmm-5.2-v3,
> but the driver changes for HMM are expected to land in 5.2 and will need to
> be rebased on those HMM changes.
>
> I'd like to work out a flow between Jerome, Dave, Alex and myself that
> allows us to test the latest version of HMM on amd-staging-drm-next so
> that ideally everything comes together in master without much need for
> rebasing and retesting.

I think we have that now, I'm running a hmm.git that is clean and can
be used for merging into DRM related trees (and RDMA). I've commited
to send this tree to Linus at the start of the merge window.

See here:

 https://lore.kernel.org/lkml/20190524124455.GB16845@ziepe.ca/

The tree is here:

 https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=hmm

However please consult with me before making a merge commit to be
co-ordinated. Thanks

I see in this thread that AMDGPU missed 5.2 beacause of the
co-ordination problems this tree is intended to solve, so I'm very
hopeful this will help your work move into 5.3!

> Maybe having Jerome's latest HMM changes in drm-next. However, that may
> create dependencies where Jerome and Dave need to coordinate their pull-
> requests for master.
> 
> Felix Kuehling (1):
>   mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking
> 
> Philip Yang (1):
>   mm/hmm: support automatic NUMA balancing

I've applied both of these patches with Jerome's Reviewed-by to
hmm.git and added the missed Signed-off-by

If you test and confirm I think this branch would be ready for merging
toward the AMD tree.

Regards,
Jason

Re: [PATCH 0/2] Two bug-fixes for HMM

[Kuehling, Felix]

On 2019-06-06 11:11 a.m., Jason Gunthorpe wrote:
> On Fri, May 10, 2019 at 07:53:21PM +0000, Kuehling, Felix wrote:
>> These problems were found in AMD-internal testing as we're working on
>> adopting HMM. They are rebased against glisse/hmm-5.2-v3. We'd like to get
>> them applied to a mainline Linux kernel as well as drm-next and
>> amd-staging-drm-next sooner rather than later.
>>
>> Currently the HMM in amd-staging-drm-next is quite far behind hmm-5.2-v3,
>> but the driver changes for HMM are expected to land in 5.2 and will need to
>> be rebased on those HMM changes.
>>
>> I'd like to work out a flow between Jerome, Dave, Alex and myself that
>> allows us to test the latest version of HMM on amd-staging-drm-next so
>> that ideally everything comes together in master without much need for
>> rebasing and retesting.
> I think we have that now, I'm running a hmm.git that is clean and can
> be used for merging into DRM related trees (and RDMA). I've commited
> to send this tree to Linus at the start of the merge window.
>
> See here:
>
>   https://lore.kernel.org/lkml/20190524124455.GB16845@ziepe.ca/
>
> The tree is here:
>
>   https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=hmm
>
> However please consult with me before making a merge commit to be
> co-ordinated. Thanks
>
> I see in this thread that AMDGPU missed 5.2 beacause of the
> co-ordination problems this tree is intended to solve, so I'm very
> hopeful this will help your work move into 5.3!

Thanks Jason. Our two patches below were already included in the MM tree 
(https://ozlabs.org/~akpm/mmots/broken-out/). With your new hmm.git, 
does that mean HMM fixes and changes will no longer go through Andrew 
Morton but directly through your tree to Linus?

We have also applied the two patches to our internal tree which is 
currently based on 5.2-rc1 so we can make progress.

Alex, I think merging hmm would be an extra step every time you rebase 
amd-staging-drm-next. We could probably also merge hmm at other times as 
needed. Do you think this would cause trouble or confusion for 
upstreaming through drm-next?

Regards,
   Felix


>
>> Maybe having Jerome's latest HMM changes in drm-next. However, that may
>> create dependencies where Jerome and Dave need to coordinate their pull-
>> requests for master.
>>
>> Felix Kuehling (1):
>>    mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking
>>
>> Philip Yang (1):
>>    mm/hmm: support automatic NUMA balancing
> I've applied both of these patches with Jerome's Reviewed-by to
> hmm.git and added the missed Signed-off-by
>
> If you test and confirm I think this branch would be ready for merging
> toward the AMD tree.
> Regards,
> Jason

Re: [PATCH 0/2] Two bug-fixes for HMM

[Kuehling, Felix]

[resent with correct address for Alex]

On 2019-06-06 11:11 a.m., Jason Gunthorpe wrote:

> On Fri, May 10, 2019 at 07:53:21PM +0000, Kuehling, Felix wrote:
>> These problems were found in AMD-internal testing as we're working on
>> adopting HMM. They are rebased against glisse/hmm-5.2-v3. We'd like 
>> to get
>> them applied to a mainline Linux kernel as well as drm-next and
>> amd-staging-drm-next sooner rather than later.
>>
>> Currently the HMM in amd-staging-drm-next is quite far behind hmm-5.2-v3,
>> but the driver changes for HMM are expected to land in 5.2 and will 
>> need to
>> be rebased on those HMM changes.
>>
>> I'd like to work out a flow between Jerome, Dave, Alex and myself that
>> allows us to test the latest version of HMM on amd-staging-drm-next so
>> that ideally everything comes together in master without much need for
>> rebasing and retesting.
> I think we have that now, I'm running a hmm.git that is clean and can
> be used for merging into DRM related trees (and RDMA). I've commited
> to send this tree to Linus at the start of the merge window.
>
> See here:
>
> https://lore.kernel.org/lkml/20190524124455.GB16845@ziepe.ca/
>
> The tree is here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=hmm
>
> However please consult with me before making a merge commit to be
> co-ordinated. Thanks
>
> I see in this thread that AMDGPU missed 5.2 beacause of the
> co-ordination problems this tree is intended to solve, so I'm very
> hopeful this will help your work move into 5.3!

Thanks Jason. Our two patches below were already included in the MM tree 
(https://ozlabs.org/~akpm/mmots/broken-out/). With your new hmm.git, 
does that mean HMM fixes and changes will no longer go through Andrew 
Morton but directly through your tree to Linus?

We have also applied the two patches to our internal tree which is 
currently based on 5.2-rc1 so we can make progress.

Alex, I think merging hmm would be an extra step every time you rebase 
amd-staging-drm-next. We could probably also merge hmm at other times as 
needed. Do you think this would cause trouble or confusion for 
upstreaming through drm-next?

Regards,
   Felix


>
>> Maybe having Jerome's latest HMM changes in drm-next. However, that may
>> create dependencies where Jerome and Dave need to coordinate their pull-
>> requests for master.
>>
>> Felix Kuehling (1):
>> mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking
>>
>> Philip Yang (1):
>> mm/hmm: support automatic NUMA balancing
> I've applied both of these patches with Jerome's Reviewed-by to
> hmm.git and added the missed Signed-off-by
>
> If you test and confirm I think this branch would be ready for merging
> toward the AMD tree.
> Regards,
> Jason

Re: [PATCH 0/2] Two bug-fixes for HMM

[Jason Gunthorpe]

On Thu, Jun 06, 2019 at 07:04:46PM +0000, Kuehling, Felix wrote:
> On 2019-06-06 11:11 a.m., Jason Gunthorpe wrote:
> > On Fri, May 10, 2019 at 07:53:21PM +0000, Kuehling, Felix wrote:
> >> These problems were found in AMD-internal testing as we're working on
> >> adopting HMM. They are rebased against glisse/hmm-5.2-v3. We'd like to get
> >> them applied to a mainline Linux kernel as well as drm-next and
> >> amd-staging-drm-next sooner rather than later.
> >>
> >> Currently the HMM in amd-staging-drm-next is quite far behind hmm-5.2-v3,
> >> but the driver changes for HMM are expected to land in 5.2 and will need to
> >> be rebased on those HMM changes.
> >>
> >> I'd like to work out a flow between Jerome, Dave, Alex and myself that
> >> allows us to test the latest version of HMM on amd-staging-drm-next so
> >> that ideally everything comes together in master without much need for
> >> rebasing and retesting.
> > I think we have that now, I'm running a hmm.git that is clean and can
> > be used for merging into DRM related trees (and RDMA). I've commited
> > to send this tree to Linus at the start of the merge window.
> >
> > See here:
> >
> >   https://lore.kernel.org/lkml/20190524124455.GB16845@ziepe.ca/
> >
> > The tree is here:
> >
> >   https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=hmm
> >
> > However please consult with me before making a merge commit to be
> > co-ordinated. Thanks
> >
> > I see in this thread that AMDGPU missed 5.2 beacause of the
> > co-ordination problems this tree is intended to solve, so I'm very
> > hopeful this will help your work move into 5.3!
> 
> Thanks Jason. Our two patches below were already included in the MM tree 
> (https://ozlabs.org/~akpm/mmots/broken-out/). With your new hmm.git, 
> does that mean HMM fixes and changes will no longer go through Andrew 
> Morton but directly through your tree to Linus?

I belive so, that is what we agreed to in the RFC. At least for this
cycle.

I already noticed the duplication and sent Andrew a separate note..

It will be best if most of the things touching mm/hmm.c go to hmm.git
to avoid conflicts for Linus.

> We have also applied the two patches to our internal tree which is 
> currently based on 5.2-rc1 so we can make progress.

Makes sense, this is is also why this shared tree should be very
helpful..

I intend to run it as a clean and stable non-rebasing tree, ah
probably starting tomorrow since I see there is still another
fixup. :)

> Alex, I think merging hmm would be an extra step every time you rebase
> amd-staging-drm-next. We could probably also merge hmm at other times as
> needed. Do you think this would cause trouble or confusion for 
> upstreaming through drm-next?

I'm not sure what the workflow the amd tree uses, but..

Broadly: If the AMD tree is rebasing then likely you can simply rebase
your AMD tree on top of hmm.git (or maybe hmm.git merge'd into
DRM).

Most likely we will want to send a PR for hmm.git to main DRM tree
prior to merging AMD's tree, but I'm also rather relying on DRM folks
to help build the workflow they want in their world..

There are quite a few options depending on people's preferences and
workflow.

Thanks,
Jason