On Wed, 2011-11-23 at 12:29 +0200, Sergey Kibrik wrote: > On 11/22/2011 11:21 AM, Tomi Valkeinen wrote: > > dss_cache struct contains a spinlock used to protect the struct. A more > > logical place for the spinlock is outside the struct that it is > > protecting. So move it there. > > a small question: isn't it clearer to keep lock inside struct, so it would be easier to read code? Say, if we meet > > > spin_lock_irqsave(&dss_cache.lock, flags); > > in code we already aware of what struct being actually protected, and in case of external lock it's not that obvious But if you meet code like: op = get_ovl_priv(ovl); You don't see that the data is inside the struct protected with the spinlock. So you still need to understand what it protects, and what the above function returns. But I see your point. I'm not sure which way is better. I thought it like this: the lock protects a struct, but if the lock is inside the struct, the lock would protect also itself. Which it doesn't. That said, I'm fine with both ways. It doesn't matter much. I didn't really look for any established patterns for this in the kernel code, but if everybody else have their locks inside the structs they protect, then obviously we should also. Tomi
Attachment:
signature.asc
Description: This is a digitally signed message part
