Re: [PATCH v4 2/2] mm/gup/writeback: add callbacks for inaccessible pages

From: Christian Borntraeger <borntraeger@de.ibm.com>
To: Dave Hansen <dave.hansen@intel.com>,
	Claudio Imbrenda <imbrenda@linux.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Andy Lutomirski <luto@kernel.org>,
	linux-next@vger.kernel.org, akpm@linux-foundation.org,
	jack@suse.cz, kirill@shutemov.name, "Edgecombe,
	Rick P" <rick.p.edgecombe@intel.com>,
	Sean Christopherson <sean.j.christopherson@intel.com>,
	david@redhat.com, aarcange@redhat.com, linux-mm@kvack.org,
	frankja@linux.ibm.com, sfr@canb.auug.org.au, jhubbard@nvidia.com,
	linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org,
	Will Deacon <will@kernel.org>,
	"Williams, Dan J" <dan.j.williams@intel.com>
Subject: Re: [PATCH v4 2/2] mm/gup/writeback: add callbacks for inaccessible pages
Date: Tue, 28 Apr 2020 22:02:38 +0200	[thread overview]
Message-ID: <929f7d1f-0cc8-e140-6f74-4797704e5185@de.ibm.com> (raw)
In-Reply-To: <42fccd01-7e16-b18f-cd81-4040857d80d4@intel.com>

On 28.04.20 21:43, Dave Hansen wrote:
> On 4/21/20 2:31 PM, Dave Hansen wrote:
>> On 4/16/20 12:02 PM, Dave Hansen wrote:
>>> On 4/16/20 9:34 AM, Claudio Imbrenda wrote:
>>>>> Ahh, so this is *just* intended to precede I/O done on the page, when
>>>>> a non-host entity is touching the memory?
>>>> yep
>>> OK, so we've got to do an action that precedes *all* I/O to a page.
>>> That's not too bad.
>>>
>>> I still don't understand how this could work generally, though  There
>>> are lots of places where I/O is done to a page without either going
>>> through __test_set_page_writeback() or gup() with FOLL_PIN set.
>>>
>>> sendfile() is probably the best example of this:
>>>
>>> 	fd = open("/normal/ext4/file", O_RDONLY);
>>> 	sendfile(socket_fd, fd, &off, count);
>>>
>>> There's no gup in sight since the file doesn't have an address and it's
>>> not being written to so there's no writeback.
>>>
>>> How does sendfile work?
>>
>> Did you manage to see if sendfile works (or any other operation that
>> DMAs file-backed data without being preceded by a gup)?
> 
> It's been a couple of weeks with no response on this.
> 
> From where I'm standing, we have a hook in the core VM that can't
> possibly work with some existing kernel functionality and has virtually
> no chance of getting used on a second architecture.
> 
> It sounds like there may need to be some additional work here, but
> should these hooks stay in for 5.7?  Or, should we revert this patch and
> try again for 5.8?

We have something and Claudio is going to send out an addon patch soon. 
Reverting would be harmful basically allowing for I/O errors on a valid
path that can happen with normal use. The sendfile case is also valid, but
it only triggers for a handcrafted cases (you have to change QEMU to use
MAP_SHARED and you have to use a file as memory backing and you have to
splice this file then to another file that is using DIRECT_IO). Those cases
then run into an I/O error case that is recovered by the driver. the same
would not work for a swap backing.

It is also not trivial (or doable) to change all device drivers that exist
on s390 to use the DMA API. In the long run we might be able to get the 
necessary changes into the DMA api.

Claudio will send the details tomorrow. 