On Sun, May 04, 2025 at 12:56:44PM -0400, Ted Unangst wrote:
> On 2025-05-04, David Gwynne wrote:
> 
> > CAVEATS
> >      Updates must be produced infrequently enough to allow time for consumers
> >      to be able to get a consistent read without looping too often.
> 
> This was my first thought. I'm worried we'll spin and tank performance, but
> won't have any visibility. Maybe there should be a count of failures?

globally?

i could extend the state carried between enter and leave to be a
struct that counts stuff like this, and enter ddb if it spends too
long waiting. kind of like the spinout stuff in mutexes and the
kernel locks.

> 
> > pc_sprod_leave() marks the end of a single producer critical section for
>      the pcl producer/consumer lock.  The gen argument must be the value
>      returned from the preceding pc_sprod_enter() call.
> 
> I don't think there's any need for the leave functions to take gen.
> Just increment the stored gen. Multiple consumers can't change it while
> it's odd.

we could avoid a load and maybe improve diagnostics?

> +	if (gen & 1) {
> +		do {
> +			CPU_BUSY_CYCLE();
> +			gen = pcl->pcl_gen;
> +		} while (gen & 1);
> +	} else if (gen == *genp)
> +		return (0);
> 
> This will be cleaner if you put the else first. Then make it a while loop.

yes, that make sense. i also like the idea of you committing the
improvement yourself and getting the blame^Wcredit for them.