From: Jason Gunthorpe Date: Wed, 10 Feb 2021 12:56:34 +0000 Subject: Re: [PATCH 1/9] mm/migrate.c: Always allow device private pages to migrate Message-Id: <20210210125634.GL4718@ziepe.ca> List-Id: References: <20210209010722.13839-1-apopple@nvidia.com> <20210209010722.13839-2-apopple@nvidia.com> <20210209133932.GD4718@ziepe.ca> <1780857.6Ip0F2Sa4d@nvdebian> In-Reply-To: <1780857.6Ip0F2Sa4d@nvdebian> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Alistair Popple Cc: linux-mm@kvack.org, nouveau@lists.freedesktop.org, bskeggs@redhat.com, akpm@linux-foundation.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, kvm-ppc@vger.kernel.org, dri-devel@lists.freedesktop.org, jhubbard@nvidia.com, rcampbell@nvidia.com, jglisse@redhat.com On Wed, Feb 10, 2021 at 02:40:10PM +1100, Alistair Popple wrote: > On Wednesday, 10 February 2021 12:39:32 AM AEDT Jason Gunthorpe wrote: > > On Tue, Feb 09, 2021 at 12:07:14PM +1100, Alistair Popple wrote: > > > Device private pages are used to represent device memory that is not > > > directly accessible from the CPU. Extra references to a device private > > > page are only used to ensure the struct page itself remains valid whilst > > > waiting for migration entries. Therefore extra references should not > > > prevent device private page migration as this can lead to failures to > > > migrate pages back to the CPU which are fatal to the user process. > > > > This should identify the extra references in expected_count, just > > disabling this protection seems unsafe, ZONE_DEVICE is not so special > > that the refcount means nothing > > This is similar to what migarte_vma_check_page() does now. The issue is that a > migration wait takes a reference on the device private page so you can end up > with one thread stuck waiting for migration whilst the other can't migrate due > to the extra refcount. > > Given device private pages can't undergo GUP and that it's not possible to > differentiate the migration wait refcount from any other refcount we assume > any possible extra reference must be from migration wait. GUP is not the only thing that elevates the refcount, I think this is an unsafe assumption Why is migration holding an extra refcount anyhow? Jason