On Thu, Jun 30, 2016 at 02:59:19PM +0200, Peter Zijlstra wrote:
> On Mon, Jun 20, 2016 at 01:55:11PM +0900, Byungchul Park wrote:
>
> > +struct bitlock_map {
> > + struct hlist_node hash_entry;
> > + unsigned long bitaddr; /* ID */
> > + struct lockdep_map map;
> > + int ref; /* reference count */
> > +};
>
> So this is effectively bigger than just adding a struct lockdep_map into
> whatever structure holds the bit spinlock to begin with.
>
> What is the gain?
1. I don't want to make being aware of lockdep essential to user of
bit-base lock, like spin lock, mutex, semaphore ans so on. In other
words, I want to make it work transparently.
2. Bit-base lock can be used with any data type which can be seperately,
not within a structure. I mean sometimes it can be ugly to pack the
lock bit and lockdep_map instance explicitly.
3. I think this is more general approach because _any_ random bit in
memory can be used as a lock. Do we need to restrict where the bit
is so that we can place lockdep_map explicitly around the bit?
>
>
> > +static inline unsigned long get_bitaddr(int bitnum, unsigned long *addr)
> > +{
> > + return (unsigned long)((char *)addr + bitnum);
> > +}
>
> And given you keep these lockdep_map thingies out-of-line, the original
> structure remains dense and thus the above munging can easily result in
> collisions.
I am sorry. I don't understand what you said exactly. IIUC, of course
it would be safer if lockdep_map is included in a structure statically.
However, as you know, sometimes dynamical allocating and connecting can
be more useful and worth under careful implementation. CONFIG_LOCKDEP
is even a debug feature where IMHO it's more important to implement
new value than to keep it very much safest, even though of course we
have to keep it as safe as possible.
>
> Now, I suppose its rather unlikely, but given its entirely silent if it
> happens, this is bad.
