On 06-06-10 13:39 Daniel Drake wrote: > Ulrich Kunitz wrote: > > The other question is, whether the cleaning of structures is > > needed. I don't think they are performance issues, give the time > > this functions are called. OTOH we will find dangling pointer > > issues. So I still believe it is worth to keep them. > > I'm still not sure what the point of it is though - what is gained from > clearing them? > > We might as well sleep 10 milliseconds, allocate some memory and then > free it right after, or do something equally pointless. Am I missing > something? > > Also, there is no need for locking - and if there is, it is hiding any > races we have in the disconnect path.
Daniel, the clearing is definitely not in the performance-critical path and is definitely faster then 10 milliseconds. On most architectures is not even a function call. What I want to prevent is the usage of structures, after they have cleared. A cleaned structure will result in zero pointer exceptions and other errors, which are easy to understand. It is much more difficult to detect those errors with a fully initialized structure, which is just deallocated. Most malloc-debuggers support poisoning and zeroing a structure is a kind of poisoning and there is not a lot of penalty we pay for it. Regarding locking: Locks protect the access to a resource. We have only introduced two locks in our code: the zd_mac spinlock and the zd_chip mutex. The zd_mac-spinlock simply ensures consistent access to the data in the zd_mac structure with the exception of the zd_chip structure. The zd_chip mutex protects access to the chip itself and the data stored in zd_chip. Theoretical a race can always happen, if there is a possibility that interrupts, tasklets or tasks accesses the same memory. What you are doing, is to make a number of assumptions about code paths in which a function may be called. But these code paths might change, the assumptions are most of the time not explicitly expressed in the code itself and a minor change can result in huge surprises. I would even offer the opinion, that removal of locks based on the knowledge of a single person, even if he is as knowledgable as Herbert Xu, is not something I would call good engineering. I would agree with those drastic measures, if there would be data, which would support that extensive locking is an issue, but I haven't seen such data. Regarding the locks in zd_mac_clean(), yes I would agree there is no point in having those locks, if the data would not be cleared. But if we clear the structure, than we should protect it and don't make any assumptions about the code path. Cheers, Uli -- Uli Kunitz _______________________________________________ Zd1211-devs mailing list - http://zd1211.ath.cx/ Unsubscribe: https://lists.sourceforge.net/lists/listinfo/zd1211-devs
