On 07/30/2012 09:50 AM, Adam Hraska wrote: > On Fri, Jul 27, 2012 at 5:25 PM, Jakub Jermar <[email protected]> wrote: >> Hi Adam, >> >> I plan to have a thorough look at your recent changes during my back >> flight from Black Hat; at this point I have just a couple of comments, >> see below. > > Great :-).
I managed to browse through everything up to and including your branche's revision 1568. Here are the comments: - member_to_inst() can be probably used to implement list_get_instance() too - in various places you use 0 instead of NULL for pointers, which looks weird - for the rcu functionality and the cht, it would be nice to have some Big Theory Statement at the beginning of the file that briefly describes the current chosen algo; this little helper will make it easier to understand what's going on in the individual functions; here is an example Big Theory Statement from OpenSolaris: http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/common/os/rwlock.c#38 >> Is networking still working with one of the nic drivers?? > > How would I go about testing that? ... > http://trac.helenos.org/wiki/UsersGuide/Networking Well, this article actually describes how to setup your networking in HelenOS and links to the other which says how you need to start QEMU. If you spot some inconsistencies, please report or fix directly. > Yeah, I have noticed this feature. IIRC it has been used > in 2 or 3 places out of the total of ~25 hash tables > in uspace. Actually I was thinking of replacing the special > purpose hash_table_remove(partial-match) with the more > general hash_table_apply() (it already has some users) > since there are only few users of the partial matching > remove(). Moreover, the partial matching remove() cannot > be implemented efficiently, so using hash_table_apply() > instead might make that fact explicit -- that the whole > table must be traversed. I kind of like this suggestion. +1 if you decide to implement this change. > I already confessed that I have no idea how to test > the NIC drivers. Now is the right time to admit that > I have tried to mount a FAT fs and failed. I mimicked: > http://trac.helenos.org/wiki/CoreFiles > in order to create a FAT volume and add it to qemu. > Somehow, helenos did not recognize the new hdd and > I did not figure out how to mount it (ie what to supply > to mount). Moreover, ls /loc/devices and ls /lod/bd > had the same output as when running in qemu without > the hdd attached (ie without the -hda qemu switch). You first need to manually run the ata_bd driver. If you have previously started QEMU with -hda disk.img, there should be a new service named something like bd/ata1disk0. Assuming the fat server is already running, you then just do: # mount fat /data bd/ata1disk0 Of course, there needs to be a FAT file system on the image. If there is not, you can create one using mkfat. Be careful about the arguments though, as this app is known to be somewhat picky. > Sometimes it is a bit difficult for a novice such as > myself to decipher what exactly is <dev> and <moptions> > in, I don't know ;-), say: > / # mount --help > 'mount' mounts a file system. > Usage: mount <fstype> <mp> [dev] [<moptions>] Yeah, I agree. This is especially hard for the first time. Once you figure it out, it doesn't seem that cryptic. Obviously :-) > Thanks for taking the time to look at the changes. Actually, it's taking me longer than I initially expected. But I will try to make my way through the changes. Also, this incremental style of review is not very optimal. If I have another look on the entire thing just before we merge this, I won't see some of the transient stuff. But at least I can see the entire evolution of it. Jakub _______________________________________________ HelenOS-devel mailing list [email protected] http://lists.modry.cz/cgi-bin/listinfo/helenos-devel
