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

Groups > linux.kernel > #1588981 > unrolled thread

[PATCH RESEND] drm/via: use get_user_pages_unlocked()

Started byLorenzo Stoakes <lstoakes@gmail.com>
First post2017-02-27 23:20 +0100
Last post2017-02-28 21:40 +0100
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH RESEND] drm/via: use get_user_pages_unlocked() Lorenzo Stoakes <lstoakes@gmail.com> - 2017-02-27 23:20 +0100
    Re: [PATCH RESEND] drm/via: use get_user_pages_unlocked() Daniel Vetter <daniel@ffwll.ch> - 2017-02-28 10:10 +0100
      Re: [PATCH RESEND] drm/via: use get_user_pages_unlocked() Al Viro <viro@ZenIV.linux.org.uk> - 2017-02-28 20:40 +0100
        Re: [PATCH RESEND] drm/via: use get_user_pages_unlocked() Lorenzo Stoakes <lstoakes@gmail.com> - 2017-02-28 21:40 +0100

#1588981 — [PATCH RESEND] drm/via: use get_user_pages_unlocked()

FromLorenzo Stoakes <lstoakes@gmail.com>
Date2017-02-27 23:20 +0100
Subject[PATCH RESEND] drm/via: use get_user_pages_unlocked()
Message-ID<tfBO9-3yE-25@gated-at.bofh.it>
Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.

Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
 drivers/gpu/drm/via/via_dmablit.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/via/via_dmablit.c b/drivers/gpu/drm/via/via_dmablit.c
index 1a3ad769f8c8..98aae9809249 100644
--- a/drivers/gpu/drm/via/via_dmablit.c
+++ b/drivers/gpu/drm/via/via_dmablit.c
@@ -238,13 +238,9 @@ via_lock_all_dma_pages(drm_via_sg_info_t *vsg,  drm_via_dmablit_t *xfer)
 	vsg->pages = vzalloc(sizeof(struct page *) * vsg->num_pages);
 	if (NULL == vsg->pages)
 		return -ENOMEM;
-	down_read(&current->mm->mmap_sem);
-	ret = get_user_pages((unsigned long)xfer->mem_addr,
-			     vsg->num_pages,
-			     (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 0,
-			     vsg->pages, NULL);
-
-	up_read(&current->mm->mmap_sem);
+	ret = get_user_pages_unlocked((unsigned long)xfer->mem_addr,
+			vsg->num_pages, vsg->pages,
+			(vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 0);
 	if (ret != vsg->num_pages) {
 		if (ret < 0)
 			return ret;
-- 
2.11.1

[toc] | [next] | [standalone]

#1589297

FromDaniel Vetter <daniel@ffwll.ch>
Date2017-02-28 10:10 +0100
Message-ID<tfLXc-2mX-1@gated-at.bofh.it>
In reply to#1588981
On Mon, Feb 27, 2017 at 09:50:08PM +0000, Lorenzo Stoakes wrote:
> Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
> and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.
> 
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>

Queued for 4.12, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/via/via_dmablit.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/via/via_dmablit.c b/drivers/gpu/drm/via/via_dmablit.c
> index 1a3ad769f8c8..98aae9809249 100644
> --- a/drivers/gpu/drm/via/via_dmablit.c
> +++ b/drivers/gpu/drm/via/via_dmablit.c
> @@ -238,13 +238,9 @@ via_lock_all_dma_pages(drm_via_sg_info_t *vsg,  drm_via_dmablit_t *xfer)
>  	vsg->pages = vzalloc(sizeof(struct page *) * vsg->num_pages);
>  	if (NULL == vsg->pages)
>  		return -ENOMEM;
> -	down_read(&current->mm->mmap_sem);
> -	ret = get_user_pages((unsigned long)xfer->mem_addr,
> -			     vsg->num_pages,
> -			     (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 0,
> -			     vsg->pages, NULL);
> -
> -	up_read(&current->mm->mmap_sem);
> +	ret = get_user_pages_unlocked((unsigned long)xfer->mem_addr,
> +			vsg->num_pages, vsg->pages,
> +			(vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 0);
>  	if (ret != vsg->num_pages) {
>  		if (ret < 0)
>  			return ret;
> -- 
> 2.11.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

#1589728

FromAl Viro <viro@ZenIV.linux.org.uk>
Date2017-02-28 20:40 +0100
Message-ID<tfVMS-oA-31@gated-at.bofh.it>
In reply to#1589297
On Tue, Feb 28, 2017 at 10:01:10AM +0100, Daniel Vetter wrote:

> > +	ret = get_user_pages_unlocked((unsigned long)xfer->mem_addr,
> > +			vsg->num_pages, vsg->pages,
> > +			(vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 0);

Umm...  Why not
	ret = get_user_pages_fast((unsigned long)xfer->mem_addr,
			vsg->num_pages,
			vsg->direction == DMA_FROM_DEVICE,
			vsg->pages);

IOW, do you really need a warranty that ->mmap_sem will be grabbed and
released?

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

#1589754

FromLorenzo Stoakes <lstoakes@gmail.com>
Date2017-02-28 21:40 +0100
Message-ID<tfWIV-Ym-11@gated-at.bofh.it>
In reply to#1589728
On 28 February 2017 at 19:35, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Tue, Feb 28, 2017 at 10:01:10AM +0100, Daniel Vetter wrote:
>
>> > +   ret = get_user_pages_unlocked((unsigned long)xfer->mem_addr,
>> > +                   vsg->num_pages, vsg->pages,
>> > +                   (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 0);
>
> Umm...  Why not
>         ret = get_user_pages_fast((unsigned long)xfer->mem_addr,
>                         vsg->num_pages,
>                         vsg->direction == DMA_FROM_DEVICE,
>                         vsg->pages);
>
> IOW, do you really need a warranty that ->mmap_sem will be grabbed and
> released?

Daniel will be better placed to answer in this specific case, but more
generally is there any reason why we can't just use
get_user_pages_fast() in all such cases? These patches were simply a
mechanical/cautious replacement for code that is more or less exactly
equivalent but if this would make sense perhaps it'd be worth using
gup_fast() where possible?

-- 
Lorenzo Stoakes
https://ljs.io

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web