On Sat, Feb 15, 2020 at 09:42:53PM +0100, Martin Pieuchot wrote: > On 15/02/20(Sat) 16:56, Visa Hankala wrote: > > When I added the knote_acquire() call in kqueue_register(), I overlooked > > the fact that the knote could be modified by a (soft) interrupt. > > Interrupts have to be blocked when changing kn_status. Otherwise the > > state could become confused. > > Can the same knote be modified by different interrupt handlers or are we > just considering a race between process and interrupt contexts?
In theory, the same knote could be modified by different interrupt handlers if the associated event source used more than one interrupt. The kernel lock already ensures that only one CPU at at time can access a knote. This diff fixes a situation where the interrupt context is not blocked properly. The particular case in my mind is timer events. However, the same issue applies to all situations where activation can happen from interrupt context. > > The diff below adds splhigh() where kn_status is modified. This also > > ensures that the steps of knote_activate() run as an uninterrupted > > sequence. As a consequence, knote_enqueue() and knote_dequeue() can rely > > on the caller to raise the spl. > > Do you think it would make sense to document which fields required a SPL > protection just like we started doing with locks? Maybe, though I am somewhat hesitant to start adding more formalism. With mutexes the SPL requirement is implied, though not necessarily fully accurate. For example, the same mutex can protect some fields that are accessed from process context only and some other fields that are accessed from both process and interrupt contexts. With tricky code, instead of SPL it might be good to indicate somehow if multiple contexts are involved though.
