Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1521929 > unrolled thread
| Started by | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| First post | 2016-11-14 18:50 +0100 |
| Last post | 2016-11-17 20:40 +0100 |
| Articles | 20 on this page of 89 — 12 participants |
Back to article view | Back to linux.kernel
[RFC][PATCH 0/7] kref improvements Peter Zijlstra <peterz@infradead.org> - 2016-11-14 18:50 +0100
[RFC][PATCH 5/7] kref: Implement kref_put_lock() Peter Zijlstra <peterz@infradead.org> - 2016-11-14 18:50 +0100
Re: [RFC][PATCH 5/7] kref: Implement kref_put_lock() Kees Cook <keescook@chromium.org> - 2016-11-14 21:40 +0100
Re: [RFC][PATCH 5/7] kref: Implement kref_put_lock() Peter Zijlstra <peterz@infradead.org> - 2016-11-15 09:00 +0100
[RFC][PATCH 6/7] kref: Avoid more abuse Peter Zijlstra <peterz@infradead.org> - 2016-11-14 18:50 +0100
[RFC][PATCH 4/7] kref: Use kref_get_unless_zero() more Peter Zijlstra <peterz@infradead.org> - 2016-11-14 18:50 +0100
[RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-14 18:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Ingo Molnar <mingo@kernel.org> - 2016-11-15 09:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-15 10:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Ingo Molnar <mingo@kernel.org> - 2016-11-15 11:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-15 11:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Ingo Molnar <mingo@kernel.org> - 2016-11-15 14:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Kees Cook <keescook@chromium.org> - 2016-11-15 19:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-15 20:20 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Kees Cook <keescook@chromium.org> - 2016-11-15 20:30 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Ingo Molnar <mingo@kernel.org> - 2016-11-16 09:40 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Greg KH <gregkh@linuxfoundation.org> - 2016-11-16 10:00 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Ingo Molnar <mingo@kernel.org> - 2016-11-16 10:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Greg KH <gregkh@linuxfoundation.org> - 2016-11-16 10:30 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-16 11:20 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Kees Cook <keescook@chromium.org> - 2016-11-16 20:00 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-17 09:40 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Kees Cook <keescook@chromium.org> - 2016-11-17 21:00 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Kees Cook <keescook@chromium.org> - 2016-11-16 19:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Boqun Feng <boqun.feng@gmail.com> - 2016-11-15 13:40 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-15 14:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Boqun Feng <boqun.feng@gmail.com> - 2016-11-15 15:20 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-17 10:30 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Boqun Feng <boqun.feng@gmail.com> - 2016-11-17 10:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-17 11:40 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-17 11:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Greg KH <gregkh@linuxfoundation.org> - 2016-11-17 12:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-17 18:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-17 18:30 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Will Deacon <will.deacon@arm.com> - 2016-11-17 18:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Boqun Feng <boqun.feng@gmail.com> - 2016-11-18 09:30 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Will Deacon <will.deacon@arm.com> - 2016-11-18 11:20 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Will Deacon <will.deacon@arm.com> - 2016-11-17 18:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-17 18:30 +0100
RE: [RFC][PATCH 7/7] kref: Implement using refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2016-11-18 11:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-18 12:40 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Will Deacon <will.deacon@arm.com> - 2016-11-18 18:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-18 20:00 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Boqun Feng <boqun.feng@gmail.com> - 2016-11-21 05:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Ingo Molnar <mingo@kernel.org> - 2016-11-21 08:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Boqun Feng <boqun.feng@gmail.com> - 2016-11-21 09:40 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Boqun Feng <boqun.feng@gmail.com> - 2016-11-21 09:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-21 10:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Boqun Feng <boqun.feng@gmail.com> - 2016-11-21 10:40 +0100
RE: [RFC][PATCH 7/7] kref: Implement using refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2016-11-18 11:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-18 12:00 +0100
RE: [RFC][PATCH 7/7] kref: Implement using refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2016-11-18 18:00 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-18 20:00 +0100
RE: [RFC][PATCH 7/7] kref: Implement using refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2016-11-19 08:20 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-19 12:50 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Christoph Hellwig <hch@infradead.org> - 2016-11-14 19:20 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Greg KH <gregkh@linuxfoundation.org> - 2016-11-15 08:30 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-15 08:50 +0100
[PATCH] printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref' Ingo Molnar <mingo@kernel.org> - 2016-11-15 09:40 +0100
[PATCH v2] printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref' Ingo Molnar <mingo@kernel.org> - 2016-11-15 09:50 +0100
Re: [PATCH v2] printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref' Peter Zijlstra <peterz@infradead.org> - 2016-11-15 10:30 +0100
[PATCH v3] printk, locking/atomics, kref: Introduce new %pAa and %pAk format string options for atomic_t and 'struct kref' Ingo Molnar <mingo@kernel.org> - 2016-11-15 10:50 +0100
Re: [PATCH v2] printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref' kbuild test robot <lkp@intel.com> - 2016-11-15 11:10 +0100
Re: [PATCH] printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref' Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-15 17:50 +0100
Re: [PATCH] printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref' Ingo Molnar <mingo@kernel.org> - 2016-11-16 09:20 +0100
Re: [RFC][PATCH 0/7] kref improvements Greg KH <gregkh@linuxfoundation.org> - 2016-11-15 08:30 +0100
Re: [RFC][PATCH 0/7] kref improvements Ingo Molnar <mingo@kernel.org> - 2016-11-15 08:50 +0100
Re: [RFC][PATCH 0/7] kref improvements Greg KH <gregkh@linuxfoundation.org> - 2016-11-15 16:10 +0100
Re: [RFC][PATCH 0/7] kref improvements Peter Zijlstra <peterz@infradead.org> - 2016-11-15 08:50 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Greg KH <gregkh@linuxfoundation.org> - 2016-11-15 08:40 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-15 09:10 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Kees Cook <keescook@chromium.org> - 2016-11-15 22:00 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Greg KH <gregkh@linuxfoundation.org> - 2016-11-16 09:30 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-16 11:20 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Greg KH <gregkh@linuxfoundation.org> - 2016-11-16 11:20 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Greg KH <gregkh@linuxfoundation.org> - 2016-11-16 11:20 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Daniel Borkmann <daniel@iogearbox.net> - 2016-11-16 11:20 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-16 11:10 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Kees Cook <keescook@chromium.org> - 2016-11-16 20:00 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-17 09:40 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() David Windsor <dave@progbits.org> - 2016-11-17 13:50 +0100
RE: [RFC][PATCH 2/7] kref: Add kref_read() "Reshetova, Elena" <elena.reshetova@intel.com> - 2016-11-17 15:40 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-17 18:10 +0100
RE: [RFC][PATCH 2/7] kref: Add kref_read() "Reshetova, Elena" <elena.reshetova@intel.com> - 2016-11-17 18:10 +0100
RE: [RFC][PATCH 2/7] kref: Add kref_read() "Reshetova, Elena" <elena.reshetova@intel.com> - 2016-11-17 19:10 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-17 20:20 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-17 20:40 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-17 18:20 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Kees Cook <keescook@chromium.org> - 2016-11-17 20:40 +0100
Page 3 of 5 — ← Prev page 1 2 [3] 4 5 Next page →
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-11-18 12:40 +0100 |
| Subject | Re: [RFC][PATCH 7/7] kref: Implement using refcount_t |
| Message-ID | <sEPGq-6vc-33@gated-at.bofh.it> |
| In reply to | #1525148 |
On Fri, Nov 18, 2016 at 10:07:26AM +0000, Reshetova, Elena wrote:
>
> Peter do you have the changes to the refcount_t interface compare to
> the version in this patch?
> We are now starting working on atomic_t --> refcount_t conversions and
> it would save a bit of work to have latest version from you that we
> can be based upon.
The latestest version below, mostly just comment changes since last
time.
---
Subject: refcount_t: A special purpose refcount type
From: Peter Zijlstra <peterz@infradead.org>
Date: Mon Nov 14 18:06:19 CET 2016
Provide refcount_t, an atomic_t like primitive built just for
refcounting.
It provides saturation semantics such that overflow becomes impossible
and thereby 'spurious' use-after-free is avoided.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
include/linux/refcount.h | 241 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 241 insertions(+)
--- /dev/null
+++ b/include/linux/refcount.h
@@ -0,0 +1,241 @@
+#ifndef _LINUX_REFCOUNT_H
+#define _LINUX_REFCOUNT_H
+
+/*
+ * Variant of atomic_t specialized for reference counts.
+ *
+ * The interface matches the atomic_t interface (to aid in porting) but only
+ * provides the few functions one should use for reference counting.
+ *
+ * It differs in that the counter saturates at UINT_MAX and will not move once
+ * there. This avoids wrapping the counter and causing 'spurious'
+ * use-after-free issues.
+ *
+ * Memory ordering rules are slightly relaxed wrt regular atomic_t functions
+ * and provide only what is strictly required for refcounts.
+ *
+ * The increments are fully relaxed; these will not provide ordering. The
+ * rationale is that whatever is used to obtain the object we're increasing the
+ * reference count on will provide the ordering. For locked data structures,
+ * its the lock acquire, for RCU/lockless data structures its the dependent
+ * load.
+ *
+ * Do note that inc_not_zero() provides a control dependency which will order
+ * future stores against the inc, this ensures we'll never modify the object
+ * if we did not in fact acquire a reference.
+ *
+ * The decrements will provide release order, such that all the prior loads and
+ * stores will be issued before, it also provides a control dependency, which
+ * will order us against the subsequent free().
+ *
+ * The control dependency is against the load of the cmpxchg (ll/sc) that
+ * succeeded. This means the stores aren't fully ordered, but this is fine
+ * because the 1->0 transition indicates no concurrency.
+ *
+ * Note that the allocator is responsible for ordering things between free()
+ * and alloc().
+ *
+ *
+ * Note: the implementation hard relies on increments, bigger than 1 additions
+ * need explicit overflow -> saturation logic.
+ *
+ */
+
+#include <linux/atomic.h>
+#include <linux/bug.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
+
+typedef struct refcount_struct {
+ atomic_t refs;
+} refcount_t;
+
+#define REFCOUNT_INIT(n) { .refs = ATOMIC_INIT(n), }
+
+static inline void refcount_set(refcount_t *r, int n)
+{
+ atomic_set(&r->refs, n);
+}
+
+static inline unsigned int refcount_read(const refcount_t *r)
+{
+ return atomic_read(&r->refs);
+}
+
+/*
+ * Similar to atomic_inc(), will saturate at UINT_MAX and WARN.
+ *
+ * Provides no memory ordering, it is assumed the caller already has a
+ * reference on the object, will WARN when this is not so.
+ */
+static inline void refcount_inc(refcount_t *r)
+{
+ unsigned int old, new, val = atomic_read(&r->refs);
+
+ for (;;) {
+ WARN(!val, "refcount_t: increment on 0; use-after-free.\n");
+
+ if (unlikely(val == UINT_MAX))
+ return;
+
+ new = val + 1;
+ old = atomic_cmpxchg_relaxed(&r->refs, val, new);
+ if (old == val)
+ break;
+
+ val = old;
+ }
+
+ WARN(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
+}
+
+/*
+ * Similar to atomic_inc_not_zero(), will saturate at UINT_MAX and WARN.
+ *
+ * Provides no memory ordering, it is assumed the caller has guaranteed the
+ * object memory to be stable (RCU, etc.). It does provide a control dependency
+ * and thereby orders future stores. See the comment on top.
+ */
+static inline __must_check
+bool refcount_inc_not_zero(refcount_t *r)
+{
+ unsigned int old, new, val = atomic_read(&r->refs);
+
+ for (;;) {
+ if (!val)
+ return false;
+
+ if (unlikely(val == UINT_MAX))
+ return true;
+
+ new = val + 1;
+ old = atomic_cmpxchg_relaxed(&r->refs, val, new);
+ if (old == val)
+ break;
+
+ val = old;
+ }
+
+ WARN(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
+
+ return true;
+}
+
+/*
+ * Similar to atomic_dec_and_test(), it will WARN on underflow and fail to
+ * decrement when saturated at UINT_MAX.
+ *
+ * Provides release memory ordering, such that prior loads and stores are done
+ * before, and provides a control dependency such that free() must come after.
+ * See the comment on top.
+ */
+static inline __must_check
+bool refcount_dec_and_test(refcount_t *r)
+{
+ unsigned int old, new, val = atomic_read(&r->refs);
+
+ for (;;) {
+ if (val == UINT_MAX)
+ return false;
+
+ new = val - 1;
+ if (WARN(new > val, "refcount_t: underflow; use-after-free.\n"))
+ return false;
+
+ old = atomic_cmpxchg_release(&r->refs, val, new);
+ if (old == val)
+ break;
+
+ val = old;
+ }
+
+ return !new;
+}
+
+/*
+ * Similar to atomic_dec_and_mutex_lock(), it will WARN on underflow and fail
+ * to decrement when saturated at UINT_MAX.
+ *
+ * Provides release memory ordering, such that prior loads and stores are done
+ * before, and provides a control dependency such that free() must come after.
+ * See the comment on top.
+ */
+static inline __must_check
+bool refcount_dec_and_mutex_lock(refcount_t *r, struct mutex *lock)
+{
+ unsigned int old, new, val = atomic_read(&r->refs);
+ bool locked = false;
+
+ for (;;) {
+ if (val == UINT_MAX)
+ return false;
+
+ if (val == 1 && !locked) {
+ locked = true;
+ mutex_lock(lock);
+ }
+
+ new = val - 1;
+ if (WARN(new > val, "refcount_t: underflow; use-after-free.\n")) {
+ if (locked)
+ mutex_unlock(lock);
+ return false;
+ }
+
+ old = atomic_cmpxchg_release(&r->refs, val, new);
+ if (old == val)
+ break;
+
+ val = old;
+ }
+
+ if (new && locked)
+ mutex_unlock(lock);
+
+ return !new;
+}
+
+/*
+ * Similar to atomic_dec_and_lock(), it will WARN on underflow and fail to
+ * decrement when saturated at UINT_MAX.
+ *
+ * Provides release memory ordering, such that prior loads and stores are done
+ * before, and provides a control dependency such that free() must come after.
+ * See the comment on top.
+ */
+static inline __must_check
+bool refcount_dec_and_lock(refcount_t *r, spinlock_t *lock)
+{
+ unsigned int old, new, val = atomic_read(&r->refs);
+ bool locked = false;
+
+ for (;;) {
+ if (val == UINT_MAX)
+ return false;
+
+ if (val == 1 && !locked) {
+ locked = true;
+ spin_lock(lock);
+ }
+
+ new = val - 1;
+ if (WARN(new > val, "refcount_t: underflow; use-after-free.\n")) {
+ if (locked)
+ spin_unlock(lock);
+ return false;
+ }
+
+ old = atomic_cmpxchg_release(&r->refs, val, new);
+ if (old == val)
+ break;
+
+ val = old;
+ }
+
+ if (new && locked)
+ spin_unlock(lock);
+
+ return !new;
+}
+
+#endif /* _LINUX_REFCOUNT_H */
[toc] | [prev] | [next] | [standalone]
| From | Will Deacon <will.deacon@arm.com> |
|---|---|
| Date | 2016-11-18 18:10 +0100 |
| Subject | Re: [RFC][PATCH 7/7] kref: Implement using refcount_t |
| Message-ID | <sEUPM-1xM-33@gated-at.bofh.it> |
| In reply to | #1525214 |
On Fri, Nov 18, 2016 at 12:37:18PM +0100, Peter Zijlstra wrote:
> On Fri, Nov 18, 2016 at 10:07:26AM +0000, Reshetova, Elena wrote:
> >
> > Peter do you have the changes to the refcount_t interface compare to
> > the version in this patch?
>
> > We are now starting working on atomic_t --> refcount_t conversions and
> > it would save a bit of work to have latest version from you that we
> > can be based upon.
>
> The latestest version below, mostly just comment changes since last
> time.
>
> ---
> Subject: refcount_t: A special purpose refcount type
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Mon Nov 14 18:06:19 CET 2016
>
> Provide refcount_t, an atomic_t like primitive built just for
> refcounting.
>
> It provides saturation semantics such that overflow becomes impossible
> and thereby 'spurious' use-after-free is avoided.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> include/linux/refcount.h | 241 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 241 insertions(+)
>
> --- /dev/null
> +++ b/include/linux/refcount.h
> @@ -0,0 +1,241 @@
> +#ifndef _LINUX_REFCOUNT_H
> +#define _LINUX_REFCOUNT_H
> +
> +/*
> + * Variant of atomic_t specialized for reference counts.
> + *
> + * The interface matches the atomic_t interface (to aid in porting) but only
> + * provides the few functions one should use for reference counting.
> + *
> + * It differs in that the counter saturates at UINT_MAX and will not move once
> + * there. This avoids wrapping the counter and causing 'spurious'
> + * use-after-free issues.
> + *
> + * Memory ordering rules are slightly relaxed wrt regular atomic_t functions
> + * and provide only what is strictly required for refcounts.
> + *
> + * The increments are fully relaxed; these will not provide ordering. The
> + * rationale is that whatever is used to obtain the object we're increasing the
> + * reference count on will provide the ordering. For locked data structures,
> + * its the lock acquire, for RCU/lockless data structures its the dependent
> + * load.
> + *
> + * Do note that inc_not_zero() provides a control dependency which will order
> + * future stores against the inc, this ensures we'll never modify the object
> + * if we did not in fact acquire a reference.
> + *
> + * The decrements will provide release order, such that all the prior loads and
> + * stores will be issued before, it also provides a control dependency, which
> + * will order us against the subsequent free().
> + *
> + * The control dependency is against the load of the cmpxchg (ll/sc) that
> + * succeeded. This means the stores aren't fully ordered, but this is fine
> + * because the 1->0 transition indicates no concurrency.
> + *
> + * Note that the allocator is responsible for ordering things between free()
> + * and alloc().
> + *
> + *
> + * Note: the implementation hard relies on increments, bigger than 1 additions
> + * need explicit overflow -> saturation logic.
> + *
> + */
> +
> +#include <linux/atomic.h>
> +#include <linux/bug.h>
> +#include <linux/mutex.h>
> +#include <linux/spinlock.h>
> +
> +typedef struct refcount_struct {
> + atomic_t refs;
> +} refcount_t;
> +
> +#define REFCOUNT_INIT(n) { .refs = ATOMIC_INIT(n), }
> +
> +static inline void refcount_set(refcount_t *r, int n)
> +{
> + atomic_set(&r->refs, n);
> +}
> +
> +static inline unsigned int refcount_read(const refcount_t *r)
> +{
> + return atomic_read(&r->refs);
> +}
Minor nit, but it might be worth being consistent in our usage of int
(parameter to refcount_set) and unsigned int (return value of
refcount_read).
> +
> +/*
> + * Similar to atomic_inc(), will saturate at UINT_MAX and WARN.
> + *
> + * Provides no memory ordering, it is assumed the caller already has a
> + * reference on the object, will WARN when this is not so.
> + */
> +static inline void refcount_inc(refcount_t *r)
> +{
> + unsigned int old, new, val = atomic_read(&r->refs);
> +
> + for (;;) {
> + WARN(!val, "refcount_t: increment on 0; use-after-free.\n");
> +
> + if (unlikely(val == UINT_MAX))
> + return;
> +
> + new = val + 1;
> + old = atomic_cmpxchg_relaxed(&r->refs, val, new);
> + if (old == val)
> + break;
> +
> + val = old;
> + }
> +
> + WARN(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
> +}
> +
> +/*
> + * Similar to atomic_inc_not_zero(), will saturate at UINT_MAX and WARN.
> + *
> + * Provides no memory ordering, it is assumed the caller has guaranteed the
> + * object memory to be stable (RCU, etc.). It does provide a control dependency
> + * and thereby orders future stores. See the comment on top.
> + */
> +static inline __must_check
> +bool refcount_inc_not_zero(refcount_t *r)
> +{
> + unsigned int old, new, val = atomic_read(&r->refs);
> +
> + for (;;) {
> + if (!val)
> + return false;
> +
> + if (unlikely(val == UINT_MAX))
> + return true;
> +
> + new = val + 1;
> + old = atomic_cmpxchg_relaxed(&r->refs, val, new);
> + if (old == val)
> + break;
> +
> + val = old;
Hmm, it's a shame this code is duplicated from refcount_inc, but I suppose
you can actually be racing against the counter going to zero here and really
need to check it each time round the loop. Humph. That said, given that
refcount_inc WARNs if the thing is zero, maybe that could just call
refcount_inc_not_zero and warn if it returns false? Does it matter that
we don't actually do the increment?
> + }
> +
> + WARN(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
> +
> + return true;
> +}
> +
> +/*
> + * Similar to atomic_dec_and_test(), it will WARN on underflow and fail to
> + * decrement when saturated at UINT_MAX.
It also fails to decrement in the underflow case (which is fine, but not
obvious from the comment). Same thing below.
> + *
> + * Provides release memory ordering, such that prior loads and stores are done
> + * before, and provides a control dependency such that free() must come after.
> + * See the comment on top.
> + */
> +static inline __must_check
> +bool refcount_dec_and_test(refcount_t *r)
> +{
> + unsigned int old, new, val = atomic_read(&r->refs);
> +
> + for (;;) {
> + if (val == UINT_MAX)
> + return false;
> +
> + new = val - 1;
> + if (WARN(new > val, "refcount_t: underflow; use-after-free.\n"))
> + return false;
Wouldn't it be clearer to compare val with 0 before doing the decrement?
Will
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-11-18 20:00 +0100 |
| Subject | Re: [RFC][PATCH 7/7] kref: Implement using refcount_t |
| Message-ID | <sEWyf-2tW-57@gated-at.bofh.it> |
| In reply to | #1525550 |
On Fri, Nov 18, 2016 at 05:06:55PM +0000, Will Deacon wrote:
> On Fri, Nov 18, 2016 at 12:37:18PM +0100, Peter Zijlstra wrote:
> > +static inline void refcount_set(refcount_t *r, int n)
> > +{
> > + atomic_set(&r->refs, n);
> > +}
> > +
> > +static inline unsigned int refcount_read(const refcount_t *r)
> > +{
> > + return atomic_read(&r->refs);
> > +}
>
> Minor nit, but it might be worth being consistent in our usage of int
> (parameter to refcount_set) and unsigned int (return value of
> refcount_read).
Duh, I actually spotted that once and still didn't fix that :/
> > +static inline __must_check
> > +bool refcount_inc_not_zero(refcount_t *r)
> > +{
> > + unsigned int old, new, val = atomic_read(&r->refs);
> > +
> > + for (;;) {
> > + if (!val)
> > + return false;
> > +
> > + if (unlikely(val == UINT_MAX))
> > + return true;
> > +
> > + new = val + 1;
> > + old = atomic_cmpxchg_relaxed(&r->refs, val, new);
> > + if (old == val)
> > + break;
> > +
> > + val = old;
>
> Hmm, it's a shame this code is duplicated from refcount_inc, but I suppose
> you can actually be racing against the counter going to zero here and really
> need to check it each time round the loop. Humph. That said, given that
> refcount_inc WARNs if the thing is zero, maybe that could just call
> refcount_inc_not_zero and warn if it returns false? Does it matter that
> we don't actually do the increment?
Dunno, it _should_ not, but then again, who knows.
I can certainly write it as WARN_ON(!refcount_inc_not_zero());
> > +static inline __must_check
> > +bool refcount_dec_and_test(refcount_t *r)
> > +{
> > + unsigned int old, new, val = atomic_read(&r->refs);
> > +
> > + for (;;) {
> > + if (val == UINT_MAX)
> > + return false;
> > +
> > + new = val - 1;
> > + if (WARN(new > val, "refcount_t: underflow; use-after-free.\n"))
> > + return false;
>
> Wouldn't it be clearer to compare val with 0 before doing the decrement?
Maybe, this way you can change the 1 and it'll keep working. Then again,
you can't do that with the inc side, so who cares.
[toc] | [prev] | [next] | [standalone]
| From | Boqun Feng <boqun.feng@gmail.com> |
|---|---|
| Date | 2016-11-21 05:10 +0100 |
| Subject | Re: [RFC][PATCH 7/7] kref: Implement using refcount_t |
| Message-ID | <sFO5z-4xa-3@gated-at.bofh.it> |
| In reply to | #1525550 |
[Multipart message — attachments visible in raw view] — view raw
On Fri, Nov 18, 2016 at 05:06:55PM +0000, Will Deacon wrote:
> On Fri, Nov 18, 2016 at 12:37:18PM +0100, Peter Zijlstra wrote:
> > On Fri, Nov 18, 2016 at 10:07:26AM +0000, Reshetova, Elena wrote:
> > >
> > > Peter do you have the changes to the refcount_t interface compare to
> > > the version in this patch?
> >
> > > We are now starting working on atomic_t --> refcount_t conversions and
> > > it would save a bit of work to have latest version from you that we
> > > can be based upon.
> >
> > The latestest version below, mostly just comment changes since last
> > time.
> >
> > ---
> > Subject: refcount_t: A special purpose refcount type
> > From: Peter Zijlstra <peterz@infradead.org>
> > Date: Mon Nov 14 18:06:19 CET 2016
> >
> > Provide refcount_t, an atomic_t like primitive built just for
> > refcounting.
> >
> > It provides saturation semantics such that overflow becomes impossible
> > and thereby 'spurious' use-after-free is avoided.
> >
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > ---
> > include/linux/refcount.h | 241 +++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 241 insertions(+)
> >
> > --- /dev/null
> > +++ b/include/linux/refcount.h
> > @@ -0,0 +1,241 @@
> > +#ifndef _LINUX_REFCOUNT_H
> > +#define _LINUX_REFCOUNT_H
> > +
> > +/*
> > + * Variant of atomic_t specialized for reference counts.
> > + *
> > + * The interface matches the atomic_t interface (to aid in porting) but only
> > + * provides the few functions one should use for reference counting.
> > + *
> > + * It differs in that the counter saturates at UINT_MAX and will not move once
> > + * there. This avoids wrapping the counter and causing 'spurious'
> > + * use-after-free issues.
> > + *
> > + * Memory ordering rules are slightly relaxed wrt regular atomic_t functions
> > + * and provide only what is strictly required for refcounts.
> > + *
> > + * The increments are fully relaxed; these will not provide ordering. The
> > + * rationale is that whatever is used to obtain the object we're increasing the
> > + * reference count on will provide the ordering. For locked data structures,
> > + * its the lock acquire, for RCU/lockless data structures its the dependent
> > + * load.
> > + *
> > + * Do note that inc_not_zero() provides a control dependency which will order
> > + * future stores against the inc, this ensures we'll never modify the object
> > + * if we did not in fact acquire a reference.
> > + *
> > + * The decrements will provide release order, such that all the prior loads and
> > + * stores will be issued before, it also provides a control dependency, which
> > + * will order us against the subsequent free().
> > + *
> > + * The control dependency is against the load of the cmpxchg (ll/sc) that
> > + * succeeded. This means the stores aren't fully ordered, but this is fine
> > + * because the 1->0 transition indicates no concurrency.
> > + *
> > + * Note that the allocator is responsible for ordering things between free()
> > + * and alloc().
> > + *
> > + *
> > + * Note: the implementation hard relies on increments, bigger than 1 additions
> > + * need explicit overflow -> saturation logic.
> > + *
> > + */
> > +
> > +#include <linux/atomic.h>
> > +#include <linux/bug.h>
> > +#include <linux/mutex.h>
> > +#include <linux/spinlock.h>
> > +
> > +typedef struct refcount_struct {
> > + atomic_t refs;
> > +} refcount_t;
> > +
> > +#define REFCOUNT_INIT(n) { .refs = ATOMIC_INIT(n), }
> > +
> > +static inline void refcount_set(refcount_t *r, int n)
> > +{
> > + atomic_set(&r->refs, n);
> > +}
> > +
> > +static inline unsigned int refcount_read(const refcount_t *r)
> > +{
> > + return atomic_read(&r->refs);
> > +}
>
> Minor nit, but it might be worth being consistent in our usage of int
> (parameter to refcount_set) and unsigned int (return value of
> refcount_read).
>
> > +
> > +/*
> > + * Similar to atomic_inc(), will saturate at UINT_MAX and WARN.
> > + *
> > + * Provides no memory ordering, it is assumed the caller already has a
> > + * reference on the object, will WARN when this is not so.
> > + */
> > +static inline void refcount_inc(refcount_t *r)
> > +{
> > + unsigned int old, new, val = atomic_read(&r->refs);
> > +
> > + for (;;) {
> > + WARN(!val, "refcount_t: increment on 0; use-after-free.\n");
> > +
> > + if (unlikely(val == UINT_MAX))
> > + return;
> > +
> > + new = val + 1;
> > + old = atomic_cmpxchg_relaxed(&r->refs, val, new);
> > + if (old == val)
> > + break;
> > +
> > + val = old;
> > + }
> > +
> > + WARN(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
> > +}
> > +
> > +/*
> > + * Similar to atomic_inc_not_zero(), will saturate at UINT_MAX and WARN.
> > + *
> > + * Provides no memory ordering, it is assumed the caller has guaranteed the
> > + * object memory to be stable (RCU, etc.). It does provide a control dependency
> > + * and thereby orders future stores. See the comment on top.
> > + */
> > +static inline __must_check
> > +bool refcount_inc_not_zero(refcount_t *r)
> > +{
> > + unsigned int old, new, val = atomic_read(&r->refs);
> > +
> > + for (;;) {
> > + if (!val)
> > + return false;
> > +
> > + if (unlikely(val == UINT_MAX))
> > + return true;
> > +
> > + new = val + 1;
> > + old = atomic_cmpxchg_relaxed(&r->refs, val, new);
> > + if (old == val)
> > + break;
> > +
> > + val = old;
>
> Hmm, it's a shame this code is duplicated from refcount_inc, but I suppose
> you can actually be racing against the counter going to zero here and really
> need to check it each time round the loop. Humph. That said, given that
> refcount_inc WARNs if the thing is zero, maybe that could just call
> refcount_inc_not_zero and warn if it returns false? Does it matter that
> we don't actually do the increment?
>
> > + }
> > +
> > + WARN(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
> > +
> > + return true;
> > +}
> > +
> > +/*
> > + * Similar to atomic_dec_and_test(), it will WARN on underflow and fail to
> > + * decrement when saturated at UINT_MAX.
>
> It also fails to decrement in the underflow case (which is fine, but not
> obvious from the comment). Same thing below.
>
Maybe a table in the comment like the following helps?
/*
* T: return true, F: return fasle
* W: trigger WARNING
* N: no effect
*
* | value before ops |
* | 0 | 1 | UINT_MAX - 1 | UINT_MAX |
* ---------------------+-------+-------+--------------+----------+
* inc() | W | | W | N |
* inc_not_zero() | FN | T | WT | WTN |
* dec_and_test() | WFN | T | F | FN |
* dec_and_mutex_lock() | WFN | T | F | FN |
* dec_and_spin_lock() | WFN | T | F | FN |
*/
Regards,
Boqun
> > + *
> > + * Provides release memory ordering, such that prior loads and stores are done
> > + * before, and provides a control dependency such that free() must come after.
> > + * See the comment on top.
> > + */
> > +static inline __must_check
> > +bool refcount_dec_and_test(refcount_t *r)
> > +{
> > + unsigned int old, new, val = atomic_read(&r->refs);
> > +
> > + for (;;) {
> > + if (val == UINT_MAX)
> > + return false;
> > +
> > + new = val - 1;
> > + if (WARN(new > val, "refcount_t: underflow; use-after-free.\n"))
> > + return false;
>
> Wouldn't it be clearer to compare val with 0 before doing the decrement?
>
> Will
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2016-11-21 08:50 +0100 |
| Subject | Re: [RFC][PATCH 7/7] kref: Implement using refcount_t |
| Message-ID | <sFRwu-6Fr-7@gated-at.bofh.it> |
| In reply to | #1526351 |
* Boqun Feng <boqun.feng@gmail.com> wrote: > > It also fails to decrement in the underflow case (which is fine, but not > > obvious from the comment). Same thing below. > > > > Maybe a table in the comment like the following helps? > > /* > * T: return true, F: return fasle > * W: trigger WARNING > * N: no effect > * > * | value before ops | > * | 0 | 1 | UINT_MAX - 1 | UINT_MAX | > * ---------------------+-------+-------+--------------+----------+ > * inc() | W | | W | N | > * inc_not_zero() | FN | T | WT | WTN | > * dec_and_test() | WFN | T | F | FN | > * dec_and_mutex_lock() | WFN | T | F | FN | > * dec_and_spin_lock() | WFN | T | F | FN | > */ Yes! nit: s/fasle/false Also, I think we want to do a couple of other changes as well to make it more readable, extend the columns with 'normal' values (2 and UINT_MAX-2) and order the colums properly. I.e. something like: /* * The before/after outcome of various atomic ops: * * T: returns true * F: returns false * ---------------------------------- * W: op triggers kernel WARNING * ---------------------------------- * 0: no change to atomic var value * +: atomic var value increases by 1 * -: atomic var value decreases by 1 * ---------------------------------- * -1: UINT_MAX * -2: UINT_MAX-1 * -3: UINT_MAX-2 * * ---------------------+-----+-----+-----+-----+-----+-----+ * value before: | -3 | -2 | -1 | 0 | 1 | 2 | * ---------------------+-----+-----+-----+-----+-----+-----+ * value+effect after: | * ---------------------+ | | | | | | * inc() | ..+ | W.+ | ..0 | W.+ | ..+ | ..+ | * inc_not_zero() | .T+ | WT+ | WT0 | .F0 | .T+ | .T+ | * dec_and_test() | .F- | .F- | .F0 | WF0 | .T- | .F- | * dec_and_mutex_lock() | .F- | .F- | .F0 | WF0 | .T- | .F- | * dec_and_spin_lock() | .F- | .F- | .F0 | WF0 | .T- | .F- | * ---------------------+-----+-----+-----+-----+-----+-----+ * * So for example: 'WT+' in the inc_not_zero() row and '-2' column * means that when the atomic_inc_not_zero() function is called * with an atomic var that has a value of UINT_MAX-1, then the * atomic var's value will increase to the maximum overflow value * of UINT_MAX and will produce a warning. The function returns * 'true'. */ I think this table makes the overflow/underflow semantics pretty clear and also documents the regular behavior of these atomic ops pretty intuitively. Agreed? Thanks, Ingo
[toc] | [prev] | [next] | [standalone]
| From | Boqun Feng <boqun.feng@gmail.com> |
|---|---|
| Date | 2016-11-21 09:40 +0100 |
| Subject | Re: [RFC][PATCH 7/7] kref: Implement using refcount_t |
| Message-ID | <sFSiS-7gi-15@gated-at.bofh.it> |
| In reply to | #1526426 |
[Multipart message — attachments visible in raw view] — view raw
On Mon, Nov 21, 2016 at 08:48:26AM +0100, Ingo Molnar wrote: > > * Boqun Feng <boqun.feng@gmail.com> wrote: > > > > It also fails to decrement in the underflow case (which is fine, but not > > > obvious from the comment). Same thing below. > > > > > > > Maybe a table in the comment like the following helps? > > > > /* > > * T: return true, F: return fasle > > * W: trigger WARNING > > * N: no effect > > * > > * | value before ops | > > * | 0 | 1 | UINT_MAX - 1 | UINT_MAX | > > * ---------------------+-------+-------+--------------+----------+ > > * inc() | W | | W | N | > > * inc_not_zero() | FN | T | WT | WTN | > > * dec_and_test() | WFN | T | F | FN | > > * dec_and_mutex_lock() | WFN | T | F | FN | > > * dec_and_spin_lock() | WFN | T | F | FN | > > */ > > Yes! > > nit: s/fasle/false > > Also, I think we want to do a couple of other changes as well to make it more > readable, extend the columns with 'normal' values (2 and UINT_MAX-2) and order the > colums properly. I.e. something like: > > /* > * The before/after outcome of various atomic ops: > * > * T: returns true > * F: returns false > * ---------------------------------- > * W: op triggers kernel WARNING > * ---------------------------------- > * 0: no change to atomic var value > * +: atomic var value increases by 1 > * -: atomic var value decreases by 1 > * ---------------------------------- > * -1: UINT_MAX > * -2: UINT_MAX-1 > * -3: UINT_MAX-2 > * > * ---------------------+-----+-----+-----+-----+-----+-----+ > * value before: | -3 | -2 | -1 | 0 | 1 | 2 | > * ---------------------+-----+-----+-----+-----+-----+-----+ > * value+effect after: | > * ---------------------+ | | | | | | > * inc() | ..+ | W.+ | ..0 | W.+ | ..+ | ..+ | > * inc_not_zero() | .T+ | WT+ | WT0 | .F0 | .T+ | .T+ | > * dec_and_test() | .F- | .F- | .F0 | WF0 | .T- | .F- | > * dec_and_mutex_lock() | .F- | .F- | .F0 | WF0 | .T- | .F- | > * dec_and_spin_lock() | .F- | .F- | .F0 | WF0 | .T- | .F- | > * ---------------------+-----+-----+-----+-----+-----+-----+ > * > * So for example: 'WT+' in the inc_not_zero() row and '-2' column > * means that when the atomic_inc_not_zero() function is called > * with an atomic var that has a value of UINT_MAX-1, then the > * atomic var's value will increase to the maximum overflow value > * of UINT_MAX and will produce a warning. The function returns > * 'true'. > */ > > I think this table makes the overflow/underflow semantics pretty clear and also > documents the regular behavior of these atomic ops pretty intuitively. > > Agreed? > Sure, this looks pretty great! Much more informative and readable than my version ;-) Thank you. Regards, Boqun > Thanks, > > Ingo
[toc] | [prev] | [next] | [standalone]
| From | Boqun Feng <boqun.feng@gmail.com> |
|---|---|
| Date | 2016-11-21 09:50 +0100 |
| Subject | Re: [RFC][PATCH 7/7] kref: Implement using refcount_t |
| Message-ID | <sFSsy-7jA-21@gated-at.bofh.it> |
| In reply to | #1525214 |
[Multipart message — attachments visible in raw view] — view raw
On Fri, Nov 18, 2016 at 12:37:18PM +0100, Peter Zijlstra wrote:
[snip]
> +
> +/*
> + * Similar to atomic_inc(), will saturate at UINT_MAX and WARN.
> + *
> + * Provides no memory ordering, it is assumed the caller already has a
> + * reference on the object, will WARN when this is not so.
> + */
> +static inline void refcount_inc(refcount_t *r)
> +{
> + unsigned int old, new, val = atomic_read(&r->refs);
> +
> + for (;;) {
> + WARN(!val, "refcount_t: increment on 0; use-after-free.\n");
> +
Do we want to put the address of @r into the WARN information? Which
could help us locate the problematic object quickly.
Regards,
Boqun
> + if (unlikely(val == UINT_MAX))
> + return;
> +
> + new = val + 1;
> + old = atomic_cmpxchg_relaxed(&r->refs, val, new);
> + if (old == val)
> + break;
> +
> + val = old;
> + }
> +
> + WARN(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
> +}
[...]
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-11-21 10:10 +0100 |
| Subject | Re: [RFC][PATCH 7/7] kref: Implement using refcount_t |
| Message-ID | <sFSLU-7Fl-27@gated-at.bofh.it> |
| In reply to | #1526459 |
On Mon, Nov 21, 2016 at 04:44:28PM +0800, Boqun Feng wrote:
> On Fri, Nov 18, 2016 at 12:37:18PM +0100, Peter Zijlstra wrote:
> [snip]
> > +
> > +/*
> > + * Similar to atomic_inc(), will saturate at UINT_MAX and WARN.
> > + *
> > + * Provides no memory ordering, it is assumed the caller already has a
> > + * reference on the object, will WARN when this is not so.
> > + */
> > +static inline void refcount_inc(refcount_t *r)
> > +{
> > + unsigned int old, new, val = atomic_read(&r->refs);
> > +
> > + for (;;) {
> > + WARN(!val, "refcount_t: increment on 0; use-after-free.\n");
> > +
>
> Do we want to put the address of @r into the WARN information? Which
> could help us locate the problematic object quickly.
I explicitly didn't do that because printing kernel addresses is
generally frowned upon. Also, random heap addresses are just that,
random. In most cases the backtrace is more informative.
[toc] | [prev] | [next] | [standalone]
| From | Boqun Feng <boqun.feng@gmail.com> |
|---|---|
| Date | 2016-11-21 10:40 +0100 |
| Subject | Re: [RFC][PATCH 7/7] kref: Implement using refcount_t |
| Message-ID | <sFTeW-7P2-11@gated-at.bofh.it> |
| In reply to | #1526469 |
[Multipart message — attachments visible in raw view] — view raw
On Mon, Nov 21, 2016 at 10:02:23AM +0100, Peter Zijlstra wrote:
> On Mon, Nov 21, 2016 at 04:44:28PM +0800, Boqun Feng wrote:
> > On Fri, Nov 18, 2016 at 12:37:18PM +0100, Peter Zijlstra wrote:
> > [snip]
> > > +
> > > +/*
> > > + * Similar to atomic_inc(), will saturate at UINT_MAX and WARN.
> > > + *
> > > + * Provides no memory ordering, it is assumed the caller already has a
> > > + * reference on the object, will WARN when this is not so.
> > > + */
> > > +static inline void refcount_inc(refcount_t *r)
> > > +{
> > > + unsigned int old, new, val = atomic_read(&r->refs);
> > > +
> > > + for (;;) {
> > > + WARN(!val, "refcount_t: increment on 0; use-after-free.\n");
> > > +
> >
> > Do we want to put the address of @r into the WARN information? Which
> > could help us locate the problematic object quickly.
>
> I explicitly didn't do that because printing kernel addresses is
> generally frowned upon. Also, random heap addresses are just that,
> random. In most cases the backtrace is more informative.
Fair enough ;-)
Regards,
Boqun
[toc] | [prev] | [next] | [standalone]
| From | "Reshetova, Elena" <elena.reshetova@intel.com> |
|---|---|
| Date | 2016-11-18 11:50 +0100 |
| Subject | RE: [RFC][PATCH 7/7] kref: Implement using refcount_t |
| Message-ID | <sEOU1-602-3@gated-at.bofh.it> |
| In reply to | #1521937 |
>Provide refcount_t, an atomic_t like primitive built just for refcounting. >It provides overflow and underflow checks as well as saturation semantics such that when it overflows, we'll never attempt to free it again, ever. >Peter do you have the changes to the refcount_t interface compare to the version in this patch? >We are now starting working on atomic_t --> refcount_t conversions and it would save a bit of work to have latest version from you that we can be based upon. Oh, and if we define refcount_t to be just atomic_t underneath, what about the other atomic_long_t, local_t and atomic64_t cases when it is used for recounting? I don't feel good just simply changing them to become atomic_t under refcount_t wrapper.....
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-11-18 12:00 +0100 |
| Subject | Re: [RFC][PATCH 7/7] kref: Implement using refcount_t |
| Message-ID | <sEP3I-63e-17@gated-at.bofh.it> |
| In reply to | #1525168 |
Could you please fix you mailer to not unwrap the emails? On Fri, Nov 18, 2016 at 10:47:40AM +0000, Reshetova, Elena wrote: > >Provide refcount_t, an atomic_t like primitive built just for > >refcounting. It provides overflow and underflow checks as well as > >saturation semantics such that when it overflows, we'll never attempt > >to free it again, ever. > > >Peter do you have the changes to the refcount_t interface compare to > >the version in this patch? We are now starting working on atomic_t > >--> refcount_t conversions and it would save a bit of work to have > >latest version from you that we can be based upon. > > Oh, and if we define refcount_t to be just atomic_t underneath, what > about the other atomic_long_t, local_t and atomic64_t cases when it is > used for recounting? I don't feel good just simply changing them to > become atomic_t under refcount_t wrapper..... Is there anybody using local_t ? That seems 'creative' and highly questionable. As for atomic_long_t there's very few, I'd leave them be for now, and I couldn't find a single atomic64_t refcount user.
[toc] | [prev] | [next] | [standalone]
| From | "Reshetova, Elena" <elena.reshetova@intel.com> |
|---|---|
| Date | 2016-11-18 18:00 +0100 |
| Subject | RE: [RFC][PATCH 7/7] kref: Implement using refcount_t |
| Message-ID | <sEUG6-1ff-55@gated-at.bofh.it> |
| In reply to | #1525174 |
> Could you please fix you mailer to not unwrap the emails? I wish I understand what you mean by "unwrap"... ? On Fri, Nov 18, 2016 at 10:47:40AM +0000, Reshetova, Elena wrote: > >Provide refcount_t, an atomic_t like primitive built just for > >refcounting. It provides overflow and underflow checks as well as > >saturation semantics such that when it overflows, we'll never attempt > >to free it again, ever. > > >Peter do you have the changes to the refcount_t interface compare to > >the version in this patch? We are now starting working on atomic_t > >--> refcount_t conversions and it would save a bit of work to have > >latest version from you that we can be based upon. > > Oh, and if we define refcount_t to be just atomic_t underneath, what > about the other atomic_long_t, local_t and atomic64_t cases when it is > used for recounting? I don't feel good just simply changing them to > become atomic_t under refcount_t wrapper..... > Is there anybody using local_t ? That seems 'creative' and highly questionable. I am not yet sure about refcounts, but local_t itself is used in couple of places. >As for atomic_long_t there's very few, I'd leave them be for now, Ok, I have started a list on them to keep track, but we need to do them also. There is no reason for them not to be refcounts, since so far the ones I see are classical refcounts. >and I couldn't find a single atomic64_t refcount user. I will check when I get over the atomic_t and atomic_long.
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-11-18 20:00 +0100 |
| Subject | Re: [RFC][PATCH 7/7] kref: Implement using refcount_t |
| Message-ID | <sEWye-2tW-47@gated-at.bofh.it> |
| In reply to | #1525541 |
On Fri, Nov 18, 2016 at 04:58:52PM +0000, Reshetova, Elena wrote: > > Could you please fix you mailer to not unwrap the emails? > > I wish I understand what you mean by "unwrap"... ? Where I always have lines wrapped at 78 characters, but often when I see them back in your reply, they're unwrapped and go on forever. For some reason your mailer reflows text and mucks with whitespace. I know Outlook likes to do this by default. > On Fri, Nov 18, 2016 at 10:47:40AM +0000, Reshetova, Elena wrote: > > Oh, and if we define refcount_t to be just atomic_t underneath, what > > about the other atomic_long_t, local_t and atomic64_t cases when it is > > used for recounting? I don't feel good just simply changing them to > > become atomic_t under refcount_t wrapper..... > > > Is there anybody using local_t ? That seems 'creative' and highly questionable. > I am not yet sure about refcounts, but local_t itself is used in couple of places. Sure, there's local_t usage, but I'd be very surprised if there's a single refcount usage among them. > >As for atomic_long_t there's very few, I'd leave them be for now, > Ok, I have started a list on them to keep track, but we need to do > them also. There is no reason for them not to be refcounts, since so > far the ones I see are classical refcounts. Well, if you get to tools (cocci script or whatever) to reliably work fork atomic_t, then converting the few atomic_long_t's later should be trivial.
[toc] | [prev] | [next] | [standalone]
| From | "Reshetova, Elena" <elena.reshetova@intel.com> |
|---|---|
| Date | 2016-11-19 08:20 +0100 |
| Subject | RE: [RFC][PATCH 7/7] kref: Implement using refcount_t |
| Message-ID | <sF86m-1NG-17@gated-at.bofh.it> |
| In reply to | #1525643 |
> On Fri, Nov 18, 2016 at 04:58:52PM +0000, Reshetova, Elena wrote: > > > Could you please fix you mailer to not unwrap the emails? > > > > I wish I understand what you mean by "unwrap"... ? > > Where I always have lines wrapped at 78 characters, but often when I see > them back in your reply, they're unwrapped and go on forever. > > For some reason your mailer reflows text and mucks with whitespace. I > know Outlook likes to do this by default. Ok, I think I managed to fix it. Hope it looks better now. > > On Fri, Nov 18, 2016 at 10:47:40AM +0000, Reshetova, Elena wrote: > > > > Oh, and if we define refcount_t to be just atomic_t underneath, what > > > about the other atomic_long_t, local_t and atomic64_t cases when it is > > > used for recounting? I don't feel good just simply changing them to > > > become atomic_t under refcount_t wrapper..... > > > > > Is there anybody using local_t ? That seems 'creative' and highly > questionable. > > I am not yet sure about refcounts, but local_t itself is used in couple of places. > > Sure, there's local_t usage, but I'd be very surprised if there's a > single refcount usage among them. > > > >As for atomic_long_t there's very few, I'd leave them be for now, > > > Ok, I have started a list on them to keep track, but we need to do > > them also. There is no reason for them not to be refcounts, since so > > far the ones I see are classical refcounts. > > Well, if you get to tools (cocci script or whatever) to reliably work > fork atomic_t, then converting the few atomic_long_t's later should be > trivial. I am using coccinelle to find all occurrences, but I do the changes only in semi-automated fashion. Each change needs a proper manual review anyway and often one variable usage is spread between different headers/source files, so I prefer not to go to full automation and then not being sure what I have done.
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-11-19 12:50 +0100 |
| Subject | Re: [RFC][PATCH 7/7] kref: Implement using refcount_t |
| Message-ID | <sFcjD-4o0-5@gated-at.bofh.it> |
| In reply to | #1525862 |
On Sat, Nov 19, 2016 at 07:14:08AM +0000, Reshetova, Elena wrote: > > Well, if you get to tools (cocci script or whatever) to reliably work > > fork atomic_t, then converting the few atomic_long_t's later should be > > trivial. > > I am using coccinelle to find all occurrences, but I do the changes > only in semi-automated fashion. If you can get the detection solid, that's good enough. > Each change needs a proper manual review anyway and often one variable > usage is spread between different headers/source files, so I prefer > not to go to full automation and then not being sure what I have done. Sure, every patch needs review, regardless of how it came to be.
[toc] | [prev] | [next] | [standalone]
| From | Christoph Hellwig <hch@infradead.org> |
|---|---|
| Date | 2016-11-14 19:20 +0100 |
| Subject | Re: [RFC][PATCH 2/7] kref: Add kref_read() |
| Message-ID | <sDu1k-25s-19@gated-at.bofh.it> |
| In reply to | #1521929 |
On Mon, Nov 14, 2016 at 06:39:48PM +0100, Peter Zijlstra wrote: > Since we need to change the implementation, stop exposing internals. > > Provide kref_read() to read the current reference count; typically > used for debug messages. Can we just provide a printk specifier for a kref value instead as that is the only valid use case for reading the value?
[toc] | [prev] | [next] | [standalone]
| From | Greg KH <gregkh@linuxfoundation.org> |
|---|---|
| Date | 2016-11-15 08:30 +0100 |
| Subject | Re: [RFC][PATCH 2/7] kref: Add kref_read() |
| Message-ID | <sDGlQ-1RV-17@gated-at.bofh.it> |
| In reply to | #1521965 |
On Mon, Nov 14, 2016 at 10:16:55AM -0800, Christoph Hellwig wrote: > On Mon, Nov 14, 2016 at 06:39:48PM +0100, Peter Zijlstra wrote: > > Since we need to change the implementation, stop exposing internals. > > > > Provide kref_read() to read the current reference count; typically > > used for debug messages. > > Can we just provide a printk specifier for a kref value instead as > that is the only valid use case for reading the value? Yeah, that would be great as no one should be doing anything logic-related based on the kref value. thanks, greg k-h
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-11-15 08:50 +0100 |
| Subject | Re: [RFC][PATCH 2/7] kref: Add kref_read() |
| Message-ID | <sDGFb-1YS-13@gated-at.bofh.it> |
| In reply to | #1522405 |
On Tue, Nov 15, 2016 at 08:28:55AM +0100, Greg KH wrote: > On Mon, Nov 14, 2016 at 10:16:55AM -0800, Christoph Hellwig wrote: > > On Mon, Nov 14, 2016 at 06:39:48PM +0100, Peter Zijlstra wrote: > > > Since we need to change the implementation, stop exposing internals. > > > > > > Provide kref_read() to read the current reference count; typically > > > used for debug messages. > > > > Can we just provide a printk specifier for a kref value instead as > > that is the only valid use case for reading the value? > > Yeah, that would be great as no one should be doing anything > logic-related based on the kref value. IIRC there are a few users that WARN_ON() the value with a minimum bound or somesuch. Those would be left in the cold, but yes I too like the idea of a printk() format specifier.
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2016-11-15 09:40 +0100 |
| Subject | [PATCH] printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref' |
| Message-ID | <sDHrz-2yp-9@gated-at.bofh.it> |
| In reply to | #1522405 |
* Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Nov 14, 2016 at 10:16:55AM -0800, Christoph Hellwig wrote:
> > On Mon, Nov 14, 2016 at 06:39:48PM +0100, Peter Zijlstra wrote:
> > > Since we need to change the implementation, stop exposing internals.
> > >
> > > Provide kref_read() to read the current reference count; typically
> > > used for debug messages.
> >
> > Can we just provide a printk specifier for a kref value instead as
> > that is the only valid use case for reading the value?
>
> Yeah, that would be great as no one should be doing anything
> logic-related based on the kref value.
Find below a patch that implements %pAk for 'struct kref' count printing and
%pAr for atomic_t counter printing.
This is against vanilla upstream.
Thanks,
Ingo
============================>
Subject: printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref'
From: Ingo Molnar <mingo@kernel.org>
Date: Tue Nov 15 08:53:14 CET 2016
A decade of kref internals exposed to driver writers has proven that
exposing internals to them is a bad idea.
Make the bad patterns a bit easier to detect and allow cleaner
printouts by offering two new printk format string extensions:
%pAr - print the atomic_t count in decimal
%pAk - print the struct kref count in decimal
Also add printf testcases:
[ 0.353126] test_printf: all 268 tests passed
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
Documentation/printk-formats.txt | 10 +++++++++
lib/test_printf.c | 28 ++++++++++++++++++++++++++
lib/vsprintf.c | 42 +++++++++++++++++++++++++++++++++++++++
3 files changed, 80 insertions(+)
Index: tip/Documentation/printk-formats.txt
===================================================================
--- tip.orig/Documentation/printk-formats.txt
+++ tip/Documentation/printk-formats.txt
@@ -316,6 +316,16 @@ Flags bitfields such as page flags, gfp_
Passed by reference.
+atomic variables such atomic_t or struct kref:
+
+ %pAr atomic_t count
+ %pAk struct kref count
+
+ For printing the current count value of atomic variables. This is
+ preferred to accessing the counts directly.
+
+ Passed by reference.
+
Network device features:
%pNF 0x000000000000c000
Index: tip/lib/test_printf.c
===================================================================
--- tip.orig/lib/test_printf.c
+++ tip/lib/test_printf.c
@@ -20,6 +20,8 @@
#include <linux/gfp.h>
#include <linux/mm.h>
+#include <linux/kref.h>
+
#define BUF_SIZE 256
#define PAD_SIZE 16
#define FILL_CHAR '$'
@@ -462,6 +464,31 @@ flags(void)
kfree(cmp_buffer);
}
+/*
+ * Testcases for %pAr (atomic_t) and %pAk (struct kref) count printing:
+ */
+static void __init test_atomics__atomic_t(void)
+{
+ atomic_t count = ATOMIC_INIT(1);
+
+ test("1", "%pAr", &count);
+}
+
+static void __init test_atomics__kref(void)
+{
+ struct kref kref;
+
+ kref_init(&kref);
+
+ test("1", "%pAk", &kref);
+}
+
+static void __init test_atomics(void)
+{
+ test_atomics__atomic_t();
+ test_atomics__kref();
+}
+
static void __init
test_pointer(void)
{
@@ -481,6 +508,7 @@ test_pointer(void)
bitmap();
netdev_features();
flags();
+ test_atomics();
}
static int __init
Index: tip/lib/vsprintf.c
===================================================================
--- tip.orig/lib/vsprintf.c
+++ tip/lib/vsprintf.c
@@ -38,6 +38,8 @@
#include "../mm/internal.h" /* For the trace_print_flags arrays */
+#include <linux/kref.h>
+
#include <asm/page.h> /* for PAGE_SIZE */
#include <asm/sections.h> /* for dereference_function_descriptor() */
#include <asm/byteorder.h> /* cpu_to_le16 */
@@ -1470,6 +1472,40 @@ char *flags_string(char *buf, char *end,
return format_flags(buf, end, flags, names);
}
+static noinline_for_stack
+char *atomic_var(char *buf, char *end, void *atomic_ptr, const char *fmt)
+{
+ unsigned long num;
+ const struct printf_spec numspec = {
+ .flags = SPECIAL|SMALL,
+ .field_width = -1,
+ .precision = -1,
+ .base = 10,
+ };
+
+ switch (fmt[1]) {
+ case 'r':
+ {
+ atomic_t *count_p = (void *)atomic_ptr;
+
+ num = atomic_read(count_p);
+ break;
+ }
+ case 'k':
+ {
+ struct kref *kref_p = (void *)atomic_ptr;
+
+ num = atomic_read(&kref_p->refcount);
+ break;
+ }
+ default:
+ WARN_ONCE(1, "Unsupported atomics modifier: %c\n", fmt[1]);
+ return buf;
+ }
+
+ return number(buf, end, num, numspec);
+}
+
int kptr_restrict __read_mostly;
/*
@@ -1563,6 +1599,10 @@ int kptr_restrict __read_mostly;
* p page flags (see struct page) given as pointer to unsigned long
* g gfp flags (GFP_* and __GFP_*) given as pointer to gfp_t
* v vma flags (VM_*) given as pointer to unsigned long
+ * - 'A' For the count of atomic variables to be printed.
+ * Supported flags given by option:
+ * r atomic_t ('r'aw count)
+ * k struct kref ('k'ref count)
*
* ** Please update also Documentation/printk-formats.txt when making changes **
*
@@ -1718,6 +1758,8 @@ char *pointer(const char *fmt, char *buf
case 'G':
return flags_string(buf, end, ptr, fmt);
+ case 'A':
+ return atomic_var(buf, end, ptr, fmt);
}
spec.flags |= SMALL;
if (spec.field_width == -1) {
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2016-11-15 09:50 +0100 |
| Subject | [PATCH v2] printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref' |
| Message-ID | <sDHBg-2C3-35@gated-at.bofh.it> |
| In reply to | #1522441 |
* Ingo Molnar <mingo@kernel.org> wrote:
>
> * Greg KH <gregkh@linuxfoundation.org> wrote:
>
> > On Mon, Nov 14, 2016 at 10:16:55AM -0800, Christoph Hellwig wrote:
> > > On Mon, Nov 14, 2016 at 06:39:48PM +0100, Peter Zijlstra wrote:
> > > > Since we need to change the implementation, stop exposing internals.
> > > >
> > > > Provide kref_read() to read the current reference count; typically
> > > > used for debug messages.
> > >
> > > Can we just provide a printk specifier for a kref value instead as
> > > that is the only valid use case for reading the value?
> >
> > Yeah, that would be great as no one should be doing anything
> > logic-related based on the kref value.
>
> Find below a patch that implements %pAk for 'struct kref' count printing and
> %pAr for atomic_t counter printing.
>
> This is against vanilla upstream.
The patch below is against Peter's refcount series. Note that this patch depends
on this patch in Peter's series:
kref: Implement using refcount_t
Thanks,
Ingo
==================================>
Subject: printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref'
From: Ingo Molnar <mingo@kernel.org>
Date: Tue Nov 15 08:53:14 CET 2016
A decade of kref internals exposed to driver writers has proven that
exposing internals to them is a bad idea.
Make the bad patterns a bit easier to detect and allow cleaner
printouts by offering two new printk format string extensions:
%pAr - print the atomic_t count in decimal
%pAk - print the struct kref count in decimal
Also add printf testcases:
[ 0.328495] test_printf: all 268 tests passed
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
Documentation/printk-formats.txt | 10 +++++++++
lib/test_printf.c | 28 ++++++++++++++++++++++++++
lib/vsprintf.c | 42 +++++++++++++++++++++++++++++++++++++++
3 files changed, 80 insertions(+)
Index: tip/Documentation/printk-formats.txt
===================================================================
--- tip.orig/Documentation/printk-formats.txt
+++ tip/Documentation/printk-formats.txt
@@ -316,6 +316,16 @@ Flags bitfields such as page flags, gfp_
Passed by reference.
+atomic variables such atomic_t or struct kref:
+
+ %pAr atomic_t count
+ %pAk struct kref count
+
+ For printing the current count value of atomic variables. This is
+ preferred to accessing the counts directly.
+
+ Passed by reference.
+
Network device features:
%pNF 0x000000000000c000
Index: tip/lib/test_printf.c
===================================================================
--- tip.orig/lib/test_printf.c
+++ tip/lib/test_printf.c
@@ -20,6 +20,8 @@
#include <linux/gfp.h>
#include <linux/mm.h>
+#include <linux/kref.h>
+
#define BUF_SIZE 256
#define PAD_SIZE 16
#define FILL_CHAR '$'
@@ -462,6 +464,31 @@ flags(void)
kfree(cmp_buffer);
}
+/*
+ * Testcases for %pAr (atomic_t) and %pAk (struct kref) count printing:
+ */
+static void __init test_atomics__atomic_t(void)
+{
+ atomic_t count = ATOMIC_INIT(1);
+
+ test("1", "%pAr", &count);
+}
+
+static void __init test_atomics__kref(void)
+{
+ struct kref kref;
+
+ kref_init(&kref);
+
+ test("1", "%pAk", &kref);
+}
+
+static void __init test_atomics(void)
+{
+ test_atomics__atomic_t();
+ test_atomics__kref();
+}
+
static void __init
test_pointer(void)
{
@@ -481,6 +508,7 @@ test_pointer(void)
bitmap();
netdev_features();
flags();
+ test_atomics();
}
static int __init
Index: tip/lib/vsprintf.c
===================================================================
--- tip.orig/lib/vsprintf.c
+++ tip/lib/vsprintf.c
@@ -38,6 +38,8 @@
#include "../mm/internal.h" /* For the trace_print_flags arrays */
+#include <linux/kref.h>
+
#include <asm/page.h> /* for PAGE_SIZE */
#include <asm/sections.h> /* for dereference_function_descriptor() */
#include <asm/byteorder.h> /* cpu_to_le16 */
@@ -1470,6 +1472,40 @@ char *flags_string(char *buf, char *end,
return format_flags(buf, end, flags, names);
}
+static noinline_for_stack
+char *atomic_var(char *buf, char *end, void *atomic_ptr, const char *fmt)
+{
+ unsigned long num;
+ const struct printf_spec numspec = {
+ .flags = SPECIAL|SMALL,
+ .field_width = -1,
+ .precision = -1,
+ .base = 10,
+ };
+
+ switch (fmt[1]) {
+ case 'r':
+ {
+ atomic_t *count_p = (void *)atomic_ptr;
+
+ num = atomic_read(count_p);
+ break;
+ }
+ case 'k':
+ {
+ struct kref *kref_p = (void *)atomic_ptr;
+
+ num = refcount_read(&kref_p->refcount);
+ break;
+ }
+ default:
+ WARN_ONCE(1, "Unsupported atomics modifier: %c\n", fmt[1]);
+ return buf;
+ }
+
+ return number(buf, end, num, numspec);
+}
+
int kptr_restrict __read_mostly;
/*
@@ -1563,6 +1599,10 @@ int kptr_restrict __read_mostly;
* p page flags (see struct page) given as pointer to unsigned long
* g gfp flags (GFP_* and __GFP_*) given as pointer to gfp_t
* v vma flags (VM_*) given as pointer to unsigned long
+ * - 'A' For the count of atomic variables to be printed.
+ * Supported flags given by option:
+ * r atomic_t ('r'aw count)
+ * k struct kref ('k'ref count)
*
* ** Please update also Documentation/printk-formats.txt when making changes **
*
@@ -1718,6 +1758,8 @@ char *pointer(const char *fmt, char *buf
case 'G':
return flags_string(buf, end, ptr, fmt);
+ case 'A':
+ return atomic_var(buf, end, ptr, fmt);
}
spec.flags |= SMALL;
if (spec.field_width == -1) {
[toc] | [prev] | [next] | [standalone]
Page 3 of 5 — ← Prev page 1 2 [3] 4 5 Next page →
Back to top | Article view | linux.kernel
csiph-web