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

From: Dave Hansen <dave.hansen@intel.com>
To: 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>,
	borntraeger@de.ibm.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 12:43:45 -0700	[thread overview]
Message-ID: <42fccd01-7e16-b18f-cd81-4040857d80d4@intel.com> (raw)
In-Reply-To: <cbaddd28-c5d3-61a2-84d8-c883fb3d6290@intel.com>

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?