Re: [Question] Are "device exclusive non-swap entries" / "SVM atomics in Nouveau" still getting used in practice?

[Alistair Popple <apopple@nvidia.com>]

On Fri, Jan 24, 2025 at 06:54:02PM +0100, David Hildenbrand wrote:
> > > > On integrated the gpu is tied into the coherency
> > > > fabric, so there it's not needed.
> > > > 
> > > > I think the more fundamental question with both this function here and
> > > > with forced migration to device memory is that there's no guarantee it
> > > > will work out.
> > > 
> > > Yes, in particular with device-exclusive, it doesn't really work with THP
> > > and is only limited to anonymous memory. I have patches to at least make it
> > > work reliably with THP.
> > 
> > I should have crawled through the implementation first before replying.
> > Since it only looks at folio_mapcount() make_device_exclusive() should at
> > least in theory work reliably on anon memory, and not be impacted by
> > elevated refcounts due to migration/ksm/thp/whatever.
> 
> Yes, there is -- in theory -- nothing blocking the conversion except the
> folio lock. That's different than page migration.

Indeed - this was the entire motivation for make_device_exclusive() - that we
needed a way to reliably exclude CPU access that couldn't be blocked in the same
way page migration can (otherwise we could have just migrated to a device page,
even if that may have added unwanted overhead).
> [...]
> 
> > 
> > > Then, we seem to give up too easily if we cannot lock the folio when wanting
> > > to convert to device-exclusive, which also looks rather odd. But well, maybe
> > > it just works good enough in the common case, or there is some other retry
> > > logic that makes it fly.
> > 
> > I've crawled through the path to migrate pages from device memory back to
> > system memory a few months ago, and found some livelock issues in there.
> > Wouldn't be surprised if m_d_e has some of the same, but I didn't dig
> > through it (least because intel can't use it because not-so-great hw
> > design).
> 
> Yes, that might be possible. Maybe something keeps spinning while the folio
> is locked instead of properly waiting for the lock.
> 
> ... or someone is trying to convert a tail page of a THP, in which case we
> will also keep failing the conversion right now.
>
> > > There are other concerns I have (what if the page is pinned and access
> > > outside of the user space page tables?). Maybe there was not need to handle
> > > these cases so far.
> > 
> > I think that's also ok, but might be good to document this clearly that
> > concurrent direct i/o or rdma registered buffer or whatever will mess with
> > this. The promise is only between the gpu and the cpu, not anything else,
> > in current apis. At least to my knowledge.

That is correct - we never came up with a good solution for direct i/o or
rdma so the promise is only between cpu and gpu. That said direct i/o probably
implies a shared filebacked mapping. We weren't convinced there was a useful use
case there because anything could potentially map a file and use non-atomic CPU
instructions to access shared the shared mapping anyway.

> Well, the issue is that e.g., iouring fixed buffers can be accessed from the
> CPU using the direct map from the CPU, not necessarily using DMA from a
> device.

Right. This was all targetted at private anonymous memory and it's assumed that
both cpu and gpu are using atomic operations. As soon as you're talking shared
non-anonymous mappings it seems impossible for userspace to guarantee nothing
is accessing that memory with non-atomic ops anyway, even with a coherent
interconnect.

> In any case, I'm planning on adding some code-level documentation for that
> and look into extending the high-level doc while at it.

Thanks! Please CC me in.
 
> > 
> > > So best I can do is make anonymous memory more reliable with
> > > device-exclusive and fixup some of the problematic parts that I see (e.g.,
> > > broken page reclaim, page migration, ...).
> > > 
> > > But before starting to cleanup+improve the existing handling of anonymous
> > > memory, I was wondering if this whole thing is getting used at all.
> > 
> > Yeah if this can be made reliable (under the limitation of only anon
> > memory and only excluding userspace access) then I expect we'll need this
> > for a very long time. I just had no idea whether even that is possible.
> > 
> > What isn't good is if it's only mostly reliable, like the current
> > pgmap->ops->migrate_to_ram() path in do_swap_page() still is.

Right. This is _supposed_ to be 100% reliable under those limitations although
I will admit I didn't consider THP deeply as that is not supported for
DEVICE_PRIVATE anyway.

> I'll cc you on patches once I figure out some details on how to fix some
> page table walkers that really don't expect these non-swap entries.
> 
> Fortunately, the hmm test device is in place to trigger some shaky
> scenarios.
> 
> -- 
> Cheers,
> 
> David / dhildenb
>