Thread (17 messages) 17 messages, 5 authors, 2025-07-18

Re: [PATCH v3 6/6] IB/mlx5: Use __iowrite64_copy() for write combining stores

From: Jason Gunthorpe <jgg@nvidia.com>
Date: 2025-07-18 20:03:46
Also in: linux-arch, linux-arm-kernel, linux-patches, linux-rdma, linux-s390, llvm

On Fri, Jul 18, 2025 at 07:10:06PM +0100, Catalin Marinas wrote:
On Tue, Jul 15, 2025 at 08:52:00AM -0300, Jason Gunthorpe wrote:
quoted
On Tue, Jul 15, 2025 at 11:15:25AM +0100, Will Deacon wrote:
quoted
quoted
Since STP was rejected alread we've only tested the Neon version. It
does make a huge improvement, but it still somehow fails to combine
rarely sometimes. The CPU is really bad at this :(
I think the thread was from last year so I've forgotten most of the
details, but wasn't STP rejected because it wasn't virtualisable? 
Yes, that was the claim.
quoted
In which case, doesn't NEON suffer from exactly the same (or possibly
worse) problem?
In general yes, in specific no.
For a generic iowrite function, I wouldn't use STP or Neon since it may
end up being used on emulated MMIO.
Yes, my feeling too
 
BTW, for Neon, don't you need kernel_neon_begin/end()? This may have its
own overhead and also BUG_ON for different contexts. Again, not suitable
for a generic function.
Yes, exactly right.
 
I can't think of any generic solution here, it may have to be a hack
specific to mlx5. We can also add add support for ST64B and have some
condition on system_supports_st64b() for future systems.
Ok, we will send the hack
We can also add add support for ST64B and have some
condition on system_supports_st64b() for future systems.
I have asked someone to work on the ST64B version so we can talk about
that then..
Even if we could handle virtualisation, I wonder whether
__iowrite64_copy() is the right function to implement 128-bit stores or
the larger 64-byte atomic stores. At least the comment for the generic
function suggests that it writes in 64-bit quantities. Some MMIO may
only handle such writes. A function like memcpy_toio() is more generic,
it doesn't imply any restrictions on the size of the writes (though I
think it guarantees natural alignment for the stores).
IMHO the '64' here refers to the alignment and multiple, not
necessarily the transfer granule size. This could be clarified in the
kdocs. Also, IIRC, there were few users and they were all doing WC
stores which give large TLPs on the PCIe - meaning there is no size
restriction issue.

Last time around we didn't use memcpy_toio() because it has not
requirement on alignment, requiring more checks and things which
didn't see desirable. To use ST64B under iowrite64 we only have to
check if the destination is 64 byte aligned.

Jason
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help