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

Groups > linux.kernel > #1359527 > unrolled thread

linux-next: manual merge of the tip tree with the drm tree

Started byStephen Rothwell <sfr@canb.auug.org.au>
First post2016-03-17 03:10 +0100
Last post2016-03-17 18:00 +0100
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

Contents

  linux-next: manual merge of the tip tree with the drm tree Stephen Rothwell <sfr@canb.auug.org.au> - 2016-03-17 03:10 +0100
    Re: linux-next: manual merge of the tip tree with the drm tree Arnd Bergmann <arnd@arndb.de> - 2016-03-17 10:10 +0100
      Re: linux-next: manual merge of the tip tree with the drm tree Stephen Rothwell <sfr@canb.auug.org.au> - 2016-03-17 10:40 +0100
      Re: linux-next: manual merge of the tip tree with the drm tree Dave Hansen <dave.hansen@linux.intel.com> - 2016-03-17 18:00 +0100

#1359527 — linux-next: manual merge of the tip tree with the drm tree

FromStephen Rothwell <sfr@canb.auug.org.au>
Date2016-03-17 03:10 +0100
Subjectlinux-next: manual merge of the tip tree with the drm tree
Message-ID<rdvxT-Rq-1@gated-at.bofh.it>
Hi all,

Today's linux-next merge of the tip tree got a conflict in:

  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c

between commit:

  637dd3b5ca9e ("drm/amdgpu: prevent get_user_pages recursion")

from the drm tree and commit:

  d4edcf0d5695 ("mm/gup: Switch all callers of get_user_pages() to not pass tsk/mm")

from the tip tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 0f42b1a24446,7b82e57aa09c..000000000000
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@@ -532,21 -516,9 +532,20 @@@ int amdgpu_ttm_tt_get_user_pages(struc
  	do {
  		unsigned num_pages = ttm->num_pages - pinned;
  		uint64_t userptr = gtt->userptr + pinned * PAGE_SIZE;
 -		struct page **pages = ttm->pages + pinned;
 +		struct page **p = pages + pinned;
 +		struct amdgpu_ttm_gup_task_list guptask;
 +
 +		guptask.task = current;
 +		spin_lock(&gtt->guptasklock);
 +		list_add(&guptask.list, &gtt->guptasks);
 +		spin_unlock(&gtt->guptasklock);
  
- 		r = get_user_pages(current, current->mm, userptr, num_pages,
- 				   write, 0, p, NULL);
+ 		r = get_user_pages(userptr, num_pages, write, 0, pages, NULL);
 +
 +		spin_lock(&gtt->guptasklock);
 +		list_del(&guptask.list);
 +		spin_unlock(&gtt->guptasklock);
 +
  		if (r < 0)
  			goto release_pages;

[toc] | [next] | [standalone]

#1359644

FromArnd Bergmann <arnd@arndb.de>
Date2016-03-17 10:10 +0100
Message-ID<rdC6m-5mE-13@gated-at.bofh.it>
In reply to#1359527
On Thursday 17 March 2016 13:00:29 Stephen Rothwell wrote:
> diff --cc drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 0f42b1a24446,7b82e57aa09c..000000000000
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@@ -532,21 -516,9 +532,20 @@@ int amdgpu_ttm_tt_get_user_pages(struc
>         do {
>                 unsigned num_pages = ttm->num_pages - pinned;
>                 uint64_t userptr = gtt->userptr + pinned * PAGE_SIZE;
>  -              struct page **pages = ttm->pages + pinned;
>  +              struct page **p = pages + pinned;
>  +              struct amdgpu_ttm_gup_task_list guptask;
>  +
>  +              guptask.task = current;
>  +              spin_lock(&gtt->guptasklock);
>  +              list_add(&guptask.list, &gtt->guptasks);
>  +              spin_unlock(&gtt->guptasklock);
>   
> -               r = get_user_pages(current, current->mm, userptr, num_pages,
> -                                  write, 0, p, NULL);
> +               r = get_user_pages(userptr, num_pages, write, 0, pages, NULL);
>  +
>  +              spin_lock(&gtt->guptasklock);
>  +              list_del(&guptask.list);
>  +              spin_unlock(&gtt->guptasklock);
>  +
>                 if (r < 0)
>                         goto release_pages;

Your merge looks incorrect to me, and I got a build warning for it:

drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c: In function 'amdgpu_ttm_tt_get_user_pages':
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:535:17: error: unused variable 'p' [-Werror=unused-variable]

I think the one-line change below is what is needed here, but it's probably
best for amdgpu maintainers to take a closer look.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

index 8b9b245fd0c8..ab34190859a8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -540,7 +540,7 @@ int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages)
 		list_add(&guptask.list, &gtt->guptasks);
 		spin_unlock(&gtt->guptasklock);
 
-		r = get_user_pages(userptr, num_pages, write, 0, pages, NULL);
+		r = get_user_pages(userptr, num_pages, write, 0, p, NULL);
 
 		spin_lock(&gtt->guptasklock);
 		list_del(&guptask.list);

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

#1359664

FromStephen Rothwell <sfr@canb.auug.org.au>
Date2016-03-17 10:40 +0100
Message-ID<rdCzn-5z7-11@gated-at.bofh.it>
In reply to#1359644
Hi Arnd,

On Thu, 17 Mar 2016 10:00:56 +0100 Arnd Bergmann <arnd@arndb.de> wrote:
>
> Your merge looks incorrect to me, and I got a build warning for it:
> 
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c: In function 'amdgpu_ttm_tt_get_user_pages':
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:535:17: error: unused variable 'p' [-Werror=unused-variable]
> 
> I think the one-line change below is what is needed here, but it's probably
> best for amdgpu maintainers to take a closer look.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> index 8b9b245fd0c8..ab34190859a8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -540,7 +540,7 @@ int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages)
>  		list_add(&guptask.list, &gtt->guptasks);
>  		spin_unlock(&gtt->guptasklock);
>  
> -		r = get_user_pages(userptr, num_pages, write, 0, pages, NULL);
> +		r = get_user_pages(userptr, num_pages, write, 0, p, NULL);
>  
>  		spin_lock(&gtt->guptasklock);
>  		list_del(&guptask.list);
> 

Ah, there was another commit to this area (I stopped looking to soon):

  2f568dbd6b94 ("drm/amdgpu: move get_user_pages out of amdgpu_ttm_tt_pin_userptr v6")

Your resolution certainly looks better.  I'll do that tomorrow.

-- 
Cheers,
Stephen Rothwell

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

#1360042

FromDave Hansen <dave.hansen@linux.intel.com>
Date2016-03-17 18:00 +0100
Message-ID<rdJrc-1sp-9@gated-at.bofh.it>
In reply to#1359644
On 03/17/2016 02:00 AM, Arnd Bergmann wrote:
> On Thursday 17 March 2016 13:00:29 Stephen Rothwell wrote:
>> > -               r = get_user_pages(current, current->mm, userptr, num_pages,
>> > -                                  write, 0, p, NULL);
>> > +               r = get_user_pages(userptr, num_pages, write, 0, pages, NULL);
>> >  +
>> >  +              spin_lock(&gtt->guptasklock);
>> >  +              list_del(&guptask.list);
>> >  +              spin_unlock(&gtt->guptasklock);
>> >  +
>> >                 if (r < 0)
>> >                         goto release_pages;
...
> Your merge looks incorrect to me, and I got a build warning for it:
> 
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c: In function 'amdgpu_ttm_tt_get_user_pages':
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:535:17: error: unused variable 'p' [-Werror=unused-variable]
> 
> I think the one-line change below is what is needed here, but it's probably
> best for amdgpu maintainers to take a closer look.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> index 8b9b245fd0c8..ab34190859a8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -540,7 +540,7 @@ int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages)
>  		list_add(&guptask.list, &gtt->guptasks);
>  		spin_unlock(&gtt->guptasklock);
>  
> -		r = get_user_pages(userptr, num_pages, write, 0, pages, NULL);
> +		r = get_user_pages(userptr, num_pages, write, 0, p, NULL);
>  
>  		spin_lock(&gtt->guptasklock);
>  		list_del(&guptask.list);

Yeah, Arnd's fix looks correct to me.  The loop variable "pages" got
renamed to "p" and another variable "pages" is now being passed into the
function.

The get_user_pages() call should be against 'p', the loop variable.

Also, this is obvious if you consider that the pkeys patch was always
just removing the first two arguments.  We can see in Stephen's patch
above that it both removes those arguments *and* replaces 'p' with 'pages'.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web