On Sun, May 04, 2025 at 07:10:24PM +0200, Lorenz (xha) wrote:
> hi, two questions, mostly curious though:
> 
> On Sun, May 04, 2025 at 05:20:41PM +1000, David Gwynne wrote:
> > PC_LOCK_INIT(9)            Kernel Developer's Manual           PC_LOCK_INIT(9)
> > 
> > NAME
> >      pc_lock_init, pc_cons_enter, pc_cons_leave, pc_sprod_enter,
> >      pc_sprod_leave, pc_mprod_enter, pc_mprod_leave, PC_LOCK_INITIALIZER -
> >      producer/consumer locks
> > 
> > SYNOPSIS
> >      #include <sys/pclock.h>
> > 
> >      void
> >      pc_lock_init(struct pc_lock *pcl);
> > 
> >      void
> >      pc_cons_enter(struct pc_lock *pcl, unsigned int *genp);
> 
> why not return gen as a return value?

it felt more consistent with pc_cons_leave.

> >      int
> >      pc_cons_leave(struct pc_lock *pcl, unsigned int *genp);
> > 
> >      unsigned int
> >      pc_sprod_enter(struct pc_lock *pcl);
> > 
> >      void
> >      pc_sprod_leave(struct pc_lock *pcl, unsigned int gen);
> > 
> >      unsigned int
> >      pc_mprod_enter(struct pc_lock *pcl);
> > 
> >      void
> >      pc_mprod_leave(struct pc_lock *pcl, unsigned int gen);
> > 
> >      PC_LOCK_INITIALIZER();
> 
> why do the callers have to store gen here? if i understand correctly,
> this number cannot change while odd (pcl_gen & 1) in both cases?

the prototype for this is in the per cpu counters api. storing the
gen counter meant it could avoid a reload of the value the after
the membars, so the code was a bit shorter.

the environments are a bit different between the counters and here
though. counters_read is all bare code while pc_cons_enter and leave
are function calls, so it's a bit like comparing apples and oranges.

having the gen counter maintained by the caller does give us a lot
more scope to improve diagnostics in the future though.