On Mon, May 19, 2025 at 01:13:01PM +0200, Gabriele Monaco wrote: > > > On Mon, 2025-05-19 at 12:38 +0200, Nam Cao wrote: > > On Mon, May 19, 2025 at 12:28:12PM +0200, Gabriele Monaco wrote: > > > Mmh, although definitely unlikely, I'm thinking of a case in which > > > the > > > event starts on one CPU and at the same time we see events in IRQ > > > and > > > on another CPU, let's say continuously. Nothing forbids that > > > between > > > any two consecutive try_cmpxchg another CPU/context changes the > > > next > > > state (making the local try_cmpxchg fail). > > > In practice I've never seen it going on the second iteration, as > > > the > > > critical section is really tiny, but I'm not sure we can guarantee > > > this > > > never happens. > > > Or am I missing something? > > > > I have a feeling that you missed my point. I agree that the retrying > > is > > needed, because we may race with another. > > > > What I am proposing is that we drop the MAX_DA_RETRY_RACING_EVENTS, > > and > > just keep retrying until we succeed. > > > > And that's safe to do, because the maximum number of retries is the > > number > > of tasks contending with us to set the monitor's state. So we know we > > won't > > be retrying for long. > > I get this point, what I mean is: can we really guarantee the number of > contending tasks (or contexts) is finite? > In other words, the try_cmpxchg guarantees 1 and only 1 actor wins > every time, but cannot guarantee all actors will eventually win, an > actor /could/ be hanging there forever. > > This handler is running for each event in the monitor and tracepoint > handlers can be interrupted as well as run in interrupt context (where > of course they cannot be interrupted). I don't think the number of > actors is bounded by the number of CPUs. > I see this situation is extremely unlikely, but in an exotic scenario > where a CPU is sufficiently slower than others (e.g. in a VM) I believe > we can see this critical section large enough for this to potentially > happen. > > I'm not quite afraid of infinite loops, but rather RV introducing > unbounded latency very hard to track and without any reporting. > Chances are, since tracepoints and actual traced events are not atomic, > that by the time this delayed context /wins/ the RV event is no longer > current, so we may see an error already. > > Does it make sense to you or am I making it more complex than it should > be?
Right, I can see that being a problem. But I don't know enough about it to comment further, so do as you think best, maybe someone else can help. Best regards, Nam