Re: [PATCH 2/3] percpu_rw_semaphore: add the lockdep annotations

[Oleg Nesterov <oleg@redhat.com>]

On 11/19, Andrew Morton wrote:
>
> On Sun, 18 Nov 2012 20:03:21 +0100
> Oleg Nesterov <oleg@redhat.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.