On Tue, Oct 15, 2024 at 3:44 PM Jason Gunthorpe <jgg@xxxxxxxx> wrote: > > On Tue, Oct 15, 2024 at 04:10:44AM +0300, Mina Almasry wrote: > > On Tue, Oct 15, 2024 at 3:16 AM Jakub Kicinski <kuba@xxxxxxxxxx> wrote: > > > > > > On Tue, 15 Oct 2024 01:38:20 +0300 Mina Almasry wrote: > > > > Thanks Jason. In that case I agree with Jakub we should take in his change here: > > > > > > > > https://lore.kernel.org/netdev/20241009170102.1980ed1d@xxxxxxxxxx/ > > > > > > > > With this change the driver would delegate dma_sync_for_device to the > > > > page_pool, and the page_pool will skip it altogether for the dma-buf > > > > memory provider. > > > > > > And we need a wrapper for a sync for CPU which will skip if the page > > > comes from an unreadable pool? > > > > This is where it gets a bit tricky, no? > > > > Our production code does a dma_sync_for_cpu but no > > dma_sync_for_device. That has been working reliably for us with GPU > > Those functions are all NOP on systems you are testing on. > OK, thanks. This is what I wanted to confirm. If you already know this here then there is no need to wait for me to confirm. > The question is what is correct to do on systems where it is not a > NOP, and none of this is really right, as I explained.. > > > But if you or Jason think that enforcing the 'no dma_buf_sync_for_cpu' > > now is critical, no problem. We can also provide this patch, and seek > > to revert it or fix it up properly later in the event it turns out it > > causes issues. > > What is important is you organize things going forward to be able to > do this properly, which means the required sync type is dependent on > the actual page being synced and you will eventually somehow learn > which is required from the dmabuf. > > Most likely nobody will ever run this code on system where dma_sync is > not a NOP, but we should still use the DMA API properly and things > should make architectural sense. > Makes sense. OK, we can do what Jakub suggested in the thread earlier. I.e. likely some wrapper which skips the dma_sync_for_cpu if the netmem is unreadable. -- Thanks, Mina
