On Tue, Jan 10, 2023 at 12:46:44PM +0100, Heiko Carstens wrote:
> > > + /* READ_ONCE() 16 byte header */
> > > + prev.val = __cdsg(&te->header.val, 0, 0);
> > > do {
> > > + old.val = prev.val;
> > > + new.val = prev.val;
> > > + *overflow = old.overflow;
I guess, it would also make sense to place write to overflow
after the while loop. So the output variable left intact in
case the function bailed out. Not sure if it should be part
of this patch though.
> > > + if (old.f) {
> > > /*
> > > * SDB is already set by hardware.
> > > * Abort and try to set somewhere
> > > @@ -1490,10 +1509,10 @@ static bool aux_set_alert(struct aux_buffer *aux, unsigned long alert_index,
> > > */
> > > return false;
> > > }
> > > + new.a = 1;
> > > + new.overflow = 0;
> > > + prev.val = __cdsg(&te->header.val, old.val, new.val);
> > > + } while (prev.val != old.val);
> >
> > And while this case has an early exit, it only cares about a single bit
> > (although you made it a full word) and so also shouldn't care. If
> > aux_reset_buffer() returns false, @overflow isn't consumed.
>
> Yes, except that it is anything but obvious that @overflow isn't consumed.
