On Tue, 19 May 2015, Peter Zijlstra wrote:
> On Mon, May 18, 2015 at 07:48:02PM +0200, Nicholas Mc Guire wrote:
> > static code checking with coccinelle was unhappy with:
> > ./kernel/locking/percpu-rwsem.c:113 WARNING: return of wrong type
> > int != unsigned int
> >
> >
> > current state:
> >
> > include/linux/percpu-rwsem.h:
> > struct percpu_rw_semaphore {
> > unsigned int __percpu *fast_read_ctr;
> > ...
> > atomic_t slow_read_ctr;
> >
> > allocated as int:
> >
> > int __percpu_init_rwsem(struct percpu_rw_semaphore *brw,
> > const char *name, struct lock_class_key *rwsem_key)
> > {
> > brw->fast_read_ctr = alloc_percpu(int);
> >
> > used compliant with type int in:
> >
> > static bool update_fast_ctr(struct percpu_rw_semaphore *brw,
> > unsigned int val)
> > usage (2):
> > update_fast_ctr(brw, +1)
> > update_fast_ctr(brw, -1)
> >
> > so val should be int here not unsigned int
> >
> > static int clear_fast_ctr(struct percpu_rw_semaphore *brw)
> > usage (1):
> > atomic_add(clear_fast_ctr(brw), &brw->slow_read_ctr);
> >
> > slow_read_ctr is atomic_t which is int so it would be consistent here to
> > have fast_read_ctr being changed to int as well.
> >
> > This patch changes:
> > fast_read_ctr to int
> > clear_fast_ctr():sum to int
> > update_fast_ctr():val to int
> > to make usage of fast_read_ctr consistent with respect to type.
> >
> > ( Patch was build tested with x86_64_defconfig +
> > CONFIG_UPROBE_EVENT (implies CONFIG_PERCPU_RWSEM=y))
> >
> > Signed-off-by: Nicholas Mc Guire <hof...@osadl.org>
>
> So the value is unsigned by purpose; that said we should never cross the
> 2G I think, so it really doesn't matter much.
I assumed it would not matter but did not see a simple way of getting it
type clean with unsigned either mainly due to the atomic_t being int and
val in update_fast_ctr() being passed as -1.
thanks for the review comments.
thx!
hofrat
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
