[Kos-dev] "Race condition" 1

[d2]

Hello,

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@enix.org> writes:
    Thomas> Dans le cas "normal", c'est le handler de l'IRQ timer qui
    Thomas> appelle reschedule_after_interrupt, qui retourne le
    Thomas> nouveau cpu_state au hanlder d'IRQ, qui se charge de
    Thomas> restaurer le contexte. Et la, au moment du IRET,
    Thomas> clairement les interruptions redeviennent actives, et on
    Thomas> ne passe pas du tout par cpl0_switch_with_return.

Attention quand meme a pas trop suspecter le IE lors des chgts de
contexte. En effet, le scenario suivant est VALIDE :
  - A tourne -> clac interruption
  - IT Handler: sauvegarde de l'etat de A (IE du eflags forcement a 1 !!)
  - IT Handler: election d'un nouveau thread B => A en fin de
    cpu_waitqueue
  - IT Handler: iret
  - B tourne.
  - B bloque sur 1 Semaphore
  - kwaitqueue_add: sauvegarde de l'etat de B (IE forcement a 0)
  - kwaitqueue_add: election du prochain, il se trouve que c'est A
  - cpl0_switch_with_return: restauration de A (rappel: IE sauvegarde
    == 1) => apres le iret, on est dans A, avec le IE passe a 1. Mais
    A n'est pas au niveau du get_there, puisque qu'il a ete
    interrompu forcement ailleurs que dans cpl0_switch_with_return (ou
    les IRQ sont desactivees), donc son contexte est un contexte de
    fonction qui ne correspond pas a la 2eme partie de
    cpl0_switch_with_return => pas de test if (thread_to_be_destroyed...)
  - on repart la ou A a ete interrompu (IE = 1 et c'est NORMAL)

Donc dans ce scenario valide, IE du nouveau thread passe a 1 et ca ne
doit pas etre considere comme un cas d'erreur.

Par contre, ce qui ne va pas c'est si on imagine le scenario suivant
(seule difference, cf '***') :
  - A tourne -> clac interruption
  - IT Handler: sauvegarde de l'etat de A (IE du eflags forcement a 1 !!)
  - IT Handler: election d'un nouveau thread B => A en fin de
    cpu_waitqueue
  - IT Handler: iret
  *** B se termine => thread_to_be_destroyed = B ***
  - kwaitqueue_add: sauvegarde de l'etat de B (IE forcement a 0)
  - kwaitqueue_add: election du prochain, il se trouve que c'est A
  - cpl0_switch_with_return: restauration de A (rappel: IE sauvegarde
    == 1) => apres le iret, on est dans A, avec le IE passe a 1. Mais
    A n'est pas au niveau du get_there, puisque qu'il a ete
    interrompu forcement ailleurs que dans cpl0_switch_with_return (ou
    les IRQ sont desactivees), donc son contexte est un contexte de
    fonction qui ne correspond pas a la 2eme partie de
    cpl0_switch_with_return  => pas de test if (thread_to_be_destroyed...)
  - on repart la ou A a ete interrompu (IE = 1 et c'est NORMAL)
  *** MAIS du coup on n'a pas teste thread_to_be_destroyed. ***

Si on continue juste 1 peu plus loin :
  - A tourne
  - A se termine => BOUM   ASSERT_FATAL(thread_to_be_destroyed ==
    NULL); EXPLOSE dans cpl0_switch_no_return puisque
    thread_to_be_destroyed pointe ENCORE sur B.

CQFD ! Conclusion, il faut appeler delete_kernel_thread avant le iret
dans les handlers d'interruption => dans
reschedule_after_interrupt(). On le fait deja a chq chgt de contexte
"manuel" (cpl0_switch_with_return), donc y'a pas besoin d'en rajouter
d'autres en dehors de celui dans reschedule_after_interrupt()..

A toi de jouer.

-- 
d2