Messages in this thread | | | Date | Wed, 11 Jul 2018 06:49:46 +0100 | From | Mark Rutland <> | Subject | Re: [PATCH] refcount: always allow checked forms |
| |
On Wed, Jul 04, 2018 at 10:46:41AM +0200, David Sterba wrote: > On Tue, Jul 03, 2018 at 11:01:02AM +0100, Mark Rutland wrote: > > In many cases, it would be useful to be able to use the full > > sanity-checked refcount helpers regardless of CONFIG_REFCOUNT_FULL, as > > this would help to avoid duplicate warnings where callers try to > > sanity-check refcount manipulation. > > > > This patch refactors things such that the full refcount helpers were > > always built, as refcount_${op}_checked(), such that they can be used > > regardless of CONFIG_REFCOUNT_FULL. This will allow code which *always* > > wants a checked refcount to opt-in, avoiding the need to duplicate the > > logic for warnings. > > > > There should be no functional change as a result of this patch. > > > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > > Cc: Boqun Feng <boqun.feng@gmail.com> > > Cc: David Sterba <dsterba@suse.com> > > Cc: Ingo Molnar <mingo@kernel.org> > > Cc: Kees Cook <keescook@chromium.org> > > Cc: Peter Zijlstra <peterz@infradead.org> > > Cc: Peter Zijlstra <peterz@infradead.org> > > Cc: Will Deacon <will.deacon@arm.com> > > I dare to give it my > > Reviewed-by: David Sterba <dsterba@suse.com>
Cheers!
> as my POC implementations were crap and Mark's version is much better.
Please don't think that your implementations were bad; I just already had an idea as to what this could look like.
> > --- > > include/linux/refcount.h | 27 +++++++++++++++++------- > > lib/refcount.c | 53 +++++++++++++++++++++++------------------------- > > 2 files changed, 45 insertions(+), 35 deletions(-) > > > > Dave pointed out that it would be useful to be able to opt-in to full checks > > regardless of CONFIG_REFCOUNT_FULL, so that we can simplify callsites where we > > always want checks. I've spotted a few of these in code which is still awaiting > > conversion. > > The motivation was code like > > WARN_ON(refcount_read(&ref)); > if (refcount_dec_and_test(&ref)) { ... } > > so the warning is redundant for REFCOUNT_FULL, but I'm going to use the > _checked versions everywhere the performance of refcounts is not > critical.
If you will have conversion patches, do you want to pick this up as the start of a series?
Thanks, Mark.
| |