From: "Christian König" <deathsimple@vodafone.de>
To: Jerome Glisse <j.glisse@gmail.com>
Cc: Felix Kuehling <felix.kuehling@amd.com>,
Serguei Sagalovitch <serguei.sagalovitch@amd.com>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH] drm/amdgpu: allow write access to mapped userptrs
Date: Fri, 18 Mar 2016 16:14:19 +0100 [thread overview]
Message-ID: <56EC1B4B.9080608@vodafone.de> (raw)
In-Reply-To: <20160318100253.GA26920@gmail.com>
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@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.
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
prev parent reply other threads:[~2016-03-18 15:14 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-11 14:29 [PATCH] drm/amdgpu: allow write access to mapped userptrs Christian König
2016-03-11 22:02 ` Alex Deucher
2016-03-17 19:18 ` Jerome Glisse
2016-03-17 19:26 ` Christian König
2016-03-18 10:02 ` Jerome Glisse
2016-03-18 15:14 ` Christian König [this message]
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=56EC1B4B.9080608@vodafone.de \
--to=deathsimple@vodafone.de \
--cc=dri-devel@lists.freedesktop.org \
--cc=felix.kuehling@amd.com \
--cc=j.glisse@gmail.com \
--cc=serguei.sagalovitch@amd.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.