On Sun, May 04, 2025 at 02:08:51PM +0200, Sebastien Marie wrote:
> Mostly a comment about consumer API documentation.
> 
> David Gwynne <david@gwynne.id.au> writes:
> 
> > SYNOPSIS
> 
> >      void
> >      pc_cons_enter(struct pc_lock *pcl, unsigned int *genp);
> >
> >      int
> >      pc_cons_leave(struct pc_lock *pcl, unsigned int *genp);
> 
> > DESCRIPTION
> 
> >    Consumer API
> >      pc_cons_enter() reads the current generation number from pcl and stores
> >      it in the memory provided by the caller via genp.
> >
> >      pc_cons_leave() compares the generation number in pcl with the value
> >      stored in genp by pc_cons_enter() at the start of the critical section,
> >      and returns whether the reads within the critical section need to be
> >      retried because the data has been updated by the producer.
> >
> 
> > EXAMPLES
> 
> >      A consistent read of the data from a consumer:
> >
> >            void
> >            consumer(void)
> >            {
> >                    unsigned int gen;
> >
> >                    pc_cons_enter(&pc, &gen);
> >                    do {
> >                            /* read data */
> >                    } while (pc_cons_leave(&pc, &gen) != 0);
> >            }
> 
> From my reading of pc_cons_leave() documentation, I was surprised by the
> example. At first, I would expected pc_cons_enter() call to be inside
> the do {} while. But after reading the code, pc_cons_leave() is updating
> gen content with the current generation number in pcl. Maybe it could be
> mentioned in pc_cons_leave() description ?

pc_cons_enter doesnt change the pc_lock state, so you could do it at the
top of the loop instead of before the start of the loop. leave already
knows the value though, so it made sense to keep it and avoid the
function enter function call again.

i'll have a think about how to describe it better.

> 
> Regards.
> -- 
> Sebastien Marie