Message-ID: <ZEDYt/EQJk39dTuK@infradead.org>
Date:   Wed, 19 Apr 2023 23:16:23 -0700
From:   Christoph Hellwig <hch@...radead.org>
To:     Alexander Lobakin <aleksander.lobakin@...el.com>
Cc:     Christoph Hellwig <hch@...radead.org>,
        Jakub Kicinski <kuba@...nel.org>,
        Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, netdev@...r.kernel.org,
        Björn Töpel <bjorn@...nel.org>,
        Magnus Karlsson <magnus.karlsson@...el.com>,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
        Jonathan Lemon <jonathan.lemon@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        John Fastabend <john.fastabend@...il.com>, bpf@...r.kernel.org,
        virtualization@...ts.linux-foundation.org,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Gerd Hoffmann <kraxel@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jens Axboe <axboe@...nel.dk>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH net-next] xsk: introduce xsk_dma_ops

On Wed, Apr 19, 2023 at 03:14:48PM +0200, Alexander Lobakin wrote:
> >>> dma addresses and thus dma mappings are completely driver specific.
> >>> Upper layers have no business looking at them.
> 
> Here it's not an "upper layer". XSk core doesn't look at them or pass
> them between several drivers.

Same for upper layers :)  The just do abstract operations that can sit
on top of a variety of drivers.

> It maps DMA solely via the struct device
> passed from the driver and then just gets-sets addresses for this driver
> only. Just like Page Pool does for regular Rx buffers. This got moved to
> the XSk core to not repeat the same code pattern in each driver.

Which assumes that:

 a) a DMA mapping needs to be done at all
 b) it can be done using a struct device exposed to it
 c) that DMA mapping is actually at the same granularity that it
    operates on

all of which might not be true.

> >>From the original patchset I suspect it is dma mapping something very
> > long term and then maybe doing syncs on it as needed?
> 
> As I mentioned, XSk provides some handy wrappers to map DMA for drivers.
> Previously, XSk was supported by real hardware drivers only, but here
> the developer tries to add support to virtio-net. I suspect he needs to
> use DMA mapping functions different from which the regular driver use.

Yes,  For actual hardware virtio and some more complex virtualized
setups it works just like real hardware.  For legacy virtio there is 
no DMA maping involved at all.  Because of that all DMA mapping needs
to be done inside of virtio.

> So this is far from dma_map_ops, the author picked wrong name :D
> And correct, for XSk we map one big piece of memory only once and then
> reuse it for buffers, no inflight map/unmap on hotpath (only syncs when
> needed). So this mapping is longterm and is stored in XSk core structure
> assigned to the driver which this mapping was done for.
> I think Jakub thinks of something similar, but for the "regular" Rx/Tx,
> not only XDP sockets :)

FYI, dma_map_* is not intended for long term mappings, can lead
to starvation issues.  You need to use dma_alloc_* instead.  And
"you" in that case is as I said the driver, not an upper layer.
If it's just helper called by drivers and never from core code,
that's of course fine.

Powered by blists - more mailing lists