Re: [PATCH 3/5] fs: sys_ringbuffer

[Kent Overstreet <kent.overstreet@linux.dev>]

On Mon, Jun 24, 2024 at 01:16:15AM +0200, Thomas Gleixner wrote:
> Kent!
> 
> On Sun, Jun 23 2024 at 18:21, Kent Overstreet wrote:
> > On Mon, Jun 24, 2024 at 12:13:36AM +0200, Thomas Gleixner wrote:
> >> > +	/*
> >> > +	 * We use u32s because this type is shared between the kernel and
> >> > +	 * userspace - ulong/size_t won't work here, we might be 32bit userland
> >> > +	 * and 64 bit kernel, and u64 would be preferable (reduced probability
> >> > +	 * of ABA) but not all architectures can atomically read/write to a u64;
> >> > +	 * we need to avoid torn reads/writes.
> >> 
> >> union rbmagic {
> >> 	u64	__val64;
> >>         struct {
> >>                 // TOOTIRED: Add big/little endian voodoo
> >> 	        u32	__val32;
> >>                 u32	__unused;
> >>         };
> >> };
> >> 
> >> Plus a bunch of accessors which depend on BITS_PER_LONG, no?
> >
> > Not sure I follow?
> >
> > I know biendian machines exist, but I've never heard of both big and
> > little endian being used at the same time. Nor why we'd care about
> > BITS_PER_LONG? This just uses fixed size integer types.
> 
> Read your comment above. Ideally you want to use u64, right?
> 
> The problem is that you can't do this unconditionally because of 32-bit
> systems which do not support 64-bit atomics.
> 
> So a binary which is compiled for 32-bit might unconditionally want the
> 32-bit accessors. Ditto for 32-bit kernels.
> 
> The 64bit kernel where it runs on wants to utilize u64, right?
> 
> That's fortunately a unidirectional problem as 64-bit user space cannot
> run on a 32-bit kernel ever.

Ah! Yeah, that's slick.

Your code doesn't quite work though; with this scheme, we can't just
subtract the 64 bit head and tail to get the curretly used space in the
ringbuffer, that'll give the wrong answer if the other end is 32 bit.

But we just need to mask off the high 32 bits (after the subtract,
even), and we can still use the full 64 bits for ABA avoidance.

I think I like it...