Sorry, the code review of this patch took a bit more time as it is a quite long one and mixes several topics:
- coding style cleaning
- race condition
- struct changes and their respective code modifications
- moving a variable to proc.c
It would be very helpful if you split the patch into smaller parts before I submit them. Is that feasible ?
I will try. It will probably end up in three patches, cleanup, race fix, variable move. The struct changes are part of the race fix.
Nevertheless, I dug through and noticed you modified the struct vis_info by adding "struct vis_info_entry entries[130];". This would lead to an allocation of nearly 1KB (130 * 7 Bytes) for every incoming vis packet. Is that intended ?
Yes, this is intentional, although maybe i've missed something subtle in the code. The old code allocates one instance of vis_info in vis_init() by kmalloc()'ing 1000 bytes. In generate_vis_packet() there are checks to ensure we don't go over this 1000 byte limit. I don't like this 1000 magic number and decided to clean it up.
As you said, 130 * 7, is nearly 1K, so the overall memory usage should not of changed.
Now, looking deeper, there is something subtle i missed. When sending, in send_vis_packet(), it calculates the packet length based on the number of used entries and only sends that many bytes. So in receive_server_sync_packet() which calls add_packet() it would only kmalloc() as much space needed for the received packet. My code allocated the maximum packet size. So i'm wasting space here. I will rework this code.
I also applied your coding style patches for bitarray.[ch] and hash.[ch].
Thanks
Andrew