Messages in this thread | | | Date | Fri, 1 Jul 2016 09:24:44 +0900 | From | Byungchul Park <> | Subject | Re: [PATCH 1/5] lockdep: Implement bitlock map allocator |
| |
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.
| |