linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Inki Dae <inki.dae@samsung.com>
To: 'Jerome Glisse' <j.glisse@gmail.com>
Cc: airlied@linux.ie, dri-devel@lists.freedesktop.org,
	kyungmin.park@samsung.com, sw0312.kim@samsung.com,
	linux-mm@kvack.org
Subject: RE: [PATCH 2/2 v3] drm/exynos: added userptr feature.
Date: Thu, 10 May 2012 11:44:20 +0900	[thread overview]
Message-ID: <001b01cd2e56$cd620270$68260750$%dae@samsung.com> (raw)
In-Reply-To: <CAH3drwapwva24oHQOz+3qbNt2CouoVYmUXeFBs4RkL31bvbY3Q@mail.gmail.com>

Hi Jerome,

Thank you again.

> -----Original Message-----
> From: Jerome Glisse [mailto:j.glisse@gmail.com]
> Sent: Thursday, May 10, 2012 3:33 AM
> To: Inki Dae
> Cc: airlied@linux.ie; dri-devel@lists.freedesktop.org;
> kyungmin.park@samsung.com; sw0312.kim@samsung.com; linux-mm@kvack.org
> Subject: Re: [PATCH 2/2 v3] drm/exynos: added userptr feature.
> 
> On Wed, May 9, 2012 at 10:45 AM, Jerome Glisse <j.glisse@gmail.com> wrote:
> > On Wed, May 9, 2012 at 2:17 AM, Inki Dae <inki.dae@samsung.com> wrote:
> >> this feature is used to import user space region allocated by malloc()
> or
> >> mmaped into a gem. and to guarantee the pages to user space not to be
> >> swapped out, the VMAs within the user space would be locked and then
> unlocked
> >> when the pages are released.
> >>
> >> but this lock might result in significant degradation of system
> performance
> >> because the pages couldn't be swapped out so we limit user-desired
> userptr
> >> size to pre-defined.
> >>
> >> Signed-off-by: Inki Dae <inki.dae@samsung.com>
> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> >
> >
> > Again i would like feedback from mm people (adding cc). I am not sure
> > locking the vma is the right anwser as i said in my previous mail,
> > userspace can munlock it in your back, maybe VM_RESERVED is better.
> > Anyway even not considering that you don't check at all that process
> > don't go over the limit of locked page see mm/mlock.c RLIMIT_MEMLOCK
> > for how it's done. Also you mlock complete vma but the userptr you get
> > might be inside say 16M vma and you only care about 1M of userptr, if
> > you mark the whole vma as locked than anytime a new page is fault in
> > the vma else where than in the buffer you are interested then it got
> > allocated for ever until the gem buffer is destroy, i am not sure of
> > what happen to the vma on next malloc if it grows or not (i would
> > think it won't grow at it would have different flags than new
> > anonymous memory).
> >
> > The whole business of directly using malloced memory for gpu is fishy
> > and i would really like to get it right rather than relying on never
> > hitting strange things like page migration, vma merging, or worse
> > things like over locking pages and stealing memory.
> >
> > Cheers,
> > Jerome
> 
> I had a lengthy discussion with mm people (thx a lot for that). I
> think we should split 2 different use case. The zero-copy upload case
> ie :
> app:
>     ptr = malloc()
>     ...
>     glTex/VBO/UBO/...(ptr)
>     free(ptr) or reuse it for other things
> For which i guess you want to avoid having to do a memcpy inside the
> gl library (could be anything else than gl that have same useage
> pattern).
> 

Right, in this case, we are using the userptr feature as pixman and evas
backend to use 2d accelerator.

> ie after the upload happen you don't care about those page they can
> removed from the vma or marked as cow so that anything messing with
> those page after the upload won't change what you uploaded. Of course

I'm not sure that I understood your mentions but could the pages be removed
from vma with VM_LOCKED or VM_RESERVED? once glTex/VBO/UBO/..., the VMAs to
user space would be locked. if cpu accessed significant part of all the
pages in user mode then pages to the part would be allocated by page fault
handler, after that, through userptr, the VMAs to user address space would
be locked(at this time, the remaining pages would be allocated also by
get_user_pages by calling page fault handler) I'd be glad to give me any
comments and advices if there is my missing point.

> this is assuming that the tlb cost of doing such thing is smaller than
> the cost of memcpy the data.
> 

yes, in our test case, the tlb cost(incurred by tlb miss) was smaller than
the cost of memcpy also cpu usage. of course, this would be depended on gpu
performance.

> Two way to do that, either you assume app can't not read back data
> after gl can and you do an unmap_mapping_range (make sure you only
> unmap fully covered page and that you copy non fully covered page) or
> you want to allow userspace to still read data or possibly overwrite
> them
> 
> Second use case is something more like for the opencl case of
> CL_MEM_USE_HOST_PTR, in which you want to use the same page in the gpu
> and keep the userspace vma pointing to those page. I think the
> agreement on this case is that there is no way right now to do it
> sanely inside linux kernel. mlocking will need proper accounting
> against rtlimit but this limit might be low. Also the fork case might
> be problematic.
> 
> For the fork case the memory is anonymous so it should be COWed in the
> fork child but relative to cl context that means the child could not
> use the cl context with that memory or at least if the child write to
> this memory the cl will not see those change. I guess the answer to
> that one is that you really need to use the cl api to read the object
> or get proper ptr to read it.
> 
> Anyway in all case, implementing this userptr thing need a lot more
> code. You have to check to that the vma you are trying to use is
> anonymous and only handle this case and fallback to alloc new page and
> copy otherwise..
> 

I'd like to say thank you again you gave me comments and advices in detail.
there may be my missing points but I will check it again.

Thanks,
Inki Dae

> Cheers,
> Jerome

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2012-05-10  2:44 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1335188594-17454-4-git-send-email-inki.dae@samsung.com>
     [not found] ` <1336544259-17222-1-git-send-email-inki.dae@samsung.com>
     [not found]   ` <1336544259-17222-3-git-send-email-inki.dae@samsung.com>
2012-05-09 14:45     ` Jerome Glisse
2012-05-09 18:32       ` Jerome Glisse
2012-05-10  2:44         ` Inki Dae [this message]
2012-05-10 15:05           ` Jerome Glisse
2012-05-10 15:31             ` Daniel Vetter
2012-05-10 15:52               ` Jerome Glisse
2012-05-11  1:47                 ` Inki Dae
2012-05-11  2:08                   ` Minchan Kim
2012-05-10  1:39       ` Inki Dae
2012-05-10  4:58         ` Minchan Kim
2012-05-10  6:53           ` KOSAKI Motohiro
2012-05-10  7:27             ` Minchan Kim
2012-05-10  7:31               ` Kyungmin Park
2012-05-10  7:56                 ` Minchan Kim
2012-05-10  7:58                   ` Minchan Kim
2012-05-10  6:57           ` Inki Dae
2012-05-10  7:05             ` Minchan Kim
2012-05-10  7:59               ` InKi Dae
2012-05-10  8:11                 ` Minchan Kim
2012-05-10  8:44                   ` Inki Dae
2012-05-10 17:53                     ` KOSAKI Motohiro
2012-05-11  0:50                       ` Minchan Kim
2012-05-11  2:51                         ` KOSAKI Motohiro
2012-05-11  3:01                           ` Jerome Glisse
2012-05-11 21:20                             ` KOSAKI Motohiro
2012-05-11 22:22                               ` Jerome Glisse
2012-05-11 22:59                                 ` KOSAKI Motohiro
2012-05-11 23:29                                   ` Jerome Glisse
2012-05-11 23:39                                     ` KOSAKI Motohiro
2012-05-12  4:48                                       ` InKi Dae
2012-05-14  4:29                                         ` Minchan Kim
     [not found]   ` <1336976268-14328-1-git-send-email-inki.dae@samsung.com>
2012-05-14  8:12     ` [PATCH 0/2 v4] " Inki Dae
     [not found]     ` <1336976268-14328-2-git-send-email-inki.dae@samsung.com>
2012-05-14  8:12       ` [PATCH 1/2 v4] drm/exynos: added userptr limit ioctl Inki Dae
     [not found]     ` <1336976268-14328-3-git-send-email-inki.dae@samsung.com>
     [not found]       ` <CAHGf_=qv45_uuO_JWMXOQp4VymyOxVq76rGXghoNMmDh7mURKQ@mail.gmail.com>
     [not found]         ` <003001cd319e$263c9230$72b5b690$%dae@samsung.com>
     [not found]           ` <4FB0AE87.60800@gmail.com>
2012-05-14  8:13             ` [PATCH 2/2 v4] drm/exynos: added userptr feature Inki Dae
     [not found]       ` <CAH3drwb13T2RXgEuauGchoZUDAgL+wrv3SR66sZNyGk_6tRTFw@mail.gmail.com>
2012-05-15  4:33         ` Inki Dae
2012-05-15 14:31           ` Jerome Glisse
2012-05-16  8:49             ` Inki Dae

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='001b01cd2e56$cd620270$68260750$%dae@samsung.com' \
    --to=inki.dae@samsung.com \
    --cc=airlied@linux.ie \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=j.glisse@gmail.com \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-mm@kvack.org \
    --cc=sw0312.kim@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox