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] [day] [month] [year] [list]
Message-ID: <20121120163148.GB17534@redhat.com>
Date:	Tue, 20 Nov 2012 17:31:48 +0100
From:	Oleg Nesterov <oleg@...hat.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Anton Arapov <anton@...hat.com>, Ingo Molnar <mingo@...e.hu>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Michal Marek <mmarek@...e.cz>,
	Mikulas Patocka <mpatocka@...hat.com>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] percpu_rw_semaphore: add the lockdep annotations

On 11/19, Andrew Morton wrote:
>
> On Sun, 18 Nov 2012 20:03:21 +0100
> Oleg Nesterov <oleg@...hat.com> wrote:
>
> > -extern int percpu_init_rwsem(struct percpu_rw_semaphore *);
> > +extern int __percpu_init_rwsem(struct percpu_rw_semaphore *,
> > +				const char *, struct lock_class_key *);
> >  extern void percpu_free_rwsem(struct percpu_rw_semaphore *);
> >
> > +#define percpu_init_rwsem(brw)	\
>
> Should have been called percpu_rwsem_init(). The naming in this code does
> seem to be rather inconsistent.

Oh, I agree with any naming.

I guess Mikulas chose this name because the regular rw_semaphore has
init_rwsem(), not rwsem_init(), so this looks consistent.

> s/percpu_rw_semaphore/percpu_rwsem/g
> would be a good start, then consistently use percpu_rwsem_foo where
> practical.  But percpu_rwsem_down_read() doesn't sound practical :(

But personally I agree, percpu_rwsem_* looks better, and I will be
happy to send the trivial patch.

But can we do this later? This patch should touch the code in blockdev
and uprobes, and currently there is no single tree which this rename
can be based on.

> Is there much point in doing all these changes as five separate patches

four ;) .fix will be folded, I guess.

> (so far)?  Perhaps it should all blobbed into as little as one patch(es)?

Well, I'd prefer to keep this particular change as a separate patch,
and probably 3/3 too (just because I know nothing about kbuild/etc).

But 1/3 can be folded, I agree. And I won't argue if you ask me to
resend everything as one patch.

> You sent your uprobes changes to Ingo as a git pull, but I doubt if
> Ingo's trees contain the percpu_rwsem_rw_semaphore changes.

No, it doesn't. But, correctness-wise, percpu_rw_semaphore works fine
even without these changes, just it is "too slow" as it used in uprobes.
I hope that that fix and these changes will meet together in 3.8.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ