Re: [PATCH 2/2 v4] drm/exynos: added userptr feature.

[KOSAKI Motohiro <kosaki.motohiro@gmail.com>]

(5/14/12 2:52 AM), Inki Dae wrote:
>
>
>> -----Original Message-----
>> From: KOSAKI Motohiro [mailto:kosaki.motohiro@gmail.com]
>> Sent: Monday, May 14, 2012 3:33 PM
>> To: Inki Dae
>> Cc: airlied@linux.ie; dri-devel@lists.freedesktop.org; j.glisse@gmail.com;
>> minchan@kernel.org; kyungmin.park@samsung.com; sw0312.kim@samsung.com;
>> jy0922.shim@samsung.com
>> Subject: Re: [PATCH 2/2 v4] drm/exynos: added userptr feature.
>>
>>> +       npages = buf->size>>  PAGE_SHIFT;
>>
>> Why round down? usually we use round up.
>>
>
> The size was already rounded up by exynos_drm_gem_userptr_ioctl so this is
> just used to get page count.

got it.



>>> +       down_read(&current->mm->mmap_sem);
>>> +
>>> +       /*
>>> +        * Basically, all the pages from get_user_pages() can not be not
>> only
>>> +        * migrated by CMA but also swapped out.
>>> +        *
>>> +        * The migration issue.
>>> +        * - Pages reserved by CMA for some device using DMA could be
>> used by
>>> +        * kernel and if the device driver wants to use those pages
>>> +        * while being used by kernel then the pages are copied into
>>> +        * other ones allocated to migrate them and then finally,
>>> +        * the device driver can use the pages for itself.
>>> +        * Thus, migrated, the pages being accessed by DMA could be
>> changed
>>> +        * to other so this situation may incur that DMA accesses any
>> pages
>>> +        * it doesn't want.
>>> +        *
>>> +        * But the use of get_user_pages is safe from such magration
>> issue
>>> +        * because all the pages from get_user_pages CAN NOT be not only
>>> +        * migrated, but also swapped out.
>>> +        */
>>> +       get_npages = get_user_pages(current, current->mm, userptr,
>>> +                                       npages, write, 1, buf->pages,
> NULL);
>>
>> Why force=1? It is almostly core-dump specific option. Why don't you
>> return
>
> I know that force indicates whether to force write access even  if user
> mapping is readonly.

right. and then, usually we don't want to ignore access permission. but note,
I'm only talk about generic thing. I have no knowledge drm area.



> so we just want to use pages from get_user_pages as
> read/write permission.

>> EFAULT when the page has write permission. IOW, Why your Xorg module
>> don't map memory w/ PROT_WRITE?
>
> No, Xorg can map memory w/ PROT_WRITE. Couldn't the Xorg map w/ PROT_WRITE
> if force = 1? plz, let me know if there is my missing point.

I meant, if Xorg always use PROT_WRITE, you don't need force=1.