[PATCH] drm/amdgpu: allow write access to mapped userptrs

[Christian König]

Am 18.03.2016 um 11:02 schrieb Jerome Glisse:
> On Thu, Mar 17, 2016 at 08:26:36PM +0100, Christian König wrote:
>> Am 17.03.2016 um 20:18 schrieb Jerome Glisse:
>>> On Fri, Mar 11, 2016 at 3:29 PM, Christian König
>>> <deathsimple at vodafone.de> wrote:
>>>> From: Christian König <christian.koenig at amd.com>
>>>>
>>>> With the updated MMU notifier we should also be able to
>>>> handle the writeback case correctly.
>>>>
>>> Out of curiosity what are you refering too ? I do not see anything
>>> special on amdgpu_mn.c logs and i do not see why you could not use
>>> then for write before.
>> We moved the get_user_pages() outside of reserving the BO and tested that
>> quite extensively.
>>
>> And don't ask me why that shouldn't have worked. It was you who gave the
>> advise to not allow it.
>>
>> I think the rational was something like that the writeback code disables CPU
>> writes, compute a CRC and start to write the page to disk. When the GPU
>> could write to the page in that moment the CRC won't match any more and we
>> would get errors reported from the disk driver.
>>
>> But I think that the MMU notifier should catch that case as well.
> So to make sure we talk about same code i am looking at drm-next-4.6 :
> https://cgit.freedesktop.org/~agd5f/linux/tree/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c?h=drm-next-4.6&id=390be2824fa4211c2e973c69b72e04000559bba3
>
> In which you only register invalidate_range_start and it is not enough
> to handle page write back (see mm/rmap.c page_mkclean_one() call from
> page_mkclean()). So this patch is kind of dangerous as it could end up
> with filesystem corruption (depending on fs and how unlucky you are).
>
> So you either need to add invalidate_page() callback to mmu_notifier or
> at very least reinstate the AMDGPU_GEM_USERPTR_ANONONLY flag.

Oh, crap. Yeah thanks for the info. I will be looking into that.

Either I can come up with a quick solution or we need to revert that 
once more.

Regards,
Christian.

>
> Cheers,
> Jérôme