Re: [PATCH 35/65] OMAPDSS: APPLY: move spinlock outside the struct

Tomi Valkeinen
2011-11-23T10:47:37Z

[-- Attachment #1: Type: text/plain, Size: 1276 bytes --]

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 #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]