Re: [PATCH 1/5] lockdep: Implement bitlock map allocator

[Byungchul Park <byungchul.park@lge.com>]

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.