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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160701002444.GV2279@X58A-UD3R>
Date:	Fri, 1 Jul 2016 09:24:44 +0900
From:	Byungchul Park <byungchul.park@....com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	mingo@...nel.org, linux-kernel@...r.kernel.org, npiggin@...e.de,
	sergey.senozhatsky@...il.com, gregkh@...uxfoundation.org,
	minchan@...nel.org
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.