Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1624403 > unrolled thread

Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory

Back to article view | Back to linux.kernel

Contents

  Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-16 17:50 +0200
    Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-16 18:50 +0200
      Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-17 00:40 +0200
        Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-17 07:20 +0200
          Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-17 09:30 +0200
            Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-17 19:00 +0200
              Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-17 19:10 +0200
                Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-18 07:30 +0200
              Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jerome Glisse <jglisse@redhat.com> - 2017-04-17 20:10 +0200
                Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-18 08:20 +0200
              Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-17 23:20 +0200
                Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-18 07:50 +0200
                  Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-18 08:40 +0200
    Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-17 00:30 +0200
      Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-18 18:50 +0200
        Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-18 19:30 +0200
          Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-18 20:10 +0200
            Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-18 20:40 +0200
            Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-19 03:20 +0200
          Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 01:00 +0200
          Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-19 02:10 +0200
        Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-18 20:40 +0200
          Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-18 21:10 +0200
            Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-18 21:40 +0200
              Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-18 21:50 +0200
                Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-18 22:10 +0200
              Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-18 21:50 +0200
                Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jerome Glisse <jglisse@redhat.com> - 2017-04-18 22:30 +0200
                  Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-18 22:40 +0200
                    Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-18 22:50 +0200
                      Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-19 03:20 +0200
                Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-18 23:10 +0200
                  Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-18 23:20 +0200
                    Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-18 23:30 +0200
                      Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-18 23:40 +0200
                        Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 00:20 +0200
                          Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 00:30 +0200
                            Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-19 00:50 +0200
                              Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 01:00 +0200
                                Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-19 01:30 +0200
                                  Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-19 03:30 +0200
                            Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 00:50 +0200
                              Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 01:00 +0200
                                Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 01:00 +0200
                                  Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 01:10 +0200
                      Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-19 03:30 +0200
                  Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-18 23:40 +0200
                    Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-19 00:30 +0200
                      Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 01:10 +0200
                      Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-19 04:10 +0200
                  Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-19 03:30 +0200
                    Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-19 18:00 +0200
                      Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 18:50 +0200
                        Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 19:10 +0200
                          Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jerome Glisse <jglisse@redhat.com> - 2017-04-19 19:40 +0200
                            Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 19:50 +0200
                              Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 20:20 +0200
                                Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 20:20 +0200
                                  Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 20:40 +0200
                                    Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 20:50 +0200
                                    Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 20:50 +0200
                              Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-20 22:50 +0200
                                Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory "Stephen  Bates" <sbates@raithlin.com> - 2017-04-21 01:10 +0200
                                  Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-21 07:10 +0200
                              Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory "Stephen  Bates" <sbates@raithlin.com> - 2017-04-20 22:50 +0200
                        Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-19 19:20 +0200
                          Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 20:10 +0200
                            Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-19 20:40 +0200
                              Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 21:10 +0200
                                Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-19 21:40 +0200
                                  Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 21:50 +0200
                                    Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-19 22:50 +0200
                                      Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-20 01:00 +0200
                                        Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@gmail.com> - 2017-04-20 02:10 +0200

Page 1 of 4  [1] 2 3 4  Next page →

#1624403 — Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory

On Sat, Apr 15, 2017 at 10:36 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
>
>
> On 15/04/17 04:17 PM, Benjamin Herrenschmidt wrote:
>> You can't. If the iommu is on, everything is remapped. Or do you mean
>> to have dma_map_* not do a remapping ?
>
> Well, yes, you'd have to change the code so that iomem pages do not get
> remapped and the raw BAR address is passed to the DMA engine. I said
> specifically we haven't done this at this time but it really doesn't
> seem like an unsolvable problem. It is something we will need to address
> before a proper patch set is posted though.
>
>> That's the problem again, same as before, for that to work, the
>> dma_map_* ops would have to do something special that depends on *both*
>> the source and target device.
>
> No, I don't think you have to do things different based on the source.
> Have the p2pmem device layer restrict allocating p2pmem based on the
> devices in use (similar to how the RFC code works now) and when the dma
> mapping code sees iomem pages it just needs to leave the address alone
> so it's used directly by the dma in question.
>
> It's much better to make the decision on which memory to use when you
> allocate it. If you wait until you map it, it would be a pain to fall
> back to system memory if it doesn't look like it will work. So, if when
> you allocate it, you know everything will work you just need the dma
> mapping layer to stay out of the way.

I think we very much want the dma mapping layer to be in the way.
It's the only sane semantic we have to communicate this translation.

>
>> The dma_ops today are architecture specific and have no way to
>> differenciate between normal and those special P2P DMA pages.
>
> Correct, unless Dan's idea works (which will need some investigation),
> we'd need a flag in struct page or some other similar method to
> determine that these are special iomem pages.
>
>>> Though if it does, I'd expect
>>> everything would still work you just wouldn't get the performance or
>>> traffic flow you are looking for. We've been testing with the software
>>> iommu which doesn't have this problem.
>>
>> So first, no, it's more than "you wouldn't get the performance". On
>> some systems it may also just not work. Also what do you mean by "the
>> SW iommu doesn't have this problem" ? It catches the fact that
>> addresses don't point to RAM and maps differently ?
>
> I haven't tested it but I can't imagine why an iommu would not correctly
> map the memory in the bar. But that's _way_ beside the point. We
> _really_ want to avoid that situation anyway. If the iommu maps the
> memory it defeats what we are trying to accomplish.
>
> I believe the sotfware iommu only uses bounce buffers if the DMA engine
> in use cannot address the memory. So in most cases, with modern
> hardware, it just passes the BAR's address to the DMA engine and
> everything works. The code posted in the RFC does in fact work without
> needing to do any of this fussing.
>
>>>> The problem is that the latter while seemingly easier, is also slower
>>>> and not supported by all platforms and architectures (for example,
>>>> POWER currently won't allow it, or rather only allows a store-only
>>>> subset of it under special circumstances).
>>>
>>> Yes, I think situations where we have to cross host bridges will remain
>>> unsupported by this work for a long time. There are two many cases where
>>> it just doesn't work or it performs too poorly to be useful.
>>
>> And the situation where you don't cross bridges is the one where you
>> need to also take into account the offsets.
>
> I think for the first incarnation we will just not support systems that
> have offsets. This makes things much easier and still supports all the
> use cases we are interested in.
>
>> So you are designing something that is built from scratch to only work
>> on a specific limited category of systems and is also incompatible with
>> virtualization.
>
> Yes, we are starting with support for specific use cases. Almost all
> technology starts that way. Dax has been in the kernel for years and
> only recently has someone submitted patches for it to support pmem on
> powerpc. This is not unusual. If you had forced the pmem developers to
> support all architectures in existence before allowing them upstream
> they couldn't possibly be as far as they are today.

The difference is that there was nothing fundamental in the core
design of pmem + DAX that prevented other archs from growing pmem
support. THP and memory hotplug existed on other architectures and
they just need to plug in their arch-specific enabling. p2p support
needs the same starting point of something more than one architecture
can plug into, and handling the bus address offset case needs to be
incorporated into the design.

pmem + dax did not change the meaning of what a dma_addr_t is, p2p does.

> Virtualization specifically would be a _lot_ more difficult than simply
> supporting offsets. The actual topology of the bus will probably be lost
> on the guest OS and it would therefor have a difficult time figuring out
> when it's acceptable to use p2pmem. I also have a difficult time seeing
> a use case for it and thus I have a hard time with the argument that we
> can't support use cases that do want it because use cases that don't
> want it (perhaps yet) won't work.
>
>> This is an interesting experiement to look at I suppose, but if you
>> ever want this upstream I would like at least for you to develop a
>> strategy to support the wider case, if not an actual implementation.
>
> I think there are plenty of avenues forward to support offsets, etc.
> It's just work. Nothing we'd be proposing would be incompatible with it.
> We just don't want to have to do it all upfront especially when no one
> really knows how well various architecture's hardware supports this or
> if anyone even wants to run it on systems such as those. (Keep in mind
> this is a pretty specific optimization that mostly helps systems
> designed in specific ways -- not a general "everybody gets faster" type
> situation.) Get the cases working we know will work, can easily support
> and people actually want.  Then expand it to support others as people
> come around with hardware to test and use cases for it.

I think you need to give other archs a chance to support this with a
design that considers the offset case as a first class citizen rather
than an afterthought.

[toc] | [next] | [standalone]

#1624410

On 16/04/17 09:44 AM, Dan Williams wrote:
> I think we very much want the dma mapping layer to be in the way.
> It's the only sane semantic we have to communicate this translation.

Yes, I wasn't proposing bypassing that layer, per say. I just meant that
the layer would, in the end, have to return the address without any
translations.

> The difference is that there was nothing fundamental in the core
> design of pmem + DAX that prevented other archs from growing pmem
> support. THP and memory hotplug existed on other architectures and
> they just need to plug in their arch-specific enabling. p2p support
> needs the same starting point of something more than one architecture
> can plug into, and handling the bus address offset case needs to be
> incorporated into the design.

I don't think there's a difference there either. There'd have been
nothing fundamental in our core design that says offsets couldn't have
been added later.

> pmem + dax did not change the meaning of what a dma_addr_t is, p2p does.

I don't think p2p actually really changes the meaning of dma_addr_t
either. We are just putting addresses in there that weren't used
previously. Our RFC makes no changes to anything even remotely related
to dma_addr_t.

> I think you need to give other archs a chance to support this with a
> design that considers the offset case as a first class citizen rather
> than an afterthought.

I'll consider this. Given the fact I can use your existing
get_dev_pagemap infrastructure to look up the p2pmem device this
probably isn't as hard as I thought it would be anyway (we probably
don't even need a page flag). We'd just have lookup the dev_pagemap,
test if it's a p2pmem device, and if so, call a p2pmem_dma_map function
which could apply the offset or do any other arch specific logic (if
necessary).

Logan

[toc] | [prev] | [next] | [standalone]

#1624478

On Sun, 2017-04-16 at 10:47 -0600, Logan Gunthorpe wrote:
> > I think you need to give other archs a chance to support this with a
> > design that considers the offset case as a first class citizen rather
> > than an afterthought.
> 
> I'll consider this. Given the fact I can use your existing
> get_dev_pagemap infrastructure to look up the p2pmem device this
> probably isn't as hard as I thought it would be anyway (we probably
> don't even need a page flag). We'd just have lookup the dev_pagemap,
> test if it's a p2pmem device, and if so, call a p2pmem_dma_map function
> which could apply the offset or do any other arch specific logic (if
> necessary).

I'm still not 100% why do you need a "p2mem device" mind you ...

Cheers,
Ben.

[toc] | [prev] | [next] | [standalone]

#1624532

On 16/04/17 04:32 PM, Benjamin Herrenschmidt wrote:
>> I'll consider this. Given the fact I can use your existing
>> get_dev_pagemap infrastructure to look up the p2pmem device this
>> probably isn't as hard as I thought it would be anyway (we probably
>> don't even need a page flag). We'd just have lookup the dev_pagemap,
>> test if it's a p2pmem device, and if so, call a p2pmem_dma_map function
>> which could apply the offset or do any other arch specific logic (if
>> necessary).
> 
> I'm still not 100% why do you need a "p2mem device" mind you ...

Well, you don't "need" it but it is a design choice that I think makes a
lot of sense for the following reasons:

1) p2pmem is in fact a device on the pci bus. A pci driver will need to
set it up and create the device and thus it will have a natural parent
pci device. Instantiating a struct device for it means it will appear in
the device hierarchy and one can use that to reason about its position
in the topology.

2) In order to create the struct pages we use the ZONE_DEVICE
infrastructure which requires a struct device. (See
devm_memremap_pages.) This amazingly gets us the get_dev_pagemap
architecture which also uses a struct device. So by using a p2pmem
device we can go from struct page to struct device to p2pmem device
quickly and effortlessly.

3) You wouldn't want to use the pci's struct device because it doesn't
really describe what's going on. For example, there may be multiple
devices on the pci device in question: eg. an NVME card and some p2pmem.
Or it could be a NIC with some p2pmem. Or it could just be p2pmem by
itself. And the logic to figure out what memory is available and where
the address is will be non-standard so it's really straightforward to
have any pci driver just instantiate a p2pmem device.

It is probably worth you reading the RFC patches at this point to get a
better feel for this.

Logan

[toc] | [prev] | [next] | [standalone]

#1624581

On Sun, 2017-04-16 at 23:13 -0600, Logan Gunthorpe wrote:
> 
> > 
> > I'm still not 100% why do you need a "p2mem device" mind you ...
> 
> Well, you don't "need" it but it is a design choice that I think makes a
> lot of sense for the following reasons:
> 
> 1) p2pmem is in fact a device on the pci bus. A pci driver will need to
> set it up and create the device and thus it will have a natural parent
> pci device. Instantiating a struct device for it means it will appear in
> the device hierarchy and one can use that to reason about its position
> in the topology.

But is it ? For example take a GPU, does it, in your scheme, need an
additional "p2pmem" child ? Why can't the GPU driver just use some
helper to instantiate the necessary struct pages ? What does having an
actual "struct device" child buys you ?

> 2) In order to create the struct pages we use the ZONE_DEVICE
> infrastructure which requires a struct device. (See
> devm_memremap_pages.)

Yup, but you already have one in the actual pci_dev ... What is the
benefit of adding a second one ?

>  This amazingly gets us the get_dev_pagemap
> architecture which also uses a struct device. So by using a p2pmem
> device we can go from struct page to struct device to p2pmem device
> quickly and effortlessly.

Which isn't terribly useful in itself right ? What you care about is
the "enclosing" pci_dev no ? Or am I missing something ?

> 3) You wouldn't want to use the pci's struct device because it doesn't
> really describe what's going on. For example, there may be multiple
> devices on the pci device in question: eg. an NVME card and some p2pmem.

What is "some p2pmem" ?

> Or it could be a NIC with some p2pmem.

Again what is "some p2pmem" ?

That a device might have some memory-like buffer space is all well and
good but does it need to be specifically distinguished at the device
level ? It could be inherent to what the device is... for example again
take the GPU example, why would you call the FB memory "p2pmem" ? 

>  Or it could just be p2pmem by itself. And the logic to figure out what
>  memory is available and where
> the address is will be non-standard so it's really straightforward to
> have any pci driver just instantiate a p2pmem device.

Again I'm not sure why it needs to "instanciate a p2pmem" device. Maybe
it's the term "p2pmem" that offputs me. If p2pmem allowed to have a
standard way to lookup the various offsets etc... I mentioned earlier,
then yes, it would make sense to have it as a staging point. As-is, I
don't know. 

> It is probably worth you reading the RFC patches at this point to get a
> better feel for this.

Yup, I'll have another look a bit more in depth.

Cheers,
Ben.


> Logan

[toc] | [prev] | [next] | [standalone]

#1624743

On 17/04/17 01:20 AM, Benjamin Herrenschmidt wrote:
> But is it ? For example take a GPU, does it, in your scheme, need an
> additional "p2pmem" child ? Why can't the GPU driver just use some
> helper to instantiate the necessary struct pages ? What does having an
> actual "struct device" child buys you ?

Yes, in this scheme, it needs an additional p2pmem child. Why is that an
issue? It certainly makes it a lot easier for the user to understand the
p2pmem memory in the system (through the sysfs tree) and reason about
the topology and when to use it. This is important.

> 
>> 2) In order to create the struct pages we use the ZONE_DEVICE
>> infrastructure which requires a struct device. (See
>> devm_memremap_pages.)
> 
> Yup, but you already have one in the actual pci_dev ... What is the
> benefit of adding a second one ?

But that would tie all of this very tightly to be pci only and may get
hard to differentiate if more users of ZONE_DEVICE crop up who happen to
be using a pci device. Having a specific class for this makes it very
clear how this memory would be handled. For example, although I haven't
looked into it, this could very well be a point of conflict with HMM. If
they were to use the pci device to populate the dev_pagemap then we
couldn't also use the pci device. I feel it's much better for users of
dev_pagemap to have their struct devices they own to avoid such conflicts.

> 
>>  This amazingly gets us the get_dev_pagemap
>> architecture which also uses a struct device. So by using a p2pmem
>> device we can go from struct page to struct device to p2pmem device
>> quickly and effortlessly.
> 
> Which isn't terribly useful in itself right ? What you care about is
> the "enclosing" pci_dev no ? Or am I missing something ?

Sure it is. What if we want to someday support p2pmem that's on another bus?

>> 3) You wouldn't want to use the pci's struct device because it doesn't
>> really describe what's going on. For example, there may be multiple
>> devices on the pci device in question: eg. an NVME card and some p2pmem.
> 
> What is "some p2pmem" ?
>> Or it could be a NIC with some p2pmem.
> 
> Again what is "some p2pmem" ?

Some device local memory intended for use as a DMA target from a
neighbour device or itself. On a PCI device, this would be a BAR, or a
portion of a BAR with memory behind it.

Keep in mind device classes tend to carve out common use cases and don't
have a one to one mapping with a physical pci card.

> That a device might have some memory-like buffer space is all well and
> good but does it need to be specifically distinguished at the device
> level ? It could be inherent to what the device is... for example again
> take the GPU example, why would you call the FB memory "p2pmem" ? 

Well if you are using it for p2p transactions why wouldn't you call it
p2pmem? There's no technical downside here except some vague argument
over naming. Once registered as p2pmem, that device will handle all the
dma map stuff for you and have a central obvious place to put code which
helps decide whether to use it or not based on topology.

I can certainly see an issue you'd have with the current RFC in that the
p2pmem device currently also handles memory allocation which a GPU would
 want to do itself. There are plenty of solutions to this though: we
could provide hooks for the parent device to override allocation or
something like that. However, the use cases I'm concerned with don't do
their own allocation so that is an important feature for them.

> Again I'm not sure why it needs to "instanciate a p2pmem" device. Maybe
> it's the term "p2pmem" that offputs me. If p2pmem allowed to have a
> standard way to lookup the various offsets etc... I mentioned earlier,
> then yes, it would make sense to have it as a staging point. As-is, I
> don't know. 

Well of course, at some point it would have a standard way to lookup
offsets and figure out what's necessary for a mapping. We wouldn't make
that separate from this, that would make no sense.

I also forgot:

4) We need someway in the kernel to configure drivers that use p2pmem.
That means it needs a unique name that the user can understand, lookup
and pass to other drivers. Then a way for those drivers to find it in
the system. A specific device class gets that for us in a very simple
fashion. We also don't want to have drivers like nvmet having to walk
every pci device to figure out where the p2p memory is and whether it
can use it.

IMO there are many clear benefits here and you haven't really offered an
alternative that provides the same features and potential for future use
cases.

Logan

[toc] | [prev] | [next] | [standalone]

#1624745

On Mon, Apr 17, 2017 at 9:52 AM, Logan Gunthorpe <logang@deltatee.com> wrote:
>
>
> On 17/04/17 01:20 AM, Benjamin Herrenschmidt wrote:
>> But is it ? For example take a GPU, does it, in your scheme, need an
>> additional "p2pmem" child ? Why can't the GPU driver just use some
>> helper to instantiate the necessary struct pages ? What does having an
>> actual "struct device" child buys you ?
>
> Yes, in this scheme, it needs an additional p2pmem child. Why is that an
> issue? It certainly makes it a lot easier for the user to understand the
> p2pmem memory in the system (through the sysfs tree) and reason about
> the topology and when to use it. This is important.

I think you want to go the other way in the hierarchy and find a
shared *parent* to land the p2pmem capability. Because that same agent
is going to be responsible handling address translation for the peers.

>>> 2) In order to create the struct pages we use the ZONE_DEVICE
>>> infrastructure which requires a struct device. (See
>>> devm_memremap_pages.)
>>
>> Yup, but you already have one in the actual pci_dev ... What is the
>> benefit of adding a second one ?
>
> But that would tie all of this very tightly to be pci only and may get
> hard to differentiate if more users of ZONE_DEVICE crop up who happen to
> be using a pci device. Having a specific class for this makes it very
> clear how this memory would be handled. For example, although I haven't
> looked into it, this could very well be a point of conflict with HMM. If
> they were to use the pci device to populate the dev_pagemap then we
> couldn't also use the pci device. I feel it's much better for users of
> dev_pagemap to have their struct devices they own to avoid such conflicts.

Peer-dma is always going to be a property of the bus and not the end
devices. Requiring each bus implementation to explicitly enable
peer-to-peer support is a feature not a bug.

>>>  This amazingly gets us the get_dev_pagemap
>>> architecture which also uses a struct device. So by using a p2pmem
>>> device we can go from struct page to struct device to p2pmem device
>>> quickly and effortlessly.
>>
>> Which isn't terribly useful in itself right ? What you care about is
>> the "enclosing" pci_dev no ? Or am I missing something ?
>
> Sure it is. What if we want to someday support p2pmem that's on another bus?

We shouldn't design for some future possible use case. Solve it for
pci and when / if another bus comes along then look at a more generic
abstraction.

[toc] | [prev] | [next] | [standalone]

#1625029

On 17/04/17 11:04 AM, Dan Williams wrote:
>> Yes, in this scheme, it needs an additional p2pmem child. Why is that an
>> issue? It certainly makes it a lot easier for the user to understand the
>> p2pmem memory in the system (through the sysfs tree) and reason about
>> the topology and when to use it. This is important.
> 
> I think you want to go the other way in the hierarchy and find a
> shared *parent* to land the p2pmem capability. Because that same agent
> is going to be responsible handling address translation for the peers.
>
> Peer-dma is always going to be a property of the bus and not the end
> devices. Requiring each bus implementation to explicitly enable
> peer-to-peer support is a feature not a bug.
> 
> We shouldn't design for some future possible use case. Solve it for
> pci and when / if another bus comes along then look at a more generic
> abstraction.

Thanks Dan, these are some good points. Wedding it closer to the PCI
code makes more sense to me now. I'd still think you'd want some struct
device though to appear in the device hierarchy and allow reasoning
about topology.

Logan

[toc] | [prev] | [next] | [standalone]

#1624781

On Mon, Apr 17, 2017 at 10:52:29AM -0600, Logan Gunthorpe wrote:
> 
> 
> On 17/04/17 01:20 AM, Benjamin Herrenschmidt wrote:
> > But is it ? For example take a GPU, does it, in your scheme, need an
> > additional "p2pmem" child ? Why can't the GPU driver just use some
> > helper to instantiate the necessary struct pages ? What does having an
> > actual "struct device" child buys you ?
> 
> Yes, in this scheme, it needs an additional p2pmem child. Why is that an
> issue? It certainly makes it a lot easier for the user to understand the
> p2pmem memory in the system (through the sysfs tree) and reason about
> the topology and when to use it. This is important.

I disagree here. I would rather see Peer-to-Peer mapping as a form
of helper so that device driver can opt-in for multiple mecanisms
concurrently. Like HMM and p2p.

Also it seems you are having a static vision for p2p. For GPU and
network the use case is you move some buffer into the device memory
and then you create mapping for some network adapter while the buffer
is in device memory. But this is only temporary and buffer might
move to different device memory. So usecase is highly dynamic (well
mapping lifetime is still probably few second/minutes).

I see no reason for exposing sysfs tree to userspace for all this.
This isn't too dynamic, either 2 devices can access each others
memory, either they can't. This can be hidden through the device
kernel API. Again for GPU the idea is that it is always do-able
in the sense that when it is not you fallback to using system
memory.


> >> 2) In order to create the struct pages we use the ZONE_DEVICE
> >> infrastructure which requires a struct device. (See
> >> devm_memremap_pages.)
> > 
> > Yup, but you already have one in the actual pci_dev ... What is the
> > benefit of adding a second one ?
> 
> But that would tie all of this very tightly to be pci only and may get
> hard to differentiate if more users of ZONE_DEVICE crop up who happen to
> be using a pci device. Having a specific class for this makes it very
> clear how this memory would be handled. For example, although I haven't
> looked into it, this could very well be a point of conflict with HMM. If
> they were to use the pci device to populate the dev_pagemap then we
> couldn't also use the pci device. I feel it's much better for users of
> dev_pagemap to have their struct devices they own to avoid such conflicts.

Yes this could conflict and that's why i would rather see this as a set
of helper like HMM is doing. So device driver can opt-in HMM and p2pmem
at the same time.


> >>  This amazingly gets us the get_dev_pagemap
> >> architecture which also uses a struct device. So by using a p2pmem
> >> device we can go from struct page to struct device to p2pmem device
> >> quickly and effortlessly.
> > 
> > Which isn't terribly useful in itself right ? What you care about is
> > the "enclosing" pci_dev no ? Or am I missing something ?
> 
> Sure it is. What if we want to someday support p2pmem that's on another bus?
> 
> >> 3) You wouldn't want to use the pci's struct device because it doesn't
> >> really describe what's going on. For example, there may be multiple
> >> devices on the pci device in question: eg. an NVME card and some p2pmem.
> > 
> > What is "some p2pmem" ?
> >> Or it could be a NIC with some p2pmem.
> > 
> > Again what is "some p2pmem" ?
> 
> Some device local memory intended for use as a DMA target from a
> neighbour device or itself. On a PCI device, this would be a BAR, or a
> portion of a BAR with memory behind it.
> 
> Keep in mind device classes tend to carve out common use cases and don't
> have a one to one mapping with a physical pci card.
> 
> > That a device might have some memory-like buffer space is all well and
> > good but does it need to be specifically distinguished at the device
> > level ? It could be inherent to what the device is... for example again
> > take the GPU example, why would you call the FB memory "p2pmem" ? 
> 
> Well if you are using it for p2p transactions why wouldn't you call it
> p2pmem? There's no technical downside here except some vague argument
> over naming. Once registered as p2pmem, that device will handle all the
> dma map stuff for you and have a central obvious place to put code which
> helps decide whether to use it or not based on topology.
> 
> I can certainly see an issue you'd have with the current RFC in that the
> p2pmem device currently also handles memory allocation which a GPU would
>  want to do itself. There are plenty of solutions to this though: we
> could provide hooks for the parent device to override allocation or
> something like that. However, the use cases I'm concerned with don't do
> their own allocation so that is an important feature for them.

This seems to duplicate things that already exist in each individual
driver. If a device has memory than device driver already have some
form of memory management and most likely expose some API to userspace
to allow program to use that memory.

Peer to peer DMA mapping is orthogonal to memory management, it is
an optimization ie you have some buffer allocated through some device
driver specific IOCTL and now you want some other device to directly
access it. Having to first do another allocation in a different device
driver for that to happen seems overkill.

What you really want from device driver point of view is an helper to
first tell you if 2 device can access each other and second an helper
that allow the second device to import the other device memory to allow
direct access.


> > Again I'm not sure why it needs to "instanciate a p2pmem" device. Maybe
> > it's the term "p2pmem" that offputs me. If p2pmem allowed to have a
> > standard way to lookup the various offsets etc... I mentioned earlier,
> > then yes, it would make sense to have it as a staging point. As-is, I
> > don't know. 
> 
> Well of course, at some point it would have a standard way to lookup
> offsets and figure out what's necessary for a mapping. We wouldn't make
> that separate from this, that would make no sense.
> 
> I also forgot:
> 
> 4) We need someway in the kernel to configure drivers that use p2pmem.
> That means it needs a unique name that the user can understand, lookup
> and pass to other drivers. Then a way for those drivers to find it in
> the system. A specific device class gets that for us in a very simple
> fashion. We also don't want to have drivers like nvmet having to walk
> every pci device to figure out where the p2p memory is and whether it
> can use it.
> 
> IMO there are many clear benefits here and you haven't really offered an
> alternative that provides the same features and potential for future use
> cases.

Discovering possible peer is a onetime only thing and designing around
that is wrong in my view. There is already existing hierarchy in kernel
for that in the form of the bus hierarchy (i am thinking pci bus here).
So there is already existing way to discover this and you are just
duplicating informations here.

Cheers,
Jérôme

[toc] | [prev] | [next] | [standalone]

#1625050

On 17/04/17 12:04 PM, Jerome Glisse wrote:
> I disagree here. I would rather see Peer-to-Peer mapping as a form
> of helper so that device driver can opt-in for multiple mecanisms
> concurrently. Like HMM and p2p.

I'm not against moving some of the common stuff into a library. It
sounds like the problems p2pmem solves don't overlap much with the
problems of the GPU and moving the stuff we have in common somewhere
else seems sensible.

> Also it seems you are having a static vision for p2p. For GPU and
> network the use case is you move some buffer into the device memory
> and then you create mapping for some network adapter while the buffer
> is in device memory. But this is only temporary and buffer might
> move to different device memory. So usecase is highly dynamic (well
> mapping lifetime is still probably few second/minutes).

I feel like you will need to pin the memory while it's the target of a
DMA transaction. If some network peer is sending you data and you just
invalidated the memory it is headed to then you are just going to break
applications. But really this isn't our concern: the memory we are using
with this work will be static and not prone to disappearing.

> I see no reason for exposing sysfs tree to userspace for all this.
> This isn't too dynamic, either 2 devices can access each others
> memory, either they can't. This can be hidden through the device
> kernel API. Again for GPU the idea is that it is always do-able
> in the sense that when it is not you fallback to using system
> memory.

The user has to make a decision to use it or not. This is an
optimization with significant trade-offs that may differ significantly
based on system design.

> Yes this could conflict and that's why i would rather see this as a set
> of helper like HMM is doing. So device driver can opt-in HMM and p2pmem
> at the same time.

I don't understand how that addresses the conflict. We need to each be
using unique and identifiable struct devices in the ZONE_DEVICE
dev_pagemap so we don't apply p2p dma mappings to hmm memory and vice-versa.

> This seems to duplicate things that already exist in each individual
> driver. If a device has memory than device driver already have some
> form of memory management and most likely expose some API to userspace
> to allow program to use that memory.

> Peer to peer DMA mapping is orthogonal to memory management, it is
> an optimization ie you have some buffer allocated through some device
> driver specific IOCTL and now you want some other device to directly
> access it. Having to first do another allocation in a different device
> driver for that to happen seems overkill.

The devices we are working with are adding memory specifically for
enabling p2p applications. The memory is new and there are no allocators
for any of it yet.

Also note: we've gotten _significant_ push back against exposing any of
this memory to userspace. Letting the user unknowingly have to deal with
the issues of iomem is not anything anyone wants to see. Thus we are
dealing with in-kernel users only and they need a common interface to
get the memory from.

> What you really want from device driver point of view is an helper to
> first tell you if 2 device can access each other and second an helper
> that allow the second device to import the other device memory to allow
> direct access.

Well, actually it's a bit more complicated than that but essentially
correct: There can be N devices in the mix and quite likely another
driver completely separate from all N devices. (eg. for our main use
case we have N nvme cards being talked to through an RDMA NIC with it
all being coordinated by the nvme-target driver).


> Discovering possible peer is a onetime only thing and designing around
> that is wrong in my view. There is already existing hierarchy in kernel
> for that in the form of the bus hierarchy (i am thinking pci bus here).
> So there is already existing way to discover this and you are just
> duplicating informations here.

I really don't see the solution you are proposing here. Have the user
specify a pci device name and just have them guess which ones have
suitable memory? Or do they have to walk the entire pci tree to find
ones that have such memory? There was no "duplicate" information created
by our patch set.

Logan

[toc] | [prev] | [next] | [standalone]

#1624876

On Mon, 2017-04-17 at 10:52 -0600, Logan Gunthorpe wrote:
> 
> On 17/04/17 01:20 AM, Benjamin Herrenschmidt wrote:
> > But is it ? For example take a GPU, does it, in your scheme, need an
> > additional "p2pmem" child ? Why can't the GPU driver just use some
> > helper to instantiate the necessary struct pages ? What does having an
> > actual "struct device" child buys you ?
> 
> Yes, in this scheme, it needs an additional p2pmem child. Why is that an
> issue? It certainly makes it a lot easier for the user to understand the
> p2pmem memory in the system (through the sysfs tree) and reason about
> the topology and when to use it. This is important.

Is it ? Again, you create a "concept" the user may have no idea about,
"p2pmem memory". So now any kind of memory buffer on a device can could
be use for p2p but also potentially a bunch of other things becomes
special and called "p2pmem" ...

> > > 2) In order to create the struct pages we use the ZONE_DEVICE
> > > infrastructure which requires a struct device. (See
> > > devm_memremap_pages.)
> > 
> > Yup, but you already have one in the actual pci_dev ... What is the
> > benefit of adding a second one ?
> 
> But that would tie all of this very tightly to be pci only and may get
> hard to differentiate if more users of ZONE_DEVICE crop up who happen to
> be using a pci device.

But what do you have in p2pmem that somebody benefits from. Again I
don't understand what that "p2pmem" device buys you in term of
functionality vs. having the device just instanciate the pages.

Now having some kind of way to override the dma_ops, yes I do get that,
and it could be that this "p2pmem" is typically the way to do it, but
at the moment you don't even have that. So I'm a bit at a loss here.
 
>  Having a specific class for this makes it very
> clear how this memory would be handled.

But it doesn't *have* to be. Again, take my GPU example. The fact that
a NIC might be able to DMA into it doesn't make it specifically "p2p
memory".

Essentially you are saying that any device that happens to have a piece
of mappable "memory" (or something that behaves like it) and can be
DMA'ed into should now have that "p2pmem" thing attached to it.

Now take an example where that becomes really awkward (it's also a real
example of something people want to do). I have a NIC and a GPU, the
NIC DMA's data to/from the GPU, but they also want to poke at each
other doorbell, the GPU to kick the NIC into action when data is ready
to send, the NIC to poke the GPU when data has been received.

Those doorbells are MMIO registers.

So now your "p2pmem" device needs to also be laid out on top of those
MMIO registers ? It's becoming weird.

See, basically, doing peer 2 peer between devices has 3 main challenges
today: The DMA API needing struct pages, the MMIO translation issues
and the IOMMU translation issues.

You seem to create that added device as some kind of "owner" for the
struct pages, solving #1, but leave #2 and #3 alone.

Now, as I said, it could very well be that having the devmap pointer
point to some specific device-type with a well known structure to
provide solutions for #2 and #3 such as dma_ops overrides, is indeed
the right way to solve these problems.

If we go down that path, though, rather than calling it p2pmem I would
call it something like dma_target which I find much clearer especially
since it doesn't have to be just memory.

For the sole case of creating struct page's however, I fail to see the
point.

>  For example, although I haven't
> looked into it, this could very well be a point of conflict with HMM. If
> they were to use the pci device to populate the dev_pagemap then we
> couldn't also use the pci device. I feel it's much better for users of
> dev_pagemap to have their struct devices they own to avoid such conflicts.

If we are going to create some sort of struct dma_target, HMM could
potentially just look for the parent if it needs the PCI device.

> > >  This amazingly gets us the get_dev_pagemap
> > > architecture which also uses a struct device. So by using a p2pmem
> > > device we can go from struct page to struct device to p2pmem device
> > > quickly and effortlessly.
> > 
> > Which isn't terribly useful in itself right ? What you care about is
> > the "enclosing" pci_dev no ? Or am I missing something ?
> 
> Sure it is. What if we want to someday support p2pmem that's on another bus?

But why not directly use that other bus' device in that case ?

> > > 3) You wouldn't want to use the pci's struct device because it doesn't
> > > really describe what's going on. For example, there may be multiple
> > > devices on the pci device in question: eg. an NVME card and some p2pmem.
> > 
> > What is "some p2pmem" ?
> > > Or it could be a NIC with some p2pmem.
> > 
> > Again what is "some p2pmem" ?
> 
> Some device local memory intended for use as a DMA target from a
> neighbour device or itself. On a PCI device, this would be a BAR, or a
> portion of a BAR with memory behind it.

So back to my base objections:

 - There is no reason why this has to just be memory. There are good
reasons to want to do peer DMA to MMIO registers (see above)

 - There is no reason why that memory on a device is specifically
dedicated to "peer to peer" and thus calling it "p2pmem" is something
I find actually confusing.

> Keep in mind device classes tend to carve out common use cases and don't
> have a one to one mapping with a physical pci card.
> 
> > That a device might have some memory-like buffer space is all well and
> > good but does it need to be specifically distinguished at the device
> > level ? It could be inherent to what the device is... for example again
> > take the GPU example, why would you call the FB memory "p2pmem" ? 
> 
> Well if you are using it for p2p transactions why wouldn't you call it
> p2pmem?

Im not only using it for that :)

>  There's no technical downside here except some vague argument
> over naming. Once registered as p2pmem, that device will handle all the
> dma map stuff for you and have a central obvious place to put code which
> helps decide whether to use it or not based on topology.

Except it doesn't handle any of the dma_map stuff today as far as I can
see.

> I can certainly see an issue you'd have with the current RFC in that the
> p2pmem device currently also handles memory allocation which a GPU would
>  want to do itself.

The memory allocation should be a completely orthogonal and separate
thing yes. You are conflating two completely different things now into
a single concept.

>  There are plenty of solutions to this though: we
> could provide hooks for the parent device to override allocation or
> something like that. However, the use cases I'm concerned with don't do
> their own allocation so that is an important feature for them.

No, the allocation should not even have links to the DMA peering
mechanism. This is completely orthogonal.

I feel more and more like your entire infrastructure is designed for a
special use case and conflates several problems of that specific use
case into one single "solution" rather than separating the various
problems and solving them independently.

> > Again I'm not sure why it needs to "instanciate a p2pmem" device. Maybe
> > it's the term "p2pmem" that offputs me. If p2pmem allowed to have a
> > standard way to lookup the various offsets etc... I mentioned earlier,
> > then yes, it would make sense to have it as a staging point. As-is, I
> > don't know. 
> 
> Well of course, at some point it would have a standard way to lookup
> offsets and figure out what's necessary for a mapping. We wouldn't make
> that separate from this, that would make no sense.
> 
> I also forgot:
> 
> 4) We need someway in the kernel to configure drivers that use p2pmem.
> That means it needs a unique name that the user can understand, lookup
> and pass to other drivers. Then a way for those drivers to find it in
> the system. A specific device class gets that for us in a very simple
> fashion. We also don't want to have drivers like nvmet having to walk
> every pci device to figure out where the p2p memory is and whether it
> can use it.
> 
> IMO there are many clear benefits here and you haven't really offered an
> alternative that provides the same features and potential for future use
> cases.
> 
> Logan

[toc] | [prev] | [next] | [standalone]

#1625036

On 17/04/17 03:11 PM, Benjamin Herrenschmidt wrote:
> Is it ? Again, you create a "concept" the user may have no idea about,
> "p2pmem memory". So now any kind of memory buffer on a device can could
> be use for p2p but also potentially a bunch of other things becomes
> special and called "p2pmem" ...

The user is going to have to have an idea about it if they are designing
systems to make use of it. I've said it before many times: this is an
optimization with significant trade-offs so the user does have to make
decisions regarding when to enable it.


> But what do you have in p2pmem that somebody benefits from. Again I
> don't understand what that "p2pmem" device buys you in term of
> functionality vs. having the device just instanciate the pages.

Well thanks for just taking a big shit on all of our work without even
reading the patches. Bravo.

> Now having some kind of way to override the dma_ops, yes I do get that,
> and it could be that this "p2pmem" is typically the way to do it, but
> at the moment you don't even have that. So I'm a bit at a loss here.

Yes, we've already said many times that this is something we will need
to add.

> But it doesn't *have* to be. Again, take my GPU example. The fact that
> a NIC might be able to DMA into it doesn't make it specifically "p2p
> memory".

Just because you use it for other things doesn't mean it can't also
provide the service of a "p2pmem" device.

> So now your "p2pmem" device needs to also be laid out on top of those
> MMIO registers ? It's becoming weird.

Yes, Max Gurtovoy has also expressed an interest in expanding this work
to cover things other than memory. He's suggested simply calling it a
p2p device, but until we figure out what exactly that all means we can't
really finalize a name.

> See, basically, doing peer 2 peer between devices has 3 main challenges
> today: The DMA API needing struct pages, the MMIO translation issues
> and the IOMMU translation issues.
> 
> You seem to create that added device as some kind of "owner" for the
> struct pages, solving #1, but leave #2 and #3 alone.

Well there are other challenges too. Like figuring out when it's
appropriate to use, tying together the device that provides the memory
with the driver tring to use it in DMA transactions, etc, etc. Our patch
set tackles these latter issues.

> If we go down that path, though, rather than calling it p2pmem I would
> call it something like dma_target which I find much clearer especially
> since it doesn't have to be just memory.

I'm not set on the name. My arguments have been specifically for the
existence of an independent struct device. But I'm not really interested
in getting into bike shedding arguments over what to call it at this
time when we don't even really know what it's going to end up doing in
the end.

> The memory allocation should be a completely orthogonal and separate
> thing yes. You are conflating two completely different things now into
> a single concept.

Well we need a uniform way for a driver trying to coordinate a p2p dma
to find and obtain memory from devices that supply it. We are not
dealing with GPUs that already have complicated allocators. We are
dealing with people adding memory to their devices for the _sole_
purpose of enabling p2p transfers. So having a common allocation setup
is seen as a benefit to us.

Logan

[toc] | [prev] | [next] | [standalone]

#1625066

On Mon, 2017-04-17 at 23:43 -0600, Logan Gunthorpe wrote:
> 
> On 17/04/17 03:11 PM, Benjamin Herrenschmidt wrote:
> > Is it ? Again, you create a "concept" the user may have no idea about,
> > "p2pmem memory". So now any kind of memory buffer on a device can could
> > be use for p2p but also potentially a bunch of other things becomes
> > special and called "p2pmem" ...
> 
> The user is going to have to have an idea about it if they are designing
> systems to make use of it. I've said it before many times: this is an
> optimization with significant trade-offs so the user does have to make
> decisions regarding when to enable it.

Not necessarily. There are many cases where the "end user" won't have
any idea. In any case, I think we bring the story down to those two
points of conflating the allocator with the peer to peer DMA, and the
lack of generality in the approach to solve the peer to peer DMA
problem.

> > But what do you have in p2pmem that somebody benefits from. Again I
> > don't understand what that "p2pmem" device buys you in term of
> > functionality vs. having the device just instanciate the pages.
> 
> Well thanks for just taking a big shit on all of our work without even
> reading the patches. Bravo.

Now now now .... calm down. We are being civil here. I'm not shitting
on anything, I'm asking what seems to be a reasonable question in term
of benefits of the approach you have chosen. Using that sort of
language will not get you anywhere. 

> > Now having some kind of way to override the dma_ops, yes I do get that,
> > and it could be that this "p2pmem" is typically the way to do it, but
> > at the moment you don't even have that. So I'm a bit at a loss here.
> 
> Yes, we've already said many times that this is something we will need
> to add.
> 
> > But it doesn't *have* to be. Again, take my GPU example. The fact that
> > a NIC might be able to DMA into it doesn't make it specifically "p2p
> > memory".
> 
> Just because you use it for other things doesn't mean it can't also
> provide the service of a "p2pmem" device.

But there is no such thing as a "p2pmem" device.. that's what I'm
trying to tell you...

As both Jerome and I tried to explain, there are many reason why one
may want to do peer DMA into some device memory, that doesn't make that
memory some kind of "p2pmem". It's trying to stick a generic label onto
something that isn't.

That's why I'm suggesting we disconnect the two aspects. On one hand
the problem of handling p2p DMA, whether the target is some memory,
some MMIO registers, etc...

On the other hand, some generic "utility" that can optionally be used
by drivers to manage a pool of DMA memory in the device, essentially a
simple allocator.

The two things are completely orthogonal.

> > So now your "p2pmem" device needs to also be laid out on top of those
> > MMIO registers ? It's becoming weird.
> 
> Yes, Max Gurtovoy has also expressed an interest in expanding this work
> to cover things other than memory. He's suggested simply calling it a
> p2p device, but until we figure out what exactly that all means we can't
> really finalize a name.

Possibly. In any case, I think it should be separate from the
allocation.

> > See, basically, doing peer 2 peer between devices has 3 main challenges
> > today: The DMA API needing struct pages, the MMIO translation issues
> > and the IOMMU translation issues.
> > 
> > You seem to create that added device as some kind of "owner" for the
> > struct pages, solving #1, but leave #2 and #3 alone.
> 
> Well there are other challenges too. Like figuring out when it's
> appropriate to use, tying together the device that provides the memory
> with the driver tring to use it in DMA transactions, etc, etc. Our patch
> set tackles these latter issues.

But it tries to conflate the allocation, which is basically the fact
that this is some kind of "memory pool" with the problem of doing peer
DMA.

I'm advocating for separating the concepts.

> > If we go down that path, though, rather than calling it p2pmem I would
> > call it something like dma_target which I find much clearer especially
> > since it doesn't have to be just memory.
> 
> I'm not set on the name. My arguments have been specifically for the
> existence of an independent struct device. But I'm not really interested
> in getting into bike shedding arguments over what to call it at this
> time when we don't even really know what it's going to end up doing in
> the end.

It's not bike shedding. It's about taking out the allocator part and
making it clear that this isn't something to lay out on top of a pre-
decided chunk of "memory".

> > The memory allocation should be a completely orthogonal and separate
> > thing yes. You are conflating two completely different things now into
> > a single concept.
> 
> Well we need a uniform way for a driver trying to coordinate a p2p dma
> to find and obtain memory from devices that supply it.

Again, you are bringing everything down to your special case of "p2p
memory". That's where you lose me. This looks like a special case to me
and you are making the centre point of your design.

What we need is:

 - On one hand a way to expose device space (whether it's MMIO
registers, memory, something else ...) to the DMA ops so another device
can do standard dma_map_* to/from it. (Not dma_alloc_* those shouldn't
relate to p2p at all, they are intended for a driver own allocation for
the device it manages). This includes the creation of struct pages and
the mechanism to override/adjust the dma_ops etc.... along with all the
PCI specific gunk to figure out if we are on the same bus or not etc.

 - Some kind of generic utility you can use to manage a pool of
"memory" which seems to be what your special devices use or wantf or
use by peer DMA.

>  We are not
> > dealing with GPUs that already have complicated allocators.> 
We are
> dealing with people adding memory to their devices for the _sole_
> purpose of enabling p2p transfers. So having a common allocation setup
> > i
s seen as a benefit to us.

I'm not disagreeing. I'm saying that it is completely orthogonal to the
solving the the DMA peer issue. I'm simply objecting to conflating the
two.

Cheers,
Ben.

> Logan
> 

[toc] | [prev] | [next] | [standalone]

#1624473

On Sun, 2017-04-16 at 08:44 -0700, Dan Williams wrote:
> The difference is that there was nothing fundamental in the core
> design of pmem + DAX that prevented other archs from growing pmem
> support.

Indeed. In fact we have work in progress support for pmem on power
using experimental HW.

> THP and memory hotplug existed on other architectures and
> they just need to plug in their arch-specific enabling. p2p support
> needs the same starting point of something more than one architecture
> can plug into, and handling the bus address offset case needs to be
> incorporated into the design.
> 
> pmem + dax did not change the meaning of what a dma_addr_t is, p2p does.

The more I think about it, the more I tend toward something along the
lines of having the arch DMA ops being able to quickly differentiate
between "normal" memory (which includes non-PCI pmem in some cases,
it's an architecture choice I suppose) and "special device" (page flag
? pfn bit ? ... there are options).

From there, we keep our existing fast path for the normal case.

For the special case, we need to provide a fast lookup mechanism
(assuming we can't stash enough stuff in struct page or the pfn)
to get back to a struct of some sort that provides the necessary
information to resolve the translation.

This *could* be something like a struct p2mem device that carries
a special set of DMA ops, though we probably shouldn't make the generic
structure PCI specific.

This is a slightly slower path, but that "stub" structure allows the
special DMA ops to provide the necessary bus-specific knowledge, which
for PCI for example, can check whether the devices are on the same
segment, whether the switches are configured to allow p2p, etc...

What form should that fast lookup take ? It's not completely clear to
me at that point. We could start with a simple linear lookup I suppose
and improve in a second stage.

Of course this pipes into the old discussion about disconnecting
the DMA ops from struct page. If we keep struct page, any device that
wants to be a potential DMA target will need to do something "special"
to create those struct pages etc.. though we could make that a simple
pci helper that pops the necessary bits and pieces for a given BAR &
range.

If we don't need struct page, then it might be possible to hide it all
in the PCI infrastructure.

> > Virtualization specifically would be a _lot_ more difficult than simply
> > supporting offsets. The actual topology of the bus will probably be lost
> > on the guest OS and it would therefor have a difficult time figuring out
> > when it's acceptable to use p2pmem. I also have a difficult time seeing
> > a use case for it and thus I have a hard time with the argument that we
> > can't support use cases that do want it because use cases that don't
> > want it (perhaps yet) won't work.
> > 
> > > This is an interesting experiement to look at I suppose, but if you
> > > ever want this upstream I would like at least for you to develop a
> > > strategy to support the wider case, if not an actual implementation.
> > 
> > I think there are plenty of avenues forward to support offsets, etc.
> > It's just work. Nothing we'd be proposing would be incompatible with it.
> > We just don't want to have to do it all upfront especially when no one
> > really knows how well various architecture's hardware supports this or
> > if anyone even wants to run it on systems such as those. (Keep in mind
> > this is a pretty specific optimization that mostly helps systems
> > designed in specific ways -- not a general "everybody gets faster" type
> > situation.) Get the cases working we know will work, can easily support
> > and people actually want.  Then expand it to support others as people
> > come around with hardware to test and use cases for it.
> 
> I think you need to give other archs a chance to support this with a
> design that considers the offset case as a first class citizen rather
> than an afterthought.

Thanks :-) There's a reason why I'm insisting on this. We have constant
requests for this today. We have hacks in the GPU drivers to do it for
GPUs behind a switch, but those are just that, ad-hoc hacks in the
drivers. We have similar grossness around the corner with some CAPI
NICs trying to DMA to GPUs. I have people trying to use PLX DMA engines
to whack nVME devices.

I'm very interested in a more generic solution to deal with the problem
of P2P between devices. I'm happy to contribute with code to handle the
powerpc bits but we need to agree on the design first :)

Cheers,
Ben.

[toc] | [prev] | [next] | [standalone]

#1625454

On Mon, Apr 17, 2017 at 08:23:16AM +1000, Benjamin Herrenschmidt wrote:
 
> Thanks :-) There's a reason why I'm insisting on this. We have constant
> requests for this today. We have hacks in the GPU drivers to do it for
> GPUs behind a switch, but those are just that, ad-hoc hacks in the
> drivers. We have similar grossness around the corner with some CAPI
> NICs trying to DMA to GPUs. I have people trying to use PLX DMA engines
> to whack nVME devices.

A lot of people feel this way in the RDMA community too. We have had
vendors shipping out of tree code to enable P2P for RDMA with GPU
years and years now. :(

Attempts to get things in mainline have always run into the same sort
of road blocks you've identified in this thread..

FWIW, I read this discussion and it sounds closer to an agreement than
I've ever seen in the past.

From Ben's comments, I would think that the 'first class' support that
is needed here is simply a function to return the 'struct device'
backing a CPU address range.

This is the minimal required information for the arch or IOMMU code
under the dma ops to figure out the fabric source/dest, compute the
traffic path, determine if P2P is even possible, what translation
hardware is crossed, and what DMA address should be used.

If there is going to be more core support for this stuff I think it
will be under the topic of more robustly describing the fabric to the
core and core helpers to extract data from the description: eg compute
the path, check if the path crosses translation, etc

But that isn't really related to P2P, and is probably better left to
the arch authors to figure out where they need to enhance the existing
topology data..

I think the key agreement to get out of Logan's series is that P2P DMA
means:
 - The BAR will be backed by struct pages
 - Passing the CPU __iomem address of the BAR to the DMA API is
   valid and, long term, dma ops providers are expected to fail
   or return the right DMA address
 - Mapping BAR memory into userspace and back to the kernel via
   get_user_pages works transparently, and with the DMA API above
 - The dma ops provider must be able to tell if source memory is bar
   mapped and recover the pci device backing the mapping.

At least this is what we'd like in RDMA :)

FWIW, RDMA probably wouldn't want to use a p2mem device either, we
already have APIs that map BAR memory to user space, and would like to
keep using them. A 'enable P2P for bar' helper function sounds better
to me.

Jason

[toc] | [prev] | [next] | [standalone]

#1625488

On Tue, Apr 18, 2017 at 9:45 AM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> On Mon, Apr 17, 2017 at 08:23:16AM +1000, Benjamin Herrenschmidt wrote:
>
>> Thanks :-) There's a reason why I'm insisting on this. We have constant
>> requests for this today. We have hacks in the GPU drivers to do it for
>> GPUs behind a switch, but those are just that, ad-hoc hacks in the
>> drivers. We have similar grossness around the corner with some CAPI
>> NICs trying to DMA to GPUs. I have people trying to use PLX DMA engines
>> to whack nVME devices.
>
> A lot of people feel this way in the RDMA community too. We have had
> vendors shipping out of tree code to enable P2P for RDMA with GPU
> years and years now. :(
>
> Attempts to get things in mainline have always run into the same sort
> of road blocks you've identified in this thread..
>
> FWIW, I read this discussion and it sounds closer to an agreement than
> I've ever seen in the past.
>
> From Ben's comments, I would think that the 'first class' support that
> is needed here is simply a function to return the 'struct device'
> backing a CPU address range.
>
> This is the minimal required information for the arch or IOMMU code
> under the dma ops to figure out the fabric source/dest, compute the
> traffic path, determine if P2P is even possible, what translation
> hardware is crossed, and what DMA address should be used.
>
> If there is going to be more core support for this stuff I think it
> will be under the topic of more robustly describing the fabric to the
> core and core helpers to extract data from the description: eg compute
> the path, check if the path crosses translation, etc
>
> But that isn't really related to P2P, and is probably better left to
> the arch authors to figure out where they need to enhance the existing
> topology data..
>
> I think the key agreement to get out of Logan's series is that P2P DMA
> means:
>  - The BAR will be backed by struct pages
>  - Passing the CPU __iomem address of the BAR to the DMA API is
>    valid and, long term, dma ops providers are expected to fail
>    or return the right DMA address
>  - Mapping BAR memory into userspace and back to the kernel via
>    get_user_pages works transparently, and with the DMA API above
>  - The dma ops provider must be able to tell if source memory is bar
>    mapped and recover the pci device backing the mapping.
>
> At least this is what we'd like in RDMA :)
>
> FWIW, RDMA probably wouldn't want to use a p2mem device either, we
> already have APIs that map BAR memory to user space, and would like to
> keep using them. A 'enable P2P for bar' helper function sounds better
> to me.

...and I think it's not a helper function as much as asking the bus
provider "can these two device dma to each other". The "helper" is the
dma api redirecting through a software-iommu that handles bus address
translation differently than it would handle host memory dma mapping.

[toc] | [prev] | [next] | [standalone]

#1625509

On Tue, Apr 18, 2017 at 10:27:47AM -0700, Dan Williams wrote:
> > FWIW, RDMA probably wouldn't want to use a p2mem device either, we
> > already have APIs that map BAR memory to user space, and would like to
> > keep using them. A 'enable P2P for bar' helper function sounds better
> > to me.
> 
> ...and I think it's not a helper function as much as asking the bus
> provider "can these two device dma to each other".

What I mean I could write in a RDMA driver:

  /* Allow the memory in BAR 1 to be the target of P2P transactions */
  pci_enable_p2p_bar(dev, 1);

And not require anything else..

> The "helper" is the dma api redirecting through a software-iommu
> that handles bus address translation differently than it would
> handle host memory dma mapping.

Not sure, until we see what arches actually need to do here it is hard
to design common helpers.

Here are a few obvious things that arches will need to implement to
support this broadly:

- Virtualization might need to do a hypervisor call to get the right
  translation, or consult some hypervisor specific description table.

- Anything using IOMMUs for virtualization will need to setup IOMMU
  permissions to allow the P2P flow, this might require translation to
  an address cookie.

- Fail if the PCI devices are in different domains, or setup hardware to
  do completion bus/device/function translation.

- All platforms can succeed if the PCI devices are under the same
  'segment', but where segments begin is somewhat platform specific
  knowledge. (this is 'same switch' idea Logan has talked about)

So, we can eventually design helpers for various common scenarios, but
until we see what arch code actually needs to do it seems
premature. Much of this seems to involve interaction with some kind of
hardware, or consulation of some kind of currently platform specific
data, so I'm not sure what a software-iommu would be doing??

The main thing to agree on is that this code belongs under dma ops and
that arches have to support struct page mapped BAR addresses in their
dma ops inputs. Is that resonable?

Jason

[toc] | [prev] | [next] | [standalone]

#1625531

On Tue, Apr 18, 2017 at 11:00 AM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> On Tue, Apr 18, 2017 at 10:27:47AM -0700, Dan Williams wrote:
>> > FWIW, RDMA probably wouldn't want to use a p2mem device either, we
>> > already have APIs that map BAR memory to user space, and would like to
>> > keep using them. A 'enable P2P for bar' helper function sounds better
>> > to me.
>>
>> ...and I think it's not a helper function as much as asking the bus
>> provider "can these two device dma to each other".
>
> What I mean I could write in a RDMA driver:
>
>   /* Allow the memory in BAR 1 to be the target of P2P transactions */
>   pci_enable_p2p_bar(dev, 1);
>
> And not require anything else..
>
>> The "helper" is the dma api redirecting through a software-iommu
>> that handles bus address translation differently than it would
>> handle host memory dma mapping.
>
> Not sure, until we see what arches actually need to do here it is hard
> to design common helpers.
>
> Here are a few obvious things that arches will need to implement to
> support this broadly:
>
> - Virtualization might need to do a hypervisor call to get the right
>   translation, or consult some hypervisor specific description table.
>
> - Anything using IOMMUs for virtualization will need to setup IOMMU
>   permissions to allow the P2P flow, this might require translation to
>   an address cookie.
>
> - Fail if the PCI devices are in different domains, or setup hardware to
>   do completion bus/device/function translation.
>
> - All platforms can succeed if the PCI devices are under the same
>   'segment', but where segments begin is somewhat platform specific
>   knowledge. (this is 'same switch' idea Logan has talked about)
>
> So, we can eventually design helpers for various common scenarios, but
> until we see what arch code actually needs to do it seems
> premature. Much of this seems to involve interaction with some kind of
> hardware, or consulation of some kind of currently platform specific
> data, so I'm not sure what a software-iommu would be doing??
>
> The main thing to agree on is that this code belongs under dma ops and
> that arches have to support struct page mapped BAR addresses in their
> dma ops inputs. Is that resonable?

I think we're saying the same thing by "software-iommu" and "custom
dma_ops", so yes.

[toc] | [prev] | [next] | [standalone]

#1625767

On Tue, 2017-04-18 at 12:00 -0600, Jason Gunthorpe wrote:
> - All platforms can succeed if the PCI devices are under the same
>   'segment', but where segments begin is somewhat platform specific
>   knowledge. (this is 'same switch' idea Logan has talked about)

We also need to be careful whether P2P is enabled in the switch
or not.

Cheers,
Ben.

[toc] | [prev] | [next] | [standalone]

#1625690

On Tue, Apr 18, 2017 at 3:46 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2017-04-18 at 10:27 -0700, Dan Williams wrote:
>> > FWIW, RDMA probably wouldn't want to use a p2mem device either, we
>> > already have APIs that map BAR memory to user space, and would like to
>> > keep using them. A 'enable P2P for bar' helper function sounds better
>> > to me.
>>
>> ...and I think it's not a helper function as much as asking the bus
>> provider "can these two device dma to each other". The "helper" is the
>> dma api redirecting through a software-iommu that handles bus address
>> translation differently than it would handle host memory dma mapping.
>
> Do we even need tat function ? The dma_ops have a dma_supported()
> call...
>
> If we have those override ops built into the "dma_target" object,
> then these things can make that decision knowing both the source
> and target device.
>

Yes.

[toc] | [prev] | [next] | [standalone]

Page 1 of 4  [1] 2 3 4  Next page →

Back to top | Article view | linux.kernel

csiph-web