[Kos-dev] Remarques sur le commit

[Thomas Petazzoni]

d2 <David.Decotigny@irisa.fr> writes:

> "Et pourquoi ?"... Bonne question. En effet, en apparence, qd je suis
> dans vmap (plus precisement : dans rmap), les flags du gpfme, je m'en
> sers pas (je ne les regarde jamais), donc on pourrait penser qu'il est
> pas necessaire d'interdire leur modification par pmm qd je possede le
> gpfme->lock. Erreur : rmap touche a gpfme->ref_cnt, OR put_phys_page()
> regarde ce gpfme->ref_cnt avant de liberer une page. Et donc on doit
> interdire toute action de rmap quand on est dans put_phys_page() entre
> le moment ou on regarde le gpfme->ref_cnt, et le moment ou le gpfme va
> etre transfere dans la liste des free ! Il faut donc locker ce gpfme
> pdt toute la periode de transfert d'une liste a l'autre.

Ok, je comprends bien maintenant pourquoi on doit locker les gpfme au
niveau de pmm, j'avais pense aux flags et aux listes, mais pas au
ref_cnt effectivement.

> Au debut, j'etais parti pour l'ordre inverse : gpfm_lists PUIS
> gpfme->lock. Puis finalement j'ai chosi l'ordre indiqu=E9. C'est parce
> qu'au debut, je voulais 1 lock par liste gpfm (voir plus loin). Donc
> j'ai change l'ordre de locking. La-dessus sont arrivees mes reflexions
> menant a la liste garbage, qui ont fait disparaitre ce 1 lock par
> liste a une etape donnee de ces reflexions. Apres reflexions, on
> pourrait re-implanter ce "1 lock par liste" (voir plus loin).

J'avoue n'avoir pas bien compris le role de la garbage liste. J'ai
bien lu le commentaire "[GARBAGE Note] We cannot free the gpfme
because we cannot tell whether another thread is still using it =3D>
invalidate it and move it into the garbage list.", mais je ne vois pas
bien le truc.

> En effet, dans get_physical_page(), on doit d'abord locker la liste
> des free (-> lock gpfm_lists), pour "voir sa tete". Or, puisqu'on ne
> peut pas locker le gpfme en tete dans la mesure ou on a deja le lock
> gpfm_lists, on est oblige de relacher le lock gpfm_lists. Maintenant
> seulement, on peut prendre gpfme->lock. Mais entre temps, gpfme peut
> avoir ete change de liste par un autre thread. Donc il faut verifier a
> ce niveau que le gpfme est tjs free. Si oui, on peut locker les listes
> et faire le transfert vers la liste destination. Sinon, faut
> recommencer a regarder la nouvelle tete de la liste free (normalement
> differente [sauf cas extremes, mais qui ne posent pas de pb], vu qu'un
> autre thread a reussi a changer le type de l'ancienne tete a autre
> chose que FREE, c'est donc aussi qu'il a reussi a transferer
> l'ancienne tete vers une autre liste). D'ou le do { ... } while(TRUE);
> (avec un break au milieu).

Pour cette fonction get_physical_page() effectivement, une astuce
comme celle mise en oeuvre est necessaire. Chapeau d'ailleurs !

>     Thomas> - le change_gpfme_swap_status me gene. effectivement on
>     Thomas> veut pouvoir locker/delocker une page physique (mlock,
>     Thomas> munlock), mais il faudrait aussi avoir un mecanisme pour
>     Thomas> proteger les pages du noyau. Certains seront swappables
>     Thomas> (et donc on pourra changer l'etat de swappabilisation
>     Thomas> entre swappable et non swappables) et d'autres seront non
>     Thomas> swappables a tout jamais. Avec cette fonction j'ai
>     Thomas> l'impression que meme si on veut que la page ne soit
>     Thomas> jamais swappable, y'a toujours moyen de la rendre
>     Thomas> swappable... (mais peut etre je me trompe).
>=20
> Oui, c'est exact. Il suffirait de rajouter un :
>   RETURN_VAL_IF_FAILED((gpfme->frags.page_type =3D=3D PHYS_PAGE_KERNE=
L)
>                        && (gpfme->frags.swap_status =3D=3D PHYS_PAGE_=
NON_SWAPPABLE),
>                        -1);
> Au debut de la fonction.

Euh, je vois pas bien en quoi ceci permet de resoudre le probleme. Si
tu fais un tel RETURN_VAL_IF_FAIL, ca va sortir si on a page_type !=3D
PHYS_PAGE_KERNEL || swap_status !=3D PHYS_PAGE_NON_SWAPPABLE. Or je
pense qu'il y a plusieurs types de pages :
 - les pages noyau NON swappables POUR TOUJOURS
 - les pages noyau NON swappable POUR le moment
 - les pages noyau swappable POUR le moment
 - les pages user (a priori forcement swappable)
 - les pages hw, non swappables (forcement)

Je n'arrive pas a retrouver tous ces types de pages en faisant des
combinaisons de tes flags, pourrais-tu preciser un peu ?

> Oui, je vois bien le pb. Je vois une autre "solution" : virer l'alloc
> du gpfm par le loader. Il ne sert a rien du tout dans le loader, non
> ?

En voila une idee qu'elle est bonne. Effectivement, allouer le gpfm ne
sert a rien dans le loader, et nous embete a cause de
sizeof(spinlock_t) (truc crado a synchroniser a chaque fois).

> Je bosse sur rien jusqu'a Vendredi ou Samedi (et c'est meme pas sur
> que je fasse qqch ce WE). Je veux pas me reserver cette partie locking
> des pmm. Si t'as des idees, des envies de tenter l'ordre gpfm_lists
> puis gpfme->lock, etc... y'a pas de pb. Ce WE (ou le suivant), je
> trouverai bien des trucs qui restent a faire ;) Dans mes projets
> court terme (ce WE ou le suivant) : 1 lock par liste gpfm, arch_remap,
> virer l'alloc gpfm du loader (?), create_kernel_rmap(), virer les
> sections .init. Aucune exclusivite la-dessus entre-temps. Que ca se
> sache ;) Et apres... babel (enfin) !

Bin pour l'instant je poursuis sur la terminaison d'un thread CPL3, et
je sais pas quand j'aurai fini, parce que ca a l'air
gratine. Heuresement bochs est mon ami :o)

Je vois pas bien a quoi servent new_additionnal_allocatable_page et
remove_additional_allocatable_page. Le nom indique que c'est pour
ajouter (supprimer) de la memoire allouable, mais je vois pas pourquoi
on pourrait ajouter comme ca de la memoire allouable, et surtout a
quoi ca servirait. Surement un truc qui m'echappe, mais je ne vois pas
!

Bonne journee

Thomas
--=20
PETAZZONI Thomas - thomas.petazzoni@enix.org - UIN : 34937744
(Perso)      http://www.enix.org/~thomas/
(KOS)        http://kos.enix.org/=20
(Club LinUT) http://club-linut.enix.org