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

[Jerome Glisse <j.glisse@gmail.com>]

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@vodafone.de> wrote:
> >>From: Christian König <christian.koenig@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.

Cheers,
Jérôme
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel