linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: Jerome Glisse <jglisse@redhat.com>, Hillf Danton <hdanton@sina.com>
Cc: John Hubbard <jhubbard@nvidia.com>, linux-mm <linux-mm@kvack.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Vlastimil Babka <vbabka@suse.cz>, Jan Kara <jack@suse.cz>,
	Mel Gorman <mgorman@suse.de>,
	Dan Williams <dan.j.williams@intel.com>,
	Ira Weiny <ira.weiny@intel.com>, Christoph Hellwig <hch@lst.de>,
	Jonathan Corbet <corbet@lwn.net>
Subject: Re: [RFC] mm: gup: add helper page_try_gup_pin(page)
Date: Tue, 5 Nov 2019 09:56:51 +0100	[thread overview]
Message-ID: <2f271007-0a46-bc9f-fca9-4fceeb819cc1@redhat.com> (raw)
In-Reply-To: <20191104190355.GH5134@redhat.com>

On 04.11.19 20:03, Jerome Glisse wrote:
> On Mon, Nov 04, 2019 at 06:20:50PM +0800, Hillf Danton wrote:
>>
>> On Sun, 3 Nov 2019 22:09:03 -0800 John Hubbard wrote:
>>> On 11/3/19 8:34 PM, Hillf Danton wrote:
>>> ...
>>>>>
>>>>> Well, as long as we're counting bits, I've taken 21 bits (!) to track
>>>>> "gupers". :)  More accurately, I'm sharing 31 bits with get_page()...please
>>>>
>>>> Would you please specify the reasoning of tracking multiple gupers
>>>> for a dirty page? Do you mean that it is all fine for guper-A to add
>>>> changes to guper-B's data without warning and vice versa?
>>>
>>> It's generally OK to call get_user_pages() on a page more than once.
>>
>> Does this explain that it's generally OK to gup pin a page under
>> writeback and then start DMA to it behind the flusher's back without
>> warning?
> 
> It can happens today, is it ok ... well no but we live in an imperfect
> world. GUP have been abuse by few device driver over the years and those
> never checked what it meant to use it so now we are left with existing
> device driver that we can not break that do wrong thing.
> 
> I personaly think that we should use bounce page for writeback so that
> writeback can still happens if a page is GUPed. John's patchset is the
> first step to be able to identify GUPed page and maybe special case them.
> 
>>
>>> And even though we are seeing some work to reduce the number of places
>>> in the kernel that call get_user_pages(), there are still lots of call sites.
>>> That means lots of combinations and situations that could result in more
>>> than one gup call per page.
>>>
>>> Furthermore, there is no mechanism, convention, documentation, nor anything
>>> at all that attempts to enforce "for each page, get_user_pages() may only
>>> be called once."
>>
>> What sense is this making wrt the data corruption resulting specifically
>> from multiple gup references?
> 
> Multiple GUP references do not imply corruption. Only one or more devices
> writing to the page while writeback is happening is a cause of corruption.
> Multiple device writting in the same page concurrently is like multiple
> CPU thread doing the same. Either the application/device drivers are doing
> this rightfully on purpose or the application has a bug. Either way it is
> not our problem (note here i am talking about userspace portion of the
> device driver).
> 

If I'm not completely off, we can have multiple GUP references easily by 
using KVM+VFIO.

-- 

Thanks,

David / dhildenb



  reply	other threads:[~2019-11-05  8:57 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-03 11:21 [RFC] mm: gup: add helper page_try_gup_pin(page) Hillf Danton
2019-11-03 20:20 ` John Hubbard
2019-11-04  4:34 ` Hillf Danton
2019-11-04  6:09   ` John Hubbard
2019-11-04  8:13     ` Jan Kara
2019-11-04 10:20   ` Hillf Danton
2019-11-04 19:03     ` Jerome Glisse
2019-11-05  8:56       ` David Hildenbrand [this message]
2019-11-05  4:27     ` Hillf Danton
2019-11-05 15:54       ` Jerome Glisse
2019-11-06  9:22       ` Hillf Danton
2019-11-06 15:46         ` Jerome Glisse
2019-11-07  9:50         ` Hillf Danton
2019-11-07 14:57           ` Jerome Glisse
2019-11-08  9:38           ` Hillf Danton
2019-11-08 13:59             ` Jerome Glisse

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=2f271007-0a46-bc9f-fca9-4fceeb819cc1@redhat.com \
    --to=david@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=corbet@lwn.net \
    --cc=dan.j.williams@intel.com \
    --cc=hch@lst.de \
    --cc=hdanton@sina.com \
    --cc=ira.weiny@intel.com \
    --cc=jack@suse.cz \
    --cc=jglisse@redhat.com \
    --cc=jhubbard@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=vbabka@suse.cz \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).