Hi Adam,

at the HelenOS camp last week, I managed to finish reading your changes.
I still plan to have a detailed clear look (e.g. not via an incremental
diff) at the rcu and cht algorithms before the evaluation deadline.

Tonight, I did some testing and unfortunately it appears the branch is
not that scrubbed or even tested as one would have hoped. These are the
issues I noticed:

- test cht panics the kernel when run on real amd64 or ia32 hardware
(4-core Intel i5, 1.4GHz, 4G memory)
- test cht panics the kernel when run on the above machine in QEMU (-smp
4 --enable-kvm)
- unfortunately the panic message is interleaved with some other
concurrent output so it is completely unreadable (some synchronization
should be probably added back to printf)
  - just noticed that when run in QEMU (without kvm, just 1 CPU), it
    crashes too due to a failed assertion at adt/cht.c:872:
    rcu_read_locked()

- make check reveals that only amd64 and ia32 targets build
- other targets do not build for various reasons:
  - spinlock_t * needs to be defined for UP builds too because it is
    now needed by condvars regardless of whether the build is UP or SMP
  - sparc64 is missing some proc/thread.h includes
  - non x86 platforms are missing atomic_cas_* implementations (the
    first step would be to make all platforms at least build using just
    dummy implementations, the next step would be to provide
    functional, but perhaps still limited implementations for these);
    for example, armv4 does not have atomic instructions, so all one
    can do is disable interrupts and then do a non-atomic cas

- I also find it a little suboptimal that you didn't provide any of the
Big Theory Statements at the beginning of RCU a CHT files as I
previously suggested

- finally, what worries me is that the cht obviously did not see any
systematic testing (as evidenced by the panics) and also it was not
tried for the global page hash table (sparc64 and ia64 do not build and
page_ht.c still uses the old hash table)

Jakub

_______________________________________________
HelenOS-devel mailing list
[email protected]
http://lists.modry.cz/cgi-bin/listinfo/helenos-devel

Reply via email to