* Re: [PATCH RESEND] drm/via: use get_user_pages_unlocked()
@ 2017-02-28 9:01 ` Daniel Vetter
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-02-28 9:01 UTC (permalink / raw)
To: Lorenzo Stoakes; +Cc: linux-mm, dri-devel, linux-kernel
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(¤t->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(¤t->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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH RESEND] drm/via: use get_user_pages_unlocked()
@ 2017-02-28 9:01 ` Daniel Vetter
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-02-28 9:01 UTC (permalink / raw)
To: Lorenzo Stoakes; +Cc: Daniel Vetter, linux-kernel, dri-devel, linux-mm
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(¤t->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(¤t->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
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH RESEND] drm/via: use get_user_pages_unlocked()
2017-02-28 9:01 ` Daniel Vetter
@ 2017-02-28 19:35 ` Al Viro
-1 siblings, 0 replies; 12+ messages in thread
From: Al Viro @ 2017-02-28 19:35 UTC (permalink / raw)
To: Lorenzo Stoakes, linux-kernel, dri-devel, linux-mm
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?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND] drm/via: use get_user_pages_unlocked()
@ 2017-02-28 19:35 ` Al Viro
0 siblings, 0 replies; 12+ messages in thread
From: Al Viro @ 2017-02-28 19:35 UTC (permalink / raw)
To: Lorenzo Stoakes, linux-kernel, dri-devel, linux-mm
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?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND] drm/via: use get_user_pages_unlocked()
2017-02-28 19:35 ` Al Viro
@ 2017-02-28 20:28 ` Lorenzo Stoakes
-1 siblings, 0 replies; 12+ messages in thread
From: Lorenzo Stoakes @ 2017-02-28 20:28 UTC (permalink / raw)
To: Al Viro; +Cc: Linux Kernel Mailing List, dri-devel, linux-mm
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND] drm/via: use get_user_pages_unlocked()
@ 2017-02-28 20:28 ` Lorenzo Stoakes
0 siblings, 0 replies; 12+ messages in thread
From: Lorenzo Stoakes @ 2017-02-28 20:28 UTC (permalink / raw)
To: Al Viro; +Cc: Linux Kernel Mailing List, dri-devel, linux-mm
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
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND] drm/via: use get_user_pages_unlocked()
2017-02-28 20:28 ` Lorenzo Stoakes
(?)
@ 2017-03-01 8:43 ` Daniel Vetter
-1 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-03-01 8:43 UTC (permalink / raw)
To: Lorenzo Stoakes; +Cc: Al Viro, Linux Kernel Mailing List, dri-devel, linux-mm
On Tue, Feb 28, 2017 at 08:28:08PM +0000, Lorenzo Stoakes wrote:
> 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?
I have no idea. drm/via is unmaintained, it's a dri1 racy driver with
problems probably everywhere, and I'm not sure we even have someone left
who cares (there's an out-of-tree kms conversion of via, but it's stuck
since years).
In short, it's the drm dungeons and the only reason I merge patches is to
give people an easy target for test driving the patch submission process
to dri-devel. And to avoid drm being a blocker for tree-wide refactorings.
Otherwise 0 reasons to change anything here.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND] drm/via: use get_user_pages_unlocked()
@ 2017-03-01 8:43 ` Daniel Vetter
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-03-01 8:43 UTC (permalink / raw)
To: Lorenzo Stoakes; +Cc: linux-mm, Al Viro, dri-devel, Linux Kernel Mailing List
On Tue, Feb 28, 2017 at 08:28:08PM +0000, Lorenzo Stoakes wrote:
> 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?
I have no idea. drm/via is unmaintained, it's a dri1 racy driver with
problems probably everywhere, and I'm not sure we even have someone left
who cares (there's an out-of-tree kms conversion of via, but it's stuck
since years).
In short, it's the drm dungeons and the only reason I merge patches is to
give people an easy target for test driving the patch submission process
to dri-devel. And to avoid drm being a blocker for tree-wide refactorings.
Otherwise 0 reasons to change anything here.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND] drm/via: use get_user_pages_unlocked()
@ 2017-03-01 8:43 ` Daniel Vetter
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-03-01 8:43 UTC (permalink / raw)
To: Lorenzo Stoakes; +Cc: Al Viro, Linux Kernel Mailing List, dri-devel, linux-mm
On Tue, Feb 28, 2017 at 08:28:08PM +0000, Lorenzo Stoakes wrote:
> 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?
I have no idea. drm/via is unmaintained, it's a dri1 racy driver with
problems probably everywhere, and I'm not sure we even have someone left
who cares (there's an out-of-tree kms conversion of via, but it's stuck
since years).
In short, it's the drm dungeons and the only reason I merge patches is to
give people an easy target for test driving the patch submission process
to dri-devel. And to avoid drm being a blocker for tree-wide refactorings.
Otherwise 0 reasons to change anything here.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 12+ messages in thread