Re: Question: "Bare" set_page_dirty_lock() call in vhost.c

From: John Hubbard <jhubbard@nvidia.com>
To: Jan Kara <jack@suse.cz>
Cc: Linux-MM <linux-mm@kvack.org>, <linux-fsdevel@vger.kernel.org>,
	Alex Williamson <alex.williamson@redhat.com>
Subject: Re: Question: "Bare" set_page_dirty_lock() call in vhost.c
Date: Fri, 29 May 2020 00:28:20 -0700	[thread overview]
Message-ID: <6680c2d2-4b45-0e83-96e0-e7d3d421c571@nvidia.com> (raw)
In-Reply-To: <20200529070343.GL14550@quack2.suse.cz>

On 2020-05-29 00:03, Jan Kara wrote:
...
>> ...which actually is the the case that pin_user_pages*() is ultimately
>> helping to avoid, btw. But in this case, it's all code that runs on a
>> CPU, so no DMA or DIO is involved. But still, the "bare" use of
>> set_page_dirty_lock() seems like a problem here.
> 
> I agree that the site like above needs pin_user_pages(). The problem has
> actually nothing to do with kmap_atomic() - that actually doesn't do
> anything interesting on x86_64. The moment GUP_fast() returns, page can be
> unmapped from page tables and written back so this site has exactly same
> problems as any other using DMA or DIO. As we discussed earlier when
> designing the patch set, the problem is really with GUP reference being
> used to access page data. And the method of access (DMA, CPU access, GPU
> access, ...) doesn't really matter...
> 
> 								Honza

Awesome, I was really starting to wonder what I was missing. This all
makes perfect sense, thanks.

Maybe I'll add a "Case 5" to Documentation/core-api/pin_user_pages.rst,
to cover this sort of situation. It's not completely obvious from the
first four cases, that this code is exposed to that problem.

thanks,
-- 
John Hubbard
NVIDIA