[Kos-dev] "Race condition" 1

[Thomas Petazzoni]

This is an OpenPGP/MIME signed message (RFC 2440 and 3156)
--------------enig94DAE791D14A600DE99BE365
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 8bit

Hello,

> Si : ASSERT_FATAL(test_interrupt_enabled() == FALSE); dans
> cpl0_switch_with_return. Donc quand un thread demande un switch, avant
> le switch, IE est a 0, donc quand on retournera vers lui, dans tous
> les cas IE est remis a 0 au moment du iret. Le seul cas qui deconnait,
> c'est quand le iret ne restaurait pas un contexte de thread qui avait
> appele cpl0_switch_with_return, ie quand le iret restaurait le
> contexte d'un thread tout neuf, pour lequel IE vaut 1. Mettre IE a 0
> pour les nouveaux threads fait que dans tous les cas le iret restaure
> le contexte d'un thread avec interruptions desactivees. Il serait pas
> inutile d'ailleurs de mettre un test du type ASSERT_FATAL sur le IE a
> 0 du eflags du to_context dans cpl0_switch_no_return_internal et
> cpl0_switch_with_return .

Bon, ok, j'avais encore tort apparemment, j'ai toujours un peu de mal
avec ces changements de contexte. On peut effectivement essayer de
tester le IE du Eflags dans cpl0_switch_no_return_internal pour vérifier
qu'on fait pas n'importe quoi, mais appeler ASSERT_FATAL depuis du code
assembleur, hum ;-)

Je serais pour recopier le ASSERT_FATAL(test_interrupt_enabled() ==
FALSE); juste en dessous de l'appel à cpl0_switch_with_return_internal
pour être sur qu'on a effectivement toujours les interruptions
désactivées après le changement de contexte.

Je retiens donc ta solution : au lieu d'initialiser Eflags à 514 lors de
la création du thread on l'initialise à 2 (== (514 & ~(1<<9)), et on
fait ensuite comme tu as dit dans execute_thread.

>     Thomas> En ce qui concerne le passage du spinlock en paramètre, il
>     Thomas> faudra passer un pointeur, et donc toutes les macros
>     Thomas> *_spin_* ne fonctionneront pas.
> 
> ?


#define write_spin_lock(spinlock,flags) \
        do { save_flags(flags); cli(); \
             LOCKING_REPORT("write_lock_sl(" # spinlock " : "); \
             ASSERT_FATAL((spinlock).state == LCK_READY); \
             ASSERT_FATAL((spinlock).nb_readers == 0); \
             ASSERT_FATAL((spinlock).nb_writers == 0); \
             (spinlock).nb_writers = 1; \
             LOCKING_REPORT("read=%d,writ=%d)\n", (spinlock).nb_readers,
(spinlock).nb_writers); \
           } while(0)

Cette définition suppose qu'on a accès *directement* à la structure
spinlock_t, et non pas à un (spinlock_t *). Or, si on veut que
kwaitqueue_add_* relache un spinlock au bon moment, il faut passer un
pointeur dessus.

Thomas
-- 
PETAZZONI Thomas - thomas DOT petazzoni AT enix DOT org - UIN : 34937744
Web: http://www.enix.org/~thomas/
KOS: http://kos.enix.org/ - Lolut: http://lolut.utbm.info
Fingerprint : 0BE1 4CF3 CEA4 AC9D CC6E  1624 F653 CB30 98D3 F7A7

--------------enig94DAE791D14A600DE99BE365
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQE+0dIx9lPLMJjT96cRApjDAJwKg3qgGz0Oy7/XmSk7emw77COMNACeKpvB
XBZM71dU2LuoY7M5RZJAPH0=
=jByx
-----END PGP SIGNATURE-----

--------------enig94DAE791D14A600DE99BE365--