From: Chris Wilson
> Sent: 10 March 2020 11:50
>
> Quoting David Laight (2020-03-10 11:36:41)
> > From: Chris Wilson
> > > Sent: 10 March 2020 09:21
> > > Instruct the compiler to read the next element in the list iteration
> > > once, and that it is not allowed to reload the value from the stale
> > > element later. This is important as during the course of the safe
> > > iteration, the stale element may be poisoned (unbeknownst to the
> > > compiler).
> >
> > Eh?
> > I thought any function call will stop the compiler being allowed
> > to reload the value.
> > The 'safe' loop iterators are only 'safe' against called
> > code removing the current item from the list.
> >
> > > This helps prevent kcsan warnings over 'unsafe' conduct in releasing the
> > > list elements during list_for_each_entry_safe() and friends.
> >
> > Sounds like kcsan is buggy ????
>
> The warning kcsan gave made sense (a strange case where the emptying the
> list from inside the safe iterator would allow that list to be taken
> under a global mutex and have one extra request added to it. The
> list_for_each_entry_safe() should be ok in this scenario, so long as the
> next element is read before this element is dropped, and the compiler is
> instructed not to reload the element.
Normally the loop iteration code has to hold the mutex.
I guess it can be released inside the loop provided no other
code can ever delete entries.
> kcsan is a little more insistent on having that annotation :)
>
> In this instance I would say it was a false positive from kcsan, but I
> can see why it would complain and suspect that given a sufficiently
> aggressive compiler, we may be caught out by a late reload of the next
> element.
If you have:
for (; p; p = next) {
next = p->next;
external_function_call(void);
}
the compiler must assume that the function call
can change 'p->next' and read it before the call.
Is this a list with strange locking rules?
The only deletes are from within the loop.
Adds and deletes are locked.
The list traversal isn't locked.
I suspect kcsan bleats because it doesn't assume the compiler
will use a single instruction/memory operation to read p->next.
That is just stupid.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
