* [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops
2020-11-13 17:46 [PATCH v2 00/13] Introduce seqnum_ops Shuah Khan
@ 2020-11-13 17:46 ` Shuah Khan
2020-11-13 21:03 ` Matthew Wilcox
2020-11-16 14:53 ` Peter Zijlstra
2020-11-13 17:46 ` [PATCH v2 02/13] selftests: lib:test_seqnum_ops: add new test for seqnum_ops Shuah Khan
` (11 subsequent siblings)
12 siblings, 2 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw)
To: corbet, keescook, gregkh, peterz; +Cc: Shuah Khan, linux-doc, linux-kernel
Sequence Number api provides interfaces for unsigned atomic up counters
leveraging atomic_t and atomic64_t ops underneath.
There are a number of atomic_t usages in the kernel where atomic_t api
is used for counting sequence numbers and other statistical counters.
Several of these usages, convert atomic_read() and atomic_inc_return()
return values to unsigned. Introducing sequence number ops supports
these use-cases with a standard core-api.
The atomic_t api provides a wide range of atomic operations as a base
api to implement atomic counters, bitops, spinlock interfaces. The usages
also evolved into being used for resource lifetimes and state management.
The refcount_t api was introduced to address resource lifetime problems
related to atomic_t wrapping. There is a large overlap between the
atomic_t api used for resource lifetimes and just counters, stats, and
sequence numbers. It has become difficult to differentiate between the
atomic_t usages that should be converted to refcount_t and the ones that
can be left alone. Introducing seqnum_ops to wrap the usages that are
stats, counters, sequence numbers makes it easier for tools that scan
for underflow and overflow on atomic_t usages to detect overflow and
underflows to scan just the cases that are prone to errors.
In addition, to supporting sequence number use-cases, Sequence Number Ops
helps differentiate atomic_t counter usages from atomic_t usages that guard
object lifetimes, hence prone to overflow and underflow errors from up
counting use-cases.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
Documentation/core-api/atomic_ops.rst | 4 +
Documentation/core-api/index.rst | 1 +
Documentation/core-api/seqnum_ops.rst | 80 +++++++++++++++++
MAINTAINERS | 7 ++
include/linux/seqnum_ops.h | 116 +++++++++++++++++++++++++
lib/Kconfig | 9 ++
lib/Makefile | 1 +
lib/test_seqnum_ops.c | 119 ++++++++++++++++++++++++++
8 files changed, 337 insertions(+)
create mode 100644 Documentation/core-api/seqnum_ops.rst
create mode 100644 include/linux/seqnum_ops.h
create mode 100644 lib/test_seqnum_ops.c
diff --git a/Documentation/core-api/atomic_ops.rst b/Documentation/core-api/atomic_ops.rst
index 724583453e1f..762cbc0947e7 100644
--- a/Documentation/core-api/atomic_ops.rst
+++ b/Documentation/core-api/atomic_ops.rst
@@ -1,3 +1,7 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+.. _atomic_ops:
+
=======================================================
Semantics and Behavior of Atomic and Bitmask Operations
=======================================================
diff --git a/Documentation/core-api/index.rst b/Documentation/core-api/index.rst
index 69171b1799f2..be958afe757c 100644
--- a/Documentation/core-api/index.rst
+++ b/Documentation/core-api/index.rst
@@ -55,6 +55,7 @@ How Linux keeps everything from happening at the same time. See
atomic_ops
refcount-vs-atomic
+ seqnum_ops
irq/index
local_ops
padata
diff --git a/Documentation/core-api/seqnum_ops.rst b/Documentation/core-api/seqnum_ops.rst
new file mode 100644
index 000000000000..10b775a9ac05
--- /dev/null
+++ b/Documentation/core-api/seqnum_ops.rst
@@ -0,0 +1,80 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+.. include:: <isonum.txt>
+
+.. _seqnum_ops:
+
+==========================
+Sequence Number Operations
+==========================
+
+:Author: Shuah Khan
+:Copyright: |copy| 2020, The Linux Foundation
+:Copyright: |copy| 2020, Shuah Khan <skhan@linuxfoundation.org>
+
+Sequence Number api provides interfaces for unsigned up counters
+leveraging atomic_t and atomic64_t ops underneath.
+
+There are a number of atomic_t usages in the kernel where atomic_t api
+is used for counting sequence numbers and other statistical counters.
+Several of these usages, convert atomic_read() and atomic_inc_return()
+return values to unsigned. Introducing sequence number ops supports
+these use-cases with a standard core-api.
+
+The atomic_t api provides a wide range of atomic operations as a base
+api to implement atomic counters, bitops, spinlock interfaces. The usages
+also evolved into being used for resource lifetimes and state management.
+The refcount_t api was introduced to address resource lifetime problems
+related to atomic_t wrapping. There is a large overlap between the
+atomic_t api used for resource lifetimes and just counters, stats, and
+sequence numbers. It has become difficult to differentiate between the
+atomic_t usages that should be converted to refcount_t and the ones that
+can be left alone. Introducing seqnum_ops to wrap the usages that are
+stats, counters, sequence numbers makes it easier for tools that scan
+for underflow and overflow on atomic_t usages to detect overflow and
+underflows to scan just the cases that are prone to errors.
+
+In addition, to supporting sequence number use-cases, Sequence Number Ops
+helps differentiate atomic_t counter usages from atomic_t usages that guard
+object lifetimes, hence prone to overflow and underflow errors from up
+counting use-cases.
+
+Sequence Number Ops
+===================
+
+seqnum32 and seqnum64 types use atomic_t and atomic64_t underneath to
+leverage atomic_t api, to provide increment by 1 and return new value
+and fetch current value interfaces necessary to support unsigned up
+counters. ::
+
+ struct seqnum32 { atomic_t seqnum; };
+ struct seqnum64 { atomic64_t seqnum; };
+
+Please see :ref:`Documentation/core-api/atomic_ops.rst <atomic_ops>` for
+information on the Semantics and Behavior of Atomic operations.
+
+Initializers
+------------
+
+Interfaces for initializing sequence numbers are write operations which
+in turn invoke their ``ATOMIC_INIT() and atomic_set()`` counterparts ::
+
+ #define SEQNUM_INIT(i) { .seqnum = ATOMIC_INIT(i) }
+ seqnum32_init() --> atomic_set() to 0
+ seqnum64_init() --> atomic64_set() to 0
+
+Increment interface
+-------------------
+
+Increments sequence number and returns the new value. ::
+
+ seqnum32_inc_return() --> (u32) atomic_inc_return(seqnum)
+ seqnum64_inc_return() --> (u64) atomic64_inc_return(seqnum)
+
+Fetch interface
+---------------
+
+Fetched and returns current sequence number value. ::
+
+ seqnum32_fetch() --> (u32) atomic_add_return(0, seqnum)
+ seqnum64_fetch() --> (u64) atomic64_add_return(0, seqnum)
diff --git a/MAINTAINERS b/MAINTAINERS
index b516bb34a8d5..c83a6f05610b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15977,6 +15977,13 @@ S: Maintained
F: Documentation/fb/sm712fb.rst
F: drivers/video/fbdev/sm712*
+SEQNUM OPS
+M: Shuah Khan <skhan@linuxfoundation.org>
+L: linux-kernel@vger.kernel.org
+S: Maintained
+F: include/linux/seqnum_ops.h
+F: lib/test_seqnum_ops.c
+
SIMPLE FIRMWARE INTERFACE (SFI)
S: Obsolete
W: http://simplefirmware.org/
diff --git a/include/linux/seqnum_ops.h b/include/linux/seqnum_ops.h
new file mode 100644
index 000000000000..17d327b78050
--- /dev/null
+++ b/include/linux/seqnum_ops.h
@@ -0,0 +1,116 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * seqnum_ops.h - Interfaces for sequential and statistical counters.
+ *
+ * Copyright (c) 2020 Shuah Khan <skhan@linuxfoundation.org>
+ * Copyright (c) 2020 The Linux Foundation
+ *
+ * Sequence Number functions provide support for unsgined atomic up
+ * counters.
+ *
+ * The interface provides:
+ * seqnumu32 & seqnumu64 functions:
+ * initialization
+ * increment and return
+ * fetch and return
+ *
+ * seqnumu32 and seqnumu64 functions leverage/use atomic*_t ops to
+ * implement support for unsigned atomic up counters.
+ *
+ * Reference and API guide:
+ * Documentation/core-api/seqnum_ops.rst for more information.
+ */
+
+#ifndef __LINUX_SEQNUM_OPS_H
+#define __LINUX_SEQNUM_OPS_H
+
+#include <linux/atomic.h>
+
+/**
+ * struct seqnum32 - Sequential/Statistical atomic counter
+ * @seqnum: atomic_t
+ *
+ **/
+struct seqnum32 {
+ atomic_t seqnum;
+};
+
+#define SEQNUM_INIT(i) { .seqnum = ATOMIC_INIT(i) }
+
+/*
+ * seqnum32_init() - initialize seqnum value
+ * @seq: struct seqnum32 pointer
+ *
+ */
+static inline void seqnum32_init(struct seqnum32 *seq)
+{
+ atomic_set(&seq->seqnum, 0);
+}
+
+/*
+ * seqnum32_inc_return() - increment seqnum value and return the new value
+ * @seq: struct seqnum32 pointer
+ *
+ * Return u32
+ */
+static inline u32 seqnum32_inc_return(struct seqnum32 *seq)
+{
+ return (u32) atomic_inc_return(&seq->seqnum);
+}
+
+/*
+ * seqnum32_fetch() - return the current value
+ * @seq: struct seqnum32 pointer
+ *
+ * Return u32
+ */
+static inline u32 seqnum32_fetch(struct seqnum32 *seq)
+{
+ return (u32) atomic_add_return(0, &seq->seqnum);
+}
+
+#ifdef CONFIG_64BIT
+/* atomic64_t is defined in CONFIG_64BIT in include/linux/types.h */
+/*
+ * struct seqnum64 - Sequential/Statistical atomic counter
+ * @seq: atomic64_t
+ *
+ */
+struct seqnum64 {
+ atomic64_t seqnum;
+};
+
+/*
+ * seqnum64_init() - initialize seqnum value
+ * @seq: struct seqnum64 pointer
+ *
+ */
+static inline void seqnum64_init(struct seqnum64 *seq)
+{
+ atomic64_set(&seq->seqnum, 0);
+}
+
+/*
+ * seqnum64_inc() - increment seqnum value and return the new value
+ * @seq: struct seqnum64 pointer
+ *
+ * Return u64
+ */
+static inline u64 seqnum64_inc_return(struct seqnum64 *seq)
+{
+ return (u64) atomic64_inc_return(&seq->seqnum);
+}
+
+/*
+ * seqnum64_fetch() - return the current value
+ * @seq: struct seqnum64 pointer
+ *
+ * Return u64
+ */
+static inline u64 seqnum64_fetch(struct seqnum64 *seq)
+{
+ return (u64) atomic64_add_return(0, &seq->seqnum);
+}
+#endif /* CONFIG_64BIT */
+
+#endif /* __LINUX_SEQNUM_OPS_H */
diff --git a/lib/Kconfig b/lib/Kconfig
index b46a9fd122c8..c362c2713e11 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -663,6 +663,15 @@ config OBJAGG
config STRING_SELFTEST
tristate "Test string functions"
+config TEST_SEQNUM_OPS
+ tristate "Test Sequence Number Ops API"
+ help
+ A test module for Sequence Number Ops API. A corresponding
+ selftest can be used to test the Seqnum Ops API. Select this
+ for testing Sequence Number Ops API.
+
+ See Documentation/core-api/seqnum_ops.rst
+
endmenu
config GENERIC_IOREMAP
diff --git a/lib/Makefile b/lib/Makefile
index ce45af50983a..7d17c25e4d73 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_TEST_MEMINIT) += test_meminit.o
obj-$(CONFIG_TEST_LOCKUP) += test_lockup.o
obj-$(CONFIG_TEST_HMM) += test_hmm.o
obj-$(CONFIG_TEST_FREE_PAGES) += test_free_pages.o
+obj-$(CONFIG_TEST_SEQNUM_OPS) += test_seqnum_ops.o
#
# CFLAGS for compiling floating point code inside the kernel. x86/Makefile turns
diff --git a/lib/test_seqnum_ops.c b/lib/test_seqnum_ops.c
new file mode 100644
index 000000000000..6e58b6a0a2be
--- /dev/null
+++ b/lib/test_seqnum_ops.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * test_seqnum_ops.c - Kernel module for testing Seqnum API
+ *
+ * Copyright (c) 2020 Shuah Khan <skhan@linuxfoundation.org>
+ * Copyright (c) 2020 The Linux Foundation
+ *
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/seqnum_ops.h>
+
+static inline void
+test_seqnum32_result(char *msg, u32 start, u32 end, u32 expected)
+{
+ pr_info("%s: %u to %u - %s\n",
+ msg, start, end,
+ ((expected == end) ? "PASS" : "FAIL"));
+}
+
+
+static void test_seqnum32(void)
+{
+ static struct seqnum32 seq = SEQNUM_INIT(0);
+ u32 start_val = seqnum32_fetch(&seq);
+ u32 end_val;
+
+ end_val = seqnum32_inc_return(&seq);
+ test_seqnum32_result("Test fetch and increment",
+ start_val, end_val, start_val+1);
+
+ start_val = seqnum32_fetch(&seq);
+ seqnum32_init(&seq);
+ end_val = seqnum32_fetch(&seq);
+ test_seqnum32_result("Test init", start_val, end_val, 0);
+
+}
+
+static void test_seqnum32_overflow(void)
+{
+ static struct seqnum32 seq = SEQNUM_INIT(UINT_MAX-1);
+ u32 start_val = seqnum32_fetch(&seq);
+ u32 end_val = seqnum32_inc_return(&seq);
+
+ test_seqnum32_result("Test UINT_MAX limit compare with (val+1)",
+ start_val, end_val, start_val+1);
+
+ test_seqnum32_result("Test UINT_MAX limit compare with (UINT_MAX)",
+ start_val, end_val, UINT_MAX);
+}
+
+#ifdef CONFIG_64BIT
+static inline void
+test_seqnum64_result(char *msg, u64 start, u64 end, u64 expected)
+{
+ pr_info("%s: %llu to %llu - %s\n",
+ msg, start, end,
+ ((expected == end) ? "PASS" : "FAIL"));
+}
+
+static void test_seqnum64(void)
+{
+ static struct seqnum64 seq = SEQNUM_INIT(0);
+ u64 start_val = seqnum64_fetch(&seq);
+ u64 end_val;
+
+ end_val = seqnum64_inc_return(&seq);
+ test_seqnum64_result("Test fetch and increment",
+ start_val, end_val, start_val+1);
+
+ start_val = seqnum64_fetch(&seq);
+ seqnum64_init(&seq);
+ end_val = seqnum64_fetch(&seq);
+ test_seqnum64_result("Test init", start_val, end_val, 0);
+}
+
+static void test_seqnum64_overflow(void)
+{
+ static struct seqnum64 seq = SEQNUM_INIT(UINT_MAX-1);
+ u64 start_val = seqnum64_fetch(&seq);
+ u64 end_val = seqnum64_inc_return(&seq);
+
+ test_seqnum64_result("Test UINT_MAX limit compare with (val+1)",
+ start_val, end_val, start_val+1);
+ test_seqnum64_result("Test UINT_MAX limit compare with (UINT_MAX)",
+ start_val, end_val, UINT_MAX);
+}
+#endif /* CONFIG_64BIT */
+
+static int __init test_seqnum_ops_init(void)
+{
+ pr_info("Start seqnum32_*() interfaces test\n");
+ test_seqnum32();
+ test_seqnum32_overflow();
+ pr_info("End seqnum32_*() interfaces test\n\n");
+
+#ifdef CONFIG_64BIT
+ pr_info("Start seqnum64_*() interfaces test\n");
+ test_seqnum64();
+ test_seqnum64_overflow();
+ pr_info("End seqnum64_*() interfaces test\n\n");
+#endif /* CONFIG_64BIT */
+
+ return 0;
+}
+
+module_init(test_seqnum_ops_init);
+
+static void __exit test_seqnum_ops_exit(void)
+{
+ pr_info("exiting.\n");
+}
+
+module_exit(test_seqnum_ops_exit);
+
+MODULE_AUTHOR("Shuah Khan <skhan@linuxfoundation.org>");
+MODULE_LICENSE("GPL v2");
--
2.27.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops
2020-11-13 17:46 ` [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops Shuah Khan
@ 2020-11-13 21:03 ` Matthew Wilcox
2020-11-16 14:49 ` Peter Zijlstra
2020-11-17 16:34 ` Shuah Khan
2020-11-16 14:53 ` Peter Zijlstra
1 sibling, 2 replies; 30+ messages in thread
From: Matthew Wilcox @ 2020-11-13 21:03 UTC (permalink / raw)
To: Shuah Khan; +Cc: corbet, keescook, gregkh, peterz, linux-doc, linux-kernel
> +==========================
> +Sequence Number Operations
> +==========================
> +
> +:Author: Shuah Khan
> +:Copyright: |copy| 2020, The Linux Foundation
> +:Copyright: |copy| 2020, Shuah Khan <skhan@linuxfoundation.org>
> +
> +Sequence Number api provides interfaces for unsigned up counters
> +leveraging atomic_t and atomic64_t ops underneath.
As I said last time you posted this, the documentation is all
back-to-front. You're describing what it isn't, not what it is.
> +There are a number of atomic_t usages in the kernel where atomic_t api
> +is used for counting sequence numbers and other statistical counters.
> +Several of these usages, convert atomic_read() and atomic_inc_return()
> +return values to unsigned. Introducing sequence number ops supports
> +these use-cases with a standard core-api.
> +
> +The atomic_t api provides a wide range of atomic operations as a base
> +api to implement atomic counters, bitops, spinlock interfaces. The usages
> +also evolved into being used for resource lifetimes and state management.
> +The refcount_t api was introduced to address resource lifetime problems
> +related to atomic_t wrapping. There is a large overlap between the
> +atomic_t api used for resource lifetimes and just counters, stats, and
> +sequence numbers. It has become difficult to differentiate between the
> +atomic_t usages that should be converted to refcount_t and the ones that
> +can be left alone. Introducing seqnum_ops to wrap the usages that are
> +stats, counters, sequence numbers makes it easier for tools that scan
> +for underflow and overflow on atomic_t usages to detect overflow and
> +underflows to scan just the cases that are prone to errors.
> +
> +In addition, to supporting sequence number use-cases, Sequence Number Ops
> +helps differentiate atomic_t counter usages from atomic_t usages that guard
> +object lifetimes, hence prone to overflow and underflow errors from up
> +counting use-cases.
I think almost all of this information should go into atomic_ops.rst
pushing people towards using the other APIs instead of atomic_t.
Someone who already landed here doesn't want to read about refcount_t.
They want to know what a seqnum_t is and how to use it.
> +Sequence Number Ops
> +===================
> +
> +seqnum32 and seqnum64 types use atomic_t and atomic64_t underneath to
Don't talk about the implementation.
> +leverage atomic_t api, to provide increment by 1 and return new value
> +and fetch current value interfaces necessary to support unsigned up
> +counters. ::
> +
> + struct seqnum32 { atomic_t seqnum; };
> + struct seqnum64 { atomic64_t seqnum; };
> +
> +Please see :ref:`Documentation/core-api/atomic_ops.rst <atomic_ops>` for
> +information on the Semantics and Behavior of Atomic operations.
> +
> +Initializers
> +------------
> +
> +Interfaces for initializing sequence numbers are write operations which
> +in turn invoke their ``ATOMIC_INIT() and atomic_set()`` counterparts ::
> +
> + #define SEQNUM_INIT(i) { .seqnum = ATOMIC_INIT(i) }
> + seqnum32_init() --> atomic_set() to 0
> + seqnum64_init() --> atomic64_set() to 0
> +
> +Increment interface
> +-------------------
> +
> +Increments sequence number and returns the new value. ::
> +
> + seqnum32_inc_return() --> (u32) atomic_inc_return(seqnum)
> + seqnum64_inc_return() --> (u64) atomic64_inc_return(seqnum)
seqnum_inc() should just return the new value -- seqnum_inc_return is
too verbose. And do we not need a seqnum_add()?
Also, this would be a good point to talk about behaviour on overflow.
> +Fetch interface
> +---------------
> +
> +Fetched and returns current sequence number value. ::
> +
> + seqnum32_fetch() --> (u32) atomic_add_return(0, seqnum)
> + seqnum64_fetch() --> (u64) atomic64_add_return(0, seqnum)
Er ... why would you force an atomic operation instead of atomic_read()?
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b516bb34a8d5..c83a6f05610b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15977,6 +15977,13 @@ S: Maintained
> F: Documentation/fb/sm712fb.rst
> F: drivers/video/fbdev/sm712*
>
> +SEQNUM OPS
> +M: Shuah Khan <skhan@linuxfoundation.org>
> +L: linux-kernel@vger.kernel.org
> +S: Maintained
> +F: include/linux/seqnum_ops.h
> +F: lib/test_seqnum_ops.c
> +
> SIMPLE FIRMWARE INTERFACE (SFI)
> S: Obsolete
> W: http://simplefirmware.org/
> diff --git a/include/linux/seqnum_ops.h b/include/linux/seqnum_ops.h
> new file mode 100644
> index 000000000000..17d327b78050
> --- /dev/null
> +++ b/include/linux/seqnum_ops.h
> @@ -0,0 +1,116 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * seqnum_ops.h - Interfaces for sequential and statistical counters.
> + *
> + * Copyright (c) 2020 Shuah Khan <skhan@linuxfoundation.org>
> + * Copyright (c) 2020 The Linux Foundation
> + *
> + * Sequence Number functions provide support for unsgined atomic up
> + * counters.
> + *
> + * The interface provides:
> + * seqnumu32 & seqnumu64 functions:
> + * initialization
> + * increment and return
> + * fetch and return
> + *
> + * seqnumu32 and seqnumu64 functions leverage/use atomic*_t ops to
> + * implement support for unsigned atomic up counters.
> + *
> + * Reference and API guide:
> + * Documentation/core-api/seqnum_ops.rst for more information.
> + */
> +
> +#ifndef __LINUX_SEQNUM_OPS_H
> +#define __LINUX_SEQNUM_OPS_H
> +
> +#include <linux/atomic.h>
> +
> +/**
> + * struct seqnum32 - Sequential/Statistical atomic counter
> + * @seqnum: atomic_t
> + *
> + **/
> +struct seqnum32 {
> + atomic_t seqnum;
> +};
> +
> +#define SEQNUM_INIT(i) { .seqnum = ATOMIC_INIT(i) }
> +
> +/*
> + * seqnum32_init() - initialize seqnum value
> + * @seq: struct seqnum32 pointer
> + *
> + */
> +static inline void seqnum32_init(struct seqnum32 *seq)
> +{
> + atomic_set(&seq->seqnum, 0);
> +}
> +
> +/*
> + * seqnum32_inc_return() - increment seqnum value and return the new value
> + * @seq: struct seqnum32 pointer
> + *
> + * Return u32
> + */
> +static inline u32 seqnum32_inc_return(struct seqnum32 *seq)
> +{
> + return (u32) atomic_inc_return(&seq->seqnum);
> +}
> +
> +/*
> + * seqnum32_fetch() - return the current value
> + * @seq: struct seqnum32 pointer
> + *
> + * Return u32
> + */
> +static inline u32 seqnum32_fetch(struct seqnum32 *seq)
> +{
> + return (u32) atomic_add_return(0, &seq->seqnum);
> +}
> +
> +#ifdef CONFIG_64BIT
> +/* atomic64_t is defined in CONFIG_64BIT in include/linux/types.h */
> +/*
> + * struct seqnum64 - Sequential/Statistical atomic counter
> + * @seq: atomic64_t
> + *
> + */
> +struct seqnum64 {
> + atomic64_t seqnum;
> +};
> +
> +/*
> + * seqnum64_init() - initialize seqnum value
> + * @seq: struct seqnum64 pointer
> + *
> + */
> +static inline void seqnum64_init(struct seqnum64 *seq)
> +{
> + atomic64_set(&seq->seqnum, 0);
> +}
> +
> +/*
> + * seqnum64_inc() - increment seqnum value and return the new value
> + * @seq: struct seqnum64 pointer
> + *
> + * Return u64
> + */
> +static inline u64 seqnum64_inc_return(struct seqnum64 *seq)
> +{
> + return (u64) atomic64_inc_return(&seq->seqnum);
> +}
> +
> +/*
> + * seqnum64_fetch() - return the current value
> + * @seq: struct seqnum64 pointer
> + *
> + * Return u64
> + */
> +static inline u64 seqnum64_fetch(struct seqnum64 *seq)
> +{
> + return (u64) atomic64_add_return(0, &seq->seqnum);
> +}
> +#endif /* CONFIG_64BIT */
> +
> +#endif /* __LINUX_SEQNUM_OPS_H */
> diff --git a/lib/Kconfig b/lib/Kconfig
> index b46a9fd122c8..c362c2713e11 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -663,6 +663,15 @@ config OBJAGG
> config STRING_SELFTEST
> tristate "Test string functions"
>
> +config TEST_SEQNUM_OPS
> + tristate "Test Sequence Number Ops API"
> + help
> + A test module for Sequence Number Ops API. A corresponding
> + selftest can be used to test the Seqnum Ops API. Select this
> + for testing Sequence Number Ops API.
> +
> + See Documentation/core-api/seqnum_ops.rst
> +
> endmenu
>
> config GENERIC_IOREMAP
> diff --git a/lib/Makefile b/lib/Makefile
> index ce45af50983a..7d17c25e4d73 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -101,6 +101,7 @@ obj-$(CONFIG_TEST_MEMINIT) += test_meminit.o
> obj-$(CONFIG_TEST_LOCKUP) += test_lockup.o
> obj-$(CONFIG_TEST_HMM) += test_hmm.o
> obj-$(CONFIG_TEST_FREE_PAGES) += test_free_pages.o
> +obj-$(CONFIG_TEST_SEQNUM_OPS) += test_seqnum_ops.o
>
> #
> # CFLAGS for compiling floating point code inside the kernel. x86/Makefile turns
> diff --git a/lib/test_seqnum_ops.c b/lib/test_seqnum_ops.c
> new file mode 100644
> index 000000000000..6e58b6a0a2be
> --- /dev/null
> +++ b/lib/test_seqnum_ops.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * test_seqnum_ops.c - Kernel module for testing Seqnum API
> + *
> + * Copyright (c) 2020 Shuah Khan <skhan@linuxfoundation.org>
> + * Copyright (c) 2020 The Linux Foundation
> + *
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/module.h>
> +#include <linux/seqnum_ops.h>
> +
> +static inline void
> +test_seqnum32_result(char *msg, u32 start, u32 end, u32 expected)
> +{
> + pr_info("%s: %u to %u - %s\n",
> + msg, start, end,
> + ((expected == end) ? "PASS" : "FAIL"));
> +}
> +
> +
> +static void test_seqnum32(void)
> +{
> + static struct seqnum32 seq = SEQNUM_INIT(0);
> + u32 start_val = seqnum32_fetch(&seq);
> + u32 end_val;
> +
> + end_val = seqnum32_inc_return(&seq);
> + test_seqnum32_result("Test fetch and increment",
> + start_val, end_val, start_val+1);
> +
> + start_val = seqnum32_fetch(&seq);
> + seqnum32_init(&seq);
> + end_val = seqnum32_fetch(&seq);
> + test_seqnum32_result("Test init", start_val, end_val, 0);
> +
> +}
> +
> +static void test_seqnum32_overflow(void)
> +{
> + static struct seqnum32 seq = SEQNUM_INIT(UINT_MAX-1);
> + u32 start_val = seqnum32_fetch(&seq);
> + u32 end_val = seqnum32_inc_return(&seq);
> +
> + test_seqnum32_result("Test UINT_MAX limit compare with (val+1)",
> + start_val, end_val, start_val+1);
> +
> + test_seqnum32_result("Test UINT_MAX limit compare with (UINT_MAX)",
> + start_val, end_val, UINT_MAX);
> +}
> +
> +#ifdef CONFIG_64BIT
> +static inline void
> +test_seqnum64_result(char *msg, u64 start, u64 end, u64 expected)
> +{
> + pr_info("%s: %llu to %llu - %s\n",
> + msg, start, end,
> + ((expected == end) ? "PASS" : "FAIL"));
> +}
> +
> +static void test_seqnum64(void)
> +{
> + static struct seqnum64 seq = SEQNUM_INIT(0);
> + u64 start_val = seqnum64_fetch(&seq);
> + u64 end_val;
> +
> + end_val = seqnum64_inc_return(&seq);
> + test_seqnum64_result("Test fetch and increment",
> + start_val, end_val, start_val+1);
> +
> + start_val = seqnum64_fetch(&seq);
> + seqnum64_init(&seq);
> + end_val = seqnum64_fetch(&seq);
> + test_seqnum64_result("Test init", start_val, end_val, 0);
> +}
> +
> +static void test_seqnum64_overflow(void)
> +{
> + static struct seqnum64 seq = SEQNUM_INIT(UINT_MAX-1);
> + u64 start_val = seqnum64_fetch(&seq);
> + u64 end_val = seqnum64_inc_return(&seq);
> +
> + test_seqnum64_result("Test UINT_MAX limit compare with (val+1)",
> + start_val, end_val, start_val+1);
> + test_seqnum64_result("Test UINT_MAX limit compare with (UINT_MAX)",
> + start_val, end_val, UINT_MAX);
> +}
> +#endif /* CONFIG_64BIT */
> +
> +static int __init test_seqnum_ops_init(void)
> +{
> + pr_info("Start seqnum32_*() interfaces test\n");
> + test_seqnum32();
> + test_seqnum32_overflow();
> + pr_info("End seqnum32_*() interfaces test\n\n");
> +
> +#ifdef CONFIG_64BIT
> + pr_info("Start seqnum64_*() interfaces test\n");
> + test_seqnum64();
> + test_seqnum64_overflow();
> + pr_info("End seqnum64_*() interfaces test\n\n");
> +#endif /* CONFIG_64BIT */
> +
> + return 0;
> +}
> +
> +module_init(test_seqnum_ops_init);
> +
> +static void __exit test_seqnum_ops_exit(void)
> +{
> + pr_info("exiting.\n");
> +}
> +
> +module_exit(test_seqnum_ops_exit);
> +
> +MODULE_AUTHOR("Shuah Khan <skhan@linuxfoundation.org>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops
2020-11-13 21:03 ` Matthew Wilcox
@ 2020-11-16 14:49 ` Peter Zijlstra
2020-11-16 14:58 ` Peter Zijlstra
2020-11-17 16:34 ` Shuah Khan
1 sibling, 1 reply; 30+ messages in thread
From: Peter Zijlstra @ 2020-11-16 14:49 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Shuah Khan, corbet, keescook, gregkh, linux-doc, linux-kernel
On Fri, Nov 13, 2020 at 09:03:27PM +0000, Matthew Wilcox wrote:
> I think almost all of this information should go into atomic_ops.rst
No, we should delete atomic_ops.rst. It's bitrotted nonsense. The only
reason it still exists is because I can't seem to get around to
verifying we've actually covered everything in the new documentation:
Documentation/atomic_*.txt
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops
2020-11-16 14:49 ` Peter Zijlstra
@ 2020-11-16 14:58 ` Peter Zijlstra
2020-11-17 14:50 ` Shuah Khan
0 siblings, 1 reply; 30+ messages in thread
From: Peter Zijlstra @ 2020-11-16 14:58 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Shuah Khan, corbet, keescook, gregkh, linux-doc, linux-kernel
On Mon, Nov 16, 2020 at 03:49:14PM +0100, Peter Zijlstra wrote:
> On Fri, Nov 13, 2020 at 09:03:27PM +0000, Matthew Wilcox wrote:
> > I think almost all of this information should go into atomic_ops.rst
>
> No, we should delete atomic_ops.rst. It's bitrotted nonsense. The only
Sod it, I'll just queue a deletion. That'll stop people from trying to
update the trainwreck.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops
2020-11-16 14:58 ` Peter Zijlstra
@ 2020-11-17 14:50 ` Shuah Khan
2020-11-17 15:21 ` Peter Zijlstra
0 siblings, 1 reply; 30+ messages in thread
From: Shuah Khan @ 2020-11-17 14:50 UTC (permalink / raw)
To: Peter Zijlstra, Matthew Wilcox
Cc: corbet, keescook, gregkh, linux-doc, linux-kernel, Shuah Khan
On 11/16/20 7:58 AM, Peter Zijlstra wrote:
> On Mon, Nov 16, 2020 at 03:49:14PM +0100, Peter Zijlstra wrote:
>> On Fri, Nov 13, 2020 at 09:03:27PM +0000, Matthew Wilcox wrote:
>>> I think almost all of this information should go into atomic_ops.rst
>>
>> No, we should delete atomic_ops.rst. It's bitrotted nonsense. The only
>
> Sod it, I'll just queue a deletion. That'll stop people from trying to
> update the trainwreck.
>
Please don't delete the document. This is one of the documents that has
the information to make decisions about the api usage.
atomic_t information is spread out in various Doc directories. It should
be consolidated, instead of removing files.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops
2020-11-17 14:50 ` Shuah Khan
@ 2020-11-17 15:21 ` Peter Zijlstra
0 siblings, 0 replies; 30+ messages in thread
From: Peter Zijlstra @ 2020-11-17 15:21 UTC (permalink / raw)
To: Shuah Khan
Cc: Matthew Wilcox, corbet, keescook, gregkh, linux-doc, linux-kernel
On Tue, Nov 17, 2020 at 07:50:15AM -0700, Shuah Khan wrote:
> On 11/16/20 7:58 AM, Peter Zijlstra wrote:
> > On Mon, Nov 16, 2020 at 03:49:14PM +0100, Peter Zijlstra wrote:
> > > On Fri, Nov 13, 2020 at 09:03:27PM +0000, Matthew Wilcox wrote:
> > > > I think almost all of this information should go into atomic_ops.rst
> > >
> > > No, we should delete atomic_ops.rst. It's bitrotted nonsense. The only
> >
> > Sod it, I'll just queue a deletion. That'll stop people from trying to
> > update the trainwreck.
> >
>
> Please don't delete the document. This is one of the documents that has
> the information to make decisions about the api usage.
>
> atomic_t information is spread out in various Doc directories. It should
> be consolidated, instead of removing files.
It is gone.. Please read Documentation/atomic_*.txt if you want to know
about atomic_t or bitops.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops
2020-11-13 21:03 ` Matthew Wilcox
2020-11-16 14:49 ` Peter Zijlstra
@ 2020-11-17 16:34 ` Shuah Khan
2020-11-17 17:38 ` Matthew Wilcox
1 sibling, 1 reply; 30+ messages in thread
From: Shuah Khan @ 2020-11-17 16:34 UTC (permalink / raw)
To: Matthew Wilcox
Cc: corbet, keescook, gregkh, peterz, linux-doc, linux-kernel, Shuah Khan
On 11/13/20 2:03 PM, Matthew Wilcox wrote:
>> +==========================
>> +Sequence Number Operations
>> +==========================
>> +
>> +:Author: Shuah Khan
>> +:Copyright: |copy| 2020, The Linux Foundation
>> +:Copyright: |copy| 2020, Shuah Khan <skhan@linuxfoundation.org>
>> +
>> +Sequence Number api provides interfaces for unsigned up counters
>> +leveraging atomic_t and atomic64_t ops underneath.
>
> As I said last time you posted this, the documentation is all
> back-to-front. You're describing what it isn't, not what it is.
>
I will rephrase it to read better.
>> +There are a number of atomic_t usages in the kernel where atomic_t api
>> +is used for counting sequence numbers and other statistical counters.
>> +Several of these usages, convert atomic_read() and atomic_inc_return()
>> +return values to unsigned. Introducing sequence number ops supports
>> +these use-cases with a standard core-api.
>> +
>> +The atomic_t api provides a wide range of atomic operations as a base
>> +api to implement atomic counters, bitops, spinlock interfaces. The usages
>> +also evolved into being used for resource lifetimes and state management.
>> +The refcount_t api was introduced to address resource lifetime problems
>> +related to atomic_t wrapping. There is a large overlap between the
>> +atomic_t api used for resource lifetimes and just counters, stats, and
>> +sequence numbers. It has become difficult to differentiate between the
>> +atomic_t usages that should be converted to refcount_t and the ones that
>> +can be left alone. Introducing seqnum_ops to wrap the usages that are
>> +stats, counters, sequence numbers makes it easier for tools that scan
>> +for underflow and overflow on atomic_t usages to detect overflow and
>> +underflows to scan just the cases that are prone to errors.
>> +
>> +In addition, to supporting sequence number use-cases, Sequence Number Ops
>> +helps differentiate atomic_t counter usages from atomic_t usages that guard
>> +object lifetimes, hence prone to overflow and underflow errors from up
>> +counting use-cases.
>
> I think almost all of this information should go into atomic_ops.rst
> pushing people towards using the other APIs instead of atomic_t.
> Someone who already landed here doesn't want to read about refcount_t.
> They want to know what a seqnum_t is and how to use it.
>
Looks like this is resolved with atomic_ops.rst is now gone.
>> +Sequence Number Ops
>> +===================
>> +
>> +seqnum32 and seqnum64 types use atomic_t and atomic64_t underneath to
>
> Don't talk about the implementation.
>
>> +leverage atomic_t api, to provide increment by 1 and return new value
>> +and fetch current value interfaces necessary to support unsigned up
>> +counters. ::
>> +
>> + struct seqnum32 { atomic_t seqnum; };
>> + struct seqnum64 { atomic64_t seqnum; };
>> +
>> +Please see :ref:`Documentation/core-api/atomic_ops.rst <atomic_ops>` for
>> +information on the Semantics and Behavior of Atomic operations.
>> +
>> +Initializers
>> +------------
>> +
>> +Interfaces for initializing sequence numbers are write operations which
>> +in turn invoke their ``ATOMIC_INIT() and atomic_set()`` counterparts ::
>> +
>> + #define SEQNUM_INIT(i) { .seqnum = ATOMIC_INIT(i) }
>> + seqnum32_init() --> atomic_set() to 0
>> + seqnum64_init() --> atomic64_set() to 0
>> +
>> +Increment interface
>> +-------------------
>> +
>> +Increments sequence number and returns the new value. ::
>> +
>> + seqnum32_inc_return() --> (u32) atomic_inc_return(seqnum)
>> + seqnum64_inc_return() --> (u64) atomic64_inc_return(seqnum)
>
> seqnum_inc() should just return the new value -- seqnum_inc_return is
> too verbose. And do we not need a seqnum_add()?
>
I had the patch series with seqnum_inc() all ready to go and then
revisited the choice. My thinking is that matching the current atomic
api that has _inc() and inc_return() might be less confusing. That
being said, I have no problems with making just _inc(). The reason
for 32 and 64 appended is based on comments that it including size
in the api makes it very clear.
No need for atomic_add() - inc_return() is sufficient for this use-case.
> Also, this would be a good point to talk about behaviour on overflow.
>
I can add some overflow information.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops
2020-11-17 16:34 ` Shuah Khan
@ 2020-11-17 17:38 ` Matthew Wilcox
2020-11-17 18:23 ` Shuah Khan
2020-11-17 18:24 ` Shuah Khan
0 siblings, 2 replies; 30+ messages in thread
From: Matthew Wilcox @ 2020-11-17 17:38 UTC (permalink / raw)
To: Shuah Khan; +Cc: corbet, keescook, gregkh, peterz, linux-doc, linux-kernel
On Tue, Nov 17, 2020 at 09:34:24AM -0700, Shuah Khan wrote:
> > seqnum_inc() should just return the new value -- seqnum_inc_return is
> > too verbose. And do we not need a seqnum_add()?
>
> I had the patch series with seqnum_inc() all ready to go and then
> revisited the choice. My thinking is that matching the current atomic
> api that has _inc() and inc_return() might be less confusing. That
No, it's more confusing. I know you're converting things from using
atomic_t, but you really need to think about this in terms of "What
makes sense for this API". Unless you really want to have inc that
returns void and inc_return that returns the new value, having only
inc_return makes no sense.
> being said, I have no problems with making just _inc(). The reason
> for 32 and 64 appended is based on comments that it including size
> in the api makes it very clear.
By putting 32 and 64 in the name of the API, I would contend you're making
people think about something that they should not need to think about.
> No need for atomic_add() - inc_return() is sufficient for this use-case.
I haven't looked at the various potential users of this API, but there
are often cases where we account, eg, number of bytes transmitted.
There are also cases where read-and-zero would be a useful operation
to have. I'm thinking about sampling statistics.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops
2020-11-17 17:38 ` Matthew Wilcox
@ 2020-11-17 18:23 ` Shuah Khan
2020-11-17 18:24 ` Shuah Khan
1 sibling, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-17 18:23 UTC (permalink / raw)
To: Matthew Wilcox
Cc: corbet, keescook, gregkh, peterz, linux-doc, linux-kernel, Shuah Khan
On 11/17/20 10:38 AM, Matthew Wilcox wrote:
> On Tue, Nov 17, 2020 at 09:34:24AM -0700, Shuah Khan wrote:
>>> seqnum_inc() should just return the new value -- seqnum_inc_return is
>>> too verbose. And do we not need a seqnum_add()?
>>
>> I had the patch series with seqnum_inc() all ready to go and then
>> revisited the choice. My thinking is that matching the current atomic
>> api that has _inc() and inc_return() might be less confusing. That
>
> No, it's more confusing. I know you're converting things from using
> atomic_t, but you really need to think about this in terms of "What
> makes sense for this API". Unless you really want to have inc that
> returns void and inc_return that returns the new value, having only
> inc_return makes no sense.
>
I am fine with that. As I said I have a patch series saved with just
seqnum_inc() that increments and returns. I anticipated people would
have problems with seqnum_inc() that returns. :)
>> being said, I have no problems with making just _inc(). The reason
>> for 32 and 64 appended is based on comments that it including size
>> in the api makes it very clear.
>
> By putting 32 and 64 in the name of the API, I would contend you're making
> people think about something that they should not need to think about.
>
Are you recommending seqnum32_*() for 32bit and seqnum_*() for 64bit
which would make 64bit as a default? We have to make a distinction
for 32bit vs. 64-bit api.
>> No need for atomic_add() - inc_return() is sufficient for this use-case.
>
> I haven't looked at the various potential users of this API, but there
> are often cases where we account, eg, number of bytes transmitted.
>
> There are also cases where read-and-zero would be a useful operation
> to have. I'm thinking about sampling statistics.
>
The idea is isolating sequence number use-case first and restrict this
api for that.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops
2020-11-17 17:38 ` Matthew Wilcox
2020-11-17 18:23 ` Shuah Khan
@ 2020-11-17 18:24 ` Shuah Khan
1 sibling, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-17 18:24 UTC (permalink / raw)
To: Matthew Wilcox
Cc: corbet, keescook, gregkh, peterz, linux-doc, linux-kernel, Shuah Khan
On 11/17/20 10:38 AM, Matthew Wilcox wrote:
> On Tue, Nov 17, 2020 at 09:34:24AM -0700, Shuah Khan wrote:
>>> seqnum_inc() should just return the new value -- seqnum_inc_return is
>>> too verbose. And do we not need a seqnum_add()?
>>
>> I had the patch series with seqnum_inc() all ready to go and then
>> revisited the choice. My thinking is that matching the current atomic
>> api that has _inc() and inc_return() might be less confusing. That
>
> No, it's more confusing. I know you're converting things from using
> atomic_t, but you really need to think about this in terms of "What
> makes sense for this API". Unless you really want to have inc that
> returns void and inc_return that returns the new value, having only
> inc_return makes no sense.
>
I am fine with that. As I said I have a patch series saved with just
seqnum_inc() that increments and returns. I anticipated people would
have problems with seqnum_inc() that returns. :)
>> being said, I have no problems with making just _inc(). The reason
>> for 32 and 64 appended is based on comments that it including size
>> in the api makes it very clear.
>
> By putting 32 and 64 in the name of the API, I would contend you're making
> people think about something that they should not need to think about.
>
Are you recommending seqnum32_*() for 32bit and seqnum_*() for 64bit
which would make 64bit as a default? We have to make a distinction
for 32bit vs. 64-bit api.
>> No need for atomic_add() - inc_return() is sufficient for this use-case.
>
> I haven't looked at the various potential users of this API, but there
> are often cases where we account, eg, number of bytes transmitted.
>
> There are also cases where read-and-zero would be a useful operation
> to have. I'm thinking about sampling statistics.
>
The idea is isolating sequence number use-case first and restrict this
api for that.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops
2020-11-13 17:46 ` [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops Shuah Khan
2020-11-13 21:03 ` Matthew Wilcox
@ 2020-11-16 14:53 ` Peter Zijlstra
2020-11-17 16:15 ` Shuah Khan
1 sibling, 1 reply; 30+ messages in thread
From: Peter Zijlstra @ 2020-11-16 14:53 UTC (permalink / raw)
To: Shuah Khan; +Cc: corbet, keescook, gregkh, linux-doc, linux-kernel
On Fri, Nov 13, 2020 at 10:46:03AM -0700, Shuah Khan wrote:
> +Increment interface
> +-------------------
> +
> +Increments sequence number and returns the new value. ::
> +
> + seqnum32_inc_return() --> (u32) atomic_inc_return(seqnum)
> + seqnum64_inc_return() --> (u64) atomic64_inc_return(seqnum)
Did you think about the ordering?
> +Fetch interface
> +---------------
> +
> +Fetched and returns current sequence number value. ::
> +
> + seqnum32_fetch() --> (u32) atomic_add_return(0, seqnum)
> + seqnum64_fetch() --> (u64) atomic64_add_return(0, seqnum)
That's horrible. Please explain how that is not broken garbage.
Per the fact that it is atomic, nothing prevents the counter being
incremented concurrently. There is no such thing as a 'current' sequence
number.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops
2020-11-16 14:53 ` Peter Zijlstra
@ 2020-11-17 16:15 ` Shuah Khan
0 siblings, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-17 16:15 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: corbet, keescook, gregkh, linux-doc, linux-kernel, skhan
On 11/16/20 7:53 AM, Peter Zijlstra wrote:
> On Fri, Nov 13, 2020 at 10:46:03AM -0700, Shuah Khan wrote:
>
>> +Increment interface
>> +-------------------
>> +
>> +Increments sequence number and returns the new value. ::
>> +
>> + seqnum32_inc_return() --> (u32) atomic_inc_return(seqnum)
>> + seqnum64_inc_return() --> (u64) atomic64_inc_return(seqnum)
>
> Did you think about the ordering?
>
Looking at atomic_t.txt _inc_return() can be fully ordered without
loosing or making the intermediate state visible. This is good for
this sequence number use-case. Is there something I am overlooking?
>> +Fetch interface
>> +---------------
>> +
>> +Fetched and returns current sequence number value. ::
>> +
>> + seqnum32_fetch() --> (u32) atomic_add_return(0, seqnum)
>> + seqnum64_fetch() --> (u64) atomic64_add_return(0, seqnum)
>
> That's horrible. Please explain how that is not broken garbage.
>
> Per the fact that it is atomic, nothing prevents the counter being
> incremented concurrently. There is no such thing as a 'current' sequence
> number.
>
Correct. Some usages of this _fecth() in this patch series are for
printing sequence numbers in debug message and others are stats.
I will review the patches in this series and drop the ones that use
read/fetch - the reason being sequence numbers are strictly up counters
and don't need read/fetch.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 02/13] selftests: lib:test_seqnum_ops: add new test for seqnum_ops
2020-11-13 17:46 [PATCH v2 00/13] Introduce seqnum_ops Shuah Khan
2020-11-13 17:46 ` [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops Shuah Khan
@ 2020-11-13 17:46 ` Shuah Khan
2020-11-13 17:46 ` [PATCH v2 03/13] drivers/acpi: convert seqno seqnum_ops Shuah Khan
` (10 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw)
To: corbet, keescook, gregkh, peterz
Cc: Shuah Khan, linux-doc, linux-kernel, linux-kselftest
Add a new selftest for testing seqnum_ops. This test loads test_seqnum_ops
test module and unloads it. The test module runs tests and prints results
to dmesg.
Sequence Number api provides interfaces for unsigned atomic up counters
leveraging atomic_t and atomic64_t ops underneath.
There are a number of atomic_t usages in the kernel where atomic_t api
is used for counting sequence numbers and other statistical counters.
Several of these usages, convert atomic_read() and atomic_inc_return()
return values to unsigned. Introducing sequence number ops supports
these use-cases with a standard core-api.
The atomic_t api provides a wide range of atomic operations as a base
api to implement atomic counters, bitops, spinlock interfaces. The usages
also evolved into being used for resource lifetimes and state management.
The refcount_t api was introduced to address resource lifetime problems
related to atomic_t wrapping. There is a large overlap between the
atomic_t api used for resource lifetimes and just counters, stats, and
sequence numbers. It has become difficult to differentiate between the
atomic_t usages that should be converted to refcount_t and the ones that
can be left alone. Introducing seqnum_ops to wrap the usages that are
stats, counters, sequence numbers makes it easier for tools that scan
for underflow and overflow on atomic_t usages to detect overflow and
underflows to scan just the cases that are prone to errors.
In addition, to supporting sequence number use-cases, Sequence Number Ops
helps differentiate atomic_t counter usages from atomic_t usages that guard
object lifetimes, hence prone to overflow and underflow errors from up
counting use-cases.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
Documentation/core-api/seqnum_ops.rst | 9 +++++++++
MAINTAINERS | 1 +
include/linux/seqnum_ops.h | 2 ++
tools/testing/selftests/lib/Makefile | 1 +
tools/testing/selftests/lib/config | 1 +
tools/testing/selftests/lib/test_seqnum_ops.sh | 10 ++++++++++
6 files changed, 24 insertions(+)
create mode 100755 tools/testing/selftests/lib/test_seqnum_ops.sh
diff --git a/Documentation/core-api/seqnum_ops.rst b/Documentation/core-api/seqnum_ops.rst
index 10b775a9ac05..f66d796b148e 100644
--- a/Documentation/core-api/seqnum_ops.rst
+++ b/Documentation/core-api/seqnum_ops.rst
@@ -78,3 +78,12 @@ Fetched and returns current sequence number value. ::
seqnum32_fetch() --> (u32) atomic_add_return(0, seqnum)
seqnum64_fetch() --> (u64) atomic64_add_return(0, seqnum)
+
+Where are the seqnum_ops and how to use and test them?
+------------------------------------------------------
+
+.. kernel-doc:: include/linux/seqnum_ops.h
+
+Please see lib/test_seqnum_ops.c for examples usages and test module.
+Please find selftest: testing/selftests/lib/test_seqnum_ops.sh
+Please check dmesg for results after running test_seqnum_ops.sh.
diff --git a/MAINTAINERS b/MAINTAINERS
index c83a6f05610b..e6ae131836a5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15983,6 +15983,7 @@ L: linux-kernel@vger.kernel.org
S: Maintained
F: include/linux/seqnum_ops.h
F: lib/test_seqnum_ops.c
+F: tools/testing/selftests/lib/test_seqnum_ops.sh
SIMPLE FIRMWARE INTERFACE (SFI)
S: Obsolete
diff --git a/include/linux/seqnum_ops.h b/include/linux/seqnum_ops.h
index 17d327b78050..bbf724f21e03 100644
--- a/include/linux/seqnum_ops.h
+++ b/include/linux/seqnum_ops.h
@@ -19,6 +19,8 @@
*
* Reference and API guide:
* Documentation/core-api/seqnum_ops.rst for more information.
+ * lib/test_seqnum_ops.c - example usages and test module
+ * tools/testing/selftests/lib/test_seqnum_ops.sh
*/
#ifndef __LINUX_SEQNUM_OPS_H
diff --git a/tools/testing/selftests/lib/Makefile b/tools/testing/selftests/lib/Makefile
index a105f094676e..1818444f0e97 100644
--- a/tools/testing/selftests/lib/Makefile
+++ b/tools/testing/selftests/lib/Makefile
@@ -5,5 +5,6 @@
all:
TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh strscpy.sh
+TEST_PROGS += test_seqnum_ops.sh
include ../lib.mk
diff --git a/tools/testing/selftests/lib/config b/tools/testing/selftests/lib/config
index b80ee3f6e265..674ed2a2ac82 100644
--- a/tools/testing/selftests/lib/config
+++ b/tools/testing/selftests/lib/config
@@ -3,3 +3,4 @@ CONFIG_TEST_BITMAP=m
CONFIG_PRIME_NUMBERS=m
CONFIG_TEST_STRSCPY=m
CONFIG_TEST_BITOPS=m
+CONFIG_TEST_SEQNUM_OPS=m
diff --git a/tools/testing/selftests/lib/test_seqnum_ops.sh b/tools/testing/selftests/lib/test_seqnum_ops.sh
new file mode 100755
index 000000000000..fdce16b220ba
--- /dev/null
+++ b/tools/testing/selftests/lib/test_seqnum_ops.sh
@@ -0,0 +1,10 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (c) 2020 Shuah Khan <skhan@linuxfoundation.org>
+# Copyright (c) 2020 The Linux Foundation
+#
+# Tests the Sequence Number Ops interfaces using test_seqnum_ops
+# kernel module
+#
+$(dirname $0)/../kselftest/module.sh "test_seqnum_ops" test_seqnum_ops
--
2.27.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 03/13] drivers/acpi: convert seqno seqnum_ops
2020-11-13 17:46 [PATCH v2 00/13] Introduce seqnum_ops Shuah Khan
2020-11-13 17:46 ` [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops Shuah Khan
2020-11-13 17:46 ` [PATCH v2 02/13] selftests: lib:test_seqnum_ops: add new test for seqnum_ops Shuah Khan
@ 2020-11-13 17:46 ` Shuah Khan
2020-11-13 17:46 ` [PATCH v2 04/13] drivers/acpi/apei: convert seqno to seqnum_ops Shuah Khan
` (9 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw)
To: rafael, lenb, gregkh, keescook, peterz
Cc: Shuah Khan, linux-acpi, linux-kernel
Sequence Number api provides interfaces for unsigned atomic up counters
leveraging atomic_t and atomic64_t ops underneath.
Convert seqno atomic counter to use seqnum_ops.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
drivers/acpi/acpi_extlog.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index 72f1fb77abcd..5872ff698644 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -12,6 +12,7 @@
#include <linux/ratelimit.h>
#include <linux/edac.h>
#include <linux/ras.h>
+#include <linux/seqnum_ops.h>
#include <asm/cpu.h>
#include <asm/mce.h>
@@ -93,8 +94,7 @@ static struct acpi_hest_generic_status *extlog_elog_entry_check(int cpu, int ban
static void __print_extlog_rcd(const char *pfx,
struct acpi_hest_generic_status *estatus, int cpu)
{
- static atomic_t seqno;
- unsigned int curr_seqno;
+ static struct seqnum32 seqno;
char pfx_seq[64];
if (!pfx) {
@@ -103,8 +103,8 @@ static void __print_extlog_rcd(const char *pfx,
else
pfx = KERN_ERR;
}
- curr_seqno = atomic_inc_return(&seqno);
- snprintf(pfx_seq, sizeof(pfx_seq), "%s{%u}", pfx, curr_seqno);
+ snprintf(pfx_seq, sizeof(pfx_seq), "%s{%u}", pfx,
+ seqnum32_inc_return(&seqno));
printk("%s""Hardware error detected on CPU%d\n", pfx_seq, cpu);
cper_estatus_print(pfx_seq, estatus);
}
--
2.27.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 04/13] drivers/acpi/apei: convert seqno to seqnum_ops
2020-11-13 17:46 [PATCH v2 00/13] Introduce seqnum_ops Shuah Khan
` (2 preceding siblings ...)
2020-11-13 17:46 ` [PATCH v2 03/13] drivers/acpi: convert seqno seqnum_ops Shuah Khan
@ 2020-11-13 17:46 ` Shuah Khan
2020-11-13 17:46 ` [PATCH v2 05/13] drivers/base/test/test_async_driver_probe: convert to use seqnum_ops Shuah Khan
` (8 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw)
To: rafael, james.morse, tony.luck, bp, gregkh, keescook, peterz
Cc: Shuah Khan, linux-acpi, linux-kernel
Sequence Number api provides interfaces for unsigned atomic up counters
leveraging atomic_t and atomic64_t ops underneath.
Convert seqno atomic counter to use seqnum_ops.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
drivers/acpi/apei/ghes.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index fce7ade2aba9..36ebecf36dfb 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -41,6 +41,7 @@
#include <linux/uuid.h>
#include <linux/ras.h>
#include <linux/task_work.h>
+#include <linux/seqnum_ops.h>
#include <acpi/actbl1.h>
#include <acpi/ghes.h>
@@ -625,8 +626,7 @@ static void __ghes_print_estatus(const char *pfx,
const struct acpi_hest_generic *generic,
const struct acpi_hest_generic_status *estatus)
{
- static atomic_t seqno;
- unsigned int curr_seqno;
+ static struct seqnum32 seqno = SEQNUM_INIT(0);
char pfx_seq[64];
if (pfx == NULL) {
@@ -636,8 +636,8 @@ static void __ghes_print_estatus(const char *pfx,
else
pfx = KERN_ERR;
}
- curr_seqno = atomic_inc_return(&seqno);
- snprintf(pfx_seq, sizeof(pfx_seq), "%s{%u}" HW_ERR, pfx, curr_seqno);
+ snprintf(pfx_seq, sizeof(pfx_seq), "%s{%u}" HW_ERR, pfx,
+ seqnum32_inc_return(&seqno));
printk("%s""Hardware error from APEI Generic Hardware Error Source: %d\n",
pfx_seq, generic->header.source_id);
cper_estatus_print(pfx_seq, estatus);
--
2.27.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 05/13] drivers/base/test/test_async_driver_probe: convert to use seqnum_ops
2020-11-13 17:46 [PATCH v2 00/13] Introduce seqnum_ops Shuah Khan
` (3 preceding siblings ...)
2020-11-13 17:46 ` [PATCH v2 04/13] drivers/acpi/apei: convert seqno to seqnum_ops Shuah Khan
@ 2020-11-13 17:46 ` Shuah Khan
2020-11-13 17:46 ` [PATCH v2 06/13] drivers/char/ipmi: convert stats " Shuah Khan
` (7 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw)
To: gregkh, rafael, keescook, peterz; +Cc: Shuah Khan, linux-kernel
Sequence Number api provides interfaces for unsigned atomic up counters
leveraging atomic_t and atomic64_t ops underneath.
atomic_t variables used to count errors, warns, keep track of timeout,
and async completion are counters. Convert them to use seqnum32 and init
counters to 0.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
drivers/base/test/test_async_driver_probe.c | 28 +++++++++++++--------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/base/test/test_async_driver_probe.c b/drivers/base/test/test_async_driver_probe.c
index 3bb7beb127a9..75a3b617dcac 100644
--- a/drivers/base/test/test_async_driver_probe.c
+++ b/drivers/base/test/test_async_driver_probe.c
@@ -14,11 +14,15 @@
#include <linux/numa.h>
#include <linux/nodemask.h>
#include <linux/topology.h>
+#include <linux/seqnum_ops.h>
#define TEST_PROBE_DELAY (5 * 1000) /* 5 sec */
#define TEST_PROBE_THRESHOLD (TEST_PROBE_DELAY / 2)
-static atomic_t warnings, errors, timeout, async_completed;
+static struct seqnum32 warnings = SEQNUM_INIT(0);
+static struct seqnum32 errors = SEQNUM_INIT(0);
+static struct seqnum32 timeout = SEQNUM_INIT(0);
+static struct seqnum32 async_completed = SEQNUM_INIT(0);
static int test_probe(struct platform_device *pdev)
{
@@ -29,9 +33,9 @@ static int test_probe(struct platform_device *pdev)
* have then report it as an error, otherwise we wil sleep for the
* required amount of time and then report completion.
*/
- if (atomic_read(&timeout)) {
+ if (seqnum32_fetch(&timeout)) {
dev_err(dev, "async probe took too long\n");
- atomic_inc(&errors);
+ seqnum32_inc_return(&errors);
} else {
dev_dbg(&pdev->dev, "sleeping for %d msecs in probe\n",
TEST_PROBE_DELAY);
@@ -48,10 +52,10 @@ static int test_probe(struct platform_device *pdev)
dev_to_node(dev) != numa_node_id()) {
dev_warn(dev, "NUMA node mismatch %d != %d\n",
dev_to_node(dev), numa_node_id());
- atomic_inc(&warnings);
+ seqnum32_inc_return(&warnings);
}
- atomic_inc(&async_completed);
+ seqnum32_inc_return(&async_completed);
}
return 0;
@@ -244,11 +248,12 @@ static int __init test_async_probe_init(void)
* Otherwise if they completed without errors or warnings then
* report successful completion.
*/
- if (atomic_read(&async_completed) != async_id) {
+ if (seqnum32_fetch(&async_completed) != async_id) {
pr_err("async events still pending, forcing timeout\n");
- atomic_inc(&timeout);
+ seqnum32_inc_return(&timeout);
err = -ETIMEDOUT;
- } else if (!atomic_read(&errors) && !atomic_read(&warnings)) {
+ } else if (!seqnum32_fetch(&errors) &&
+ !seqnum32_fetch(&warnings)) {
pr_info("completed successfully\n");
return 0;
}
@@ -271,12 +276,13 @@ static int __init test_async_probe_init(void)
* errors or warnings being reported by the probe routine.
*/
if (err)
- atomic_inc(&errors);
+ seqnum32_inc_return(&errors);
else
err = -EINVAL;
- pr_err("Test failed with %d errors and %d warnings\n",
- atomic_read(&errors), atomic_read(&warnings));
+ pr_err("Test failed with %u errors and %u warnings\n",
+ seqnum32_fetch(&errors),
+ seqnum32_fetch(&warnings));
return err;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 06/13] drivers/char/ipmi: convert stats to use seqnum_ops
2020-11-13 17:46 [PATCH v2 00/13] Introduce seqnum_ops Shuah Khan
` (4 preceding siblings ...)
2020-11-13 17:46 ` [PATCH v2 05/13] drivers/base/test/test_async_driver_probe: convert to use seqnum_ops Shuah Khan
@ 2020-11-13 17:46 ` Shuah Khan
2020-11-13 17:46 ` [PATCH v2 07/13] drivers/edac: convert pci counters to seqnum_ops Shuah Khan
` (6 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw)
To: minyard, arnd, gregkh, keescook, peterz
Cc: Shuah Khan, openipmi-developer, linux-kernel
Sequence Number api provides interfaces for unsigned atomic up counters
leveraging atomic_t and atomic64_t ops underneath.
Convert stats to use seqnum_ops.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
drivers/char/ipmi/ipmi_msghandler.c | 9 +++++----
drivers/char/ipmi/ipmi_si_intf.c | 9 +++++----
drivers/char/ipmi/ipmi_ssif.c | 9 +++++----
3 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 8774a3b8ff95..a21c8e6f391b 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -35,6 +35,7 @@
#include <linux/nospec.h>
#include <linux/vmalloc.h>
#include <linux/delay.h>
+#include <linux/seqnum_ops.h>
#define IPMI_DRIVER_VERSION "39.2"
@@ -587,7 +588,7 @@ struct ipmi_smi {
struct ipmi_my_addrinfo addrinfo[IPMI_MAX_CHANNELS];
bool channels_ready;
- atomic_t stats[IPMI_NUM_STATS];
+ struct seqnum32 stats[IPMI_NUM_STATS];
/*
* run_to_completion duplicate of smb_info, smi_info
@@ -633,9 +634,9 @@ static LIST_HEAD(smi_watchers);
static DEFINE_MUTEX(smi_watchers_mutex);
#define ipmi_inc_stat(intf, stat) \
- atomic_inc(&(intf)->stats[IPMI_STAT_ ## stat])
+ seqnum32_inc_return(&(intf)->stats[IPMI_STAT_ ## stat])
#define ipmi_get_stat(intf, stat) \
- ((unsigned int) atomic_read(&(intf)->stats[IPMI_STAT_ ## stat]))
+ (seqnum32_fetch(&(intf)->stats[IPMI_STAT_ ## stat]))
static const char * const addr_src_to_str[] = {
"invalid", "hotmod", "hardcoded", "SPMI", "ACPI", "SMBIOS", "PCI",
@@ -3468,7 +3469,7 @@ int ipmi_add_smi(struct module *owner,
INIT_LIST_HEAD(&intf->cmd_rcvrs);
init_waitqueue_head(&intf->waitq);
for (i = 0; i < IPMI_NUM_STATS; i++)
- atomic_set(&intf->stats[i], 0);
+ seqnum32_init(&intf->stats[i]);
mutex_lock(&ipmi_interfaces_mutex);
/* Look for a hole in the numbers. */
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 5eac94cf4ff8..584742b51c22 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -43,6 +43,7 @@
#include "ipmi_si_sm.h"
#include <linux/string.h>
#include <linux/ctype.h>
+#include <linux/seqnum_ops.h>
/* Measure times between events in the driver. */
#undef DEBUG_TIMING
@@ -237,7 +238,7 @@ struct smi_info {
bool dev_group_added;
/* Counters and things for the proc filesystem. */
- atomic_t stats[SI_NUM_STATS];
+ struct seqnum32 stats[SI_NUM_STATS];
struct task_struct *thread;
@@ -245,9 +246,9 @@ struct smi_info {
};
#define smi_inc_stat(smi, stat) \
- atomic_inc(&(smi)->stats[SI_STAT_ ## stat])
+ seqnum32_inc_return(&(smi)->stats[SI_STAT_ ## stat])
#define smi_get_stat(smi, stat) \
- ((unsigned int) atomic_read(&(smi)->stats[SI_STAT_ ## stat]))
+ (seqnum32_fetch(&(smi)->stats[SI_STAT_ ## stat]))
#define IPMI_MAX_INTFS 4
static int force_kipmid[IPMI_MAX_INTFS];
@@ -2030,7 +2031,7 @@ static int try_smi_init(struct smi_info *new_smi)
atomic_set(&new_smi->req_events, 0);
new_smi->run_to_completion = false;
for (i = 0; i < SI_NUM_STATS; i++)
- atomic_set(&new_smi->stats[i], 0);
+ seqnum32_init(&new_smi->stats[i]);
new_smi->interrupt_disabled = true;
atomic_set(&new_smi->need_watch, 0);
diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index 0416b9c9d410..d793286a635f 100644
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -47,6 +47,7 @@
#include <linux/acpi.h>
#include <linux/ctype.h>
#include <linux/time64.h>
+#include <linux/seqnum_ops.h>
#include "ipmi_dmi.h"
#define DEVICE_NAME "ipmi_ssif"
@@ -286,13 +287,13 @@ struct ssif_info {
unsigned int multi_len;
unsigned int multi_pos;
- atomic_t stats[SSIF_NUM_STATS];
+ struct seqnum32 stats[SSIF_NUM_STATS];
};
#define ssif_inc_stat(ssif, stat) \
- atomic_inc(&(ssif)->stats[SSIF_STAT_ ## stat])
+ seqnum32_inc_return(&(ssif)->stats[SSIF_STAT_ ## stat])
#define ssif_get_stat(ssif, stat) \
- ((unsigned int) atomic_read(&(ssif)->stats[SSIF_STAT_ ## stat]))
+ (seqnum32_fetch(&(ssif)->stats[SSIF_STAT_ ## stat]))
static bool initialized;
static bool platform_registered;
@@ -1864,7 +1865,7 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
timer_setup(&ssif_info->watch_timer, watch_timeout, 0);
for (i = 0; i < SSIF_NUM_STATS; i++)
- atomic_set(&ssif_info->stats[i], 0);
+ seqnum32_init(&ssif_info->stats[i]);
if (ssif_info->supports_pec)
ssif_info->client->flags |= I2C_CLIENT_PEC;
--
2.27.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 07/13] drivers/edac: convert pci counters to seqnum_ops
2020-11-13 17:46 [PATCH v2 00/13] Introduce seqnum_ops Shuah Khan
` (5 preceding siblings ...)
2020-11-13 17:46 ` [PATCH v2 06/13] drivers/char/ipmi: convert stats " Shuah Khan
@ 2020-11-13 17:46 ` Shuah Khan
2020-11-13 17:46 ` [PATCH v2 08/13] drivers/oprofile: convert stats to use seqnum_ops Shuah Khan
` (5 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw)
To: bp, mchehab, tony.luck, james.morse, rric, gregkh, keescook, peterz
Cc: Shuah Khan, linux-edac, linux-kernel
Sequence Number api provides interfaces for unsigned atomic up counters
leveraging atomic_t and atomic64_t ops underneath.
atomic_t variables used for pci counters keep track of pci parity and
non-parity errors. Convert them to use seqnum_ops.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
drivers/edac/edac_pci.h | 5 +++--
drivers/edac/edac_pci_sysfs.c | 30 +++++++++++++++---------------
2 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/edac/edac_pci.h b/drivers/edac/edac_pci.h
index 5175f5724cfa..33b33e62c37f 100644
--- a/drivers/edac/edac_pci.h
+++ b/drivers/edac/edac_pci.h
@@ -30,12 +30,13 @@
#include <linux/pci.h>
#include <linux/types.h>
#include <linux/workqueue.h>
+#include <linux/seqnum_ops.h>
#ifdef CONFIG_PCI
struct edac_pci_counter {
- atomic_t pe_count;
- atomic_t npe_count;
+ struct seqnum32 pe_count;
+ struct seqnum32 npe_count;
};
/*
diff --git a/drivers/edac/edac_pci_sysfs.c b/drivers/edac/edac_pci_sysfs.c
index 53042af7262e..08a34ecd2fb7 100644
--- a/drivers/edac/edac_pci_sysfs.c
+++ b/drivers/edac/edac_pci_sysfs.c
@@ -23,8 +23,8 @@ static int edac_pci_log_pe = 1; /* log PCI parity errors */
static int edac_pci_log_npe = 1; /* log PCI non-parity error errors */
static int edac_pci_poll_msec = 1000; /* one second workq period */
-static atomic_t pci_parity_count = ATOMIC_INIT(0);
-static atomic_t pci_nonparity_count = ATOMIC_INIT(0);
+static struct seqnum32 pci_parity_count = SEQNUM_INIT(0);
+static struct seqnum32 pci_nonparity_count = SEQNUM_INIT(0);
static struct kobject *edac_pci_top_main_kobj;
static atomic_t edac_pci_sysfs_refcount = ATOMIC_INIT(0);
@@ -58,13 +58,13 @@ int edac_pci_get_poll_msec(void)
/**************************** EDAC PCI sysfs instance *******************/
static ssize_t instance_pe_count_show(struct edac_pci_ctl_info *pci, char *data)
{
- return sprintf(data, "%u\n", atomic_read(&pci->counters.pe_count));
+ return sprintf(data, "%u\n", seqnum32_fetch(&pci->counters.pe_count));
}
static ssize_t instance_npe_count_show(struct edac_pci_ctl_info *pci,
char *data)
{
- return sprintf(data, "%u\n", atomic_read(&pci->counters.npe_count));
+ return sprintf(data, "%u\n", seqnum32_fetch(&pci->counters.npe_count));
}
#define to_instance(k) container_of(k, struct edac_pci_ctl_info, kobj)
@@ -553,7 +553,7 @@ static void edac_pci_dev_parity_test(struct pci_dev *dev)
edac_printk(KERN_CRIT, EDAC_PCI,
"Signaled System Error on %s\n",
pci_name(dev));
- atomic_inc(&pci_nonparity_count);
+ seqnum32_inc_return(&pci_nonparity_count);
}
if (status & (PCI_STATUS_PARITY)) {
@@ -561,7 +561,7 @@ static void edac_pci_dev_parity_test(struct pci_dev *dev)
"Master Data Parity Error on %s\n",
pci_name(dev));
- atomic_inc(&pci_parity_count);
+ seqnum32_inc_return(&pci_parity_count);
}
if (status & (PCI_STATUS_DETECTED_PARITY)) {
@@ -569,7 +569,7 @@ static void edac_pci_dev_parity_test(struct pci_dev *dev)
"Detected Parity Error on %s\n",
pci_name(dev));
- atomic_inc(&pci_parity_count);
+ seqnum32_inc_return(&pci_parity_count);
}
}
@@ -592,7 +592,7 @@ static void edac_pci_dev_parity_test(struct pci_dev *dev)
edac_printk(KERN_CRIT, EDAC_PCI, "Bridge "
"Signaled System Error on %s\n",
pci_name(dev));
- atomic_inc(&pci_nonparity_count);
+ seqnum32_inc_return(&pci_nonparity_count);
}
if (status & (PCI_STATUS_PARITY)) {
@@ -600,7 +600,7 @@ static void edac_pci_dev_parity_test(struct pci_dev *dev)
"Master Data Parity Error on "
"%s\n", pci_name(dev));
- atomic_inc(&pci_parity_count);
+ seqnum32_inc_return(&pci_parity_count);
}
if (status & (PCI_STATUS_DETECTED_PARITY)) {
@@ -608,7 +608,7 @@ static void edac_pci_dev_parity_test(struct pci_dev *dev)
"Detected Parity Error on %s\n",
pci_name(dev));
- atomic_inc(&pci_parity_count);
+ seqnum32_inc_return(&pci_parity_count);
}
}
}
@@ -638,7 +638,7 @@ static inline void edac_pci_dev_parity_iterator(pci_parity_check_fn_t fn)
*/
void edac_pci_do_parity_check(void)
{
- int before_count;
+ u32 before_count;
edac_dbg(3, "\n");
@@ -646,7 +646,7 @@ void edac_pci_do_parity_check(void)
if (!check_pci_errors)
return;
- before_count = atomic_read(&pci_parity_count);
+ before_count = seqnum32_fetch(&pci_parity_count);
/* scan all PCI devices looking for a Parity Error on devices and
* bridges.
@@ -658,7 +658,7 @@ void edac_pci_do_parity_check(void)
/* Only if operator has selected panic on PCI Error */
if (edac_pci_get_panic_on_pe()) {
/* If the count is different 'after' from 'before' */
- if (before_count != atomic_read(&pci_parity_count))
+ if (before_count != seqnum32_fetch(&pci_parity_count))
panic("EDAC: PCI Parity Error");
}
}
@@ -686,7 +686,7 @@ void edac_pci_handle_pe(struct edac_pci_ctl_info *pci, const char *msg)
{
/* global PE counter incremented by edac_pci_do_parity_check() */
- atomic_inc(&pci->counters.pe_count);
+ seqnum32_inc_return(&pci->counters.pe_count);
if (edac_pci_get_log_pe())
edac_pci_printk(pci, KERN_WARNING,
@@ -711,7 +711,7 @@ void edac_pci_handle_npe(struct edac_pci_ctl_info *pci, const char *msg)
{
/* global NPE counter incremented by edac_pci_do_parity_check() */
- atomic_inc(&pci->counters.npe_count);
+ seqnum32_inc_return(&pci->counters.npe_count);
if (edac_pci_get_log_npe())
edac_pci_printk(pci, KERN_WARNING,
--
2.27.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 08/13] drivers/oprofile: convert stats to use seqnum_ops
2020-11-13 17:46 [PATCH v2 00/13] Introduce seqnum_ops Shuah Khan
` (6 preceding siblings ...)
2020-11-13 17:46 ` [PATCH v2 07/13] drivers/edac: convert pci counters to seqnum_ops Shuah Khan
@ 2020-11-13 17:46 ` Shuah Khan
2020-11-13 17:46 ` Shuah Khan
` (4 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw)
To: rric, gregkh, keescook, peterz; +Cc: Shuah Khan, oprofile-list, linux-kernel
Sequence Number api provides interfaces for unsigned atomic up counters
leveraging atomic_t and atomic64_t ops underneath.
atomic_t variables used for stats are atomic counters. Convert them
to use seqnum_ops.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
drivers/oprofile/buffer_sync.c | 9 +++++----
drivers/oprofile/event_buffer.c | 3 ++-
drivers/oprofile/oprof.c | 3 ++-
drivers/oprofile/oprofile_stats.c | 11 ++++++-----
drivers/oprofile/oprofile_stats.h | 11 ++++++-----
drivers/oprofile/oprofilefs.c | 3 ++-
include/linux/oprofile.h | 3 ++-
7 files changed, 25 insertions(+), 18 deletions(-)
diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c
index cc917865f13a..5c10b7d5d076 100644
--- a/drivers/oprofile/buffer_sync.c
+++ b/drivers/oprofile/buffer_sync.c
@@ -34,6 +34,7 @@
#include <linux/sched/mm.h>
#include <linux/sched/task.h>
#include <linux/gfp.h>
+#include <linux/seqnum_ops.h>
#include "oprofile_stats.h"
#include "event_buffer.h"
@@ -347,7 +348,7 @@ static void add_data(struct op_entry *entry, struct mm_struct *mm)
if (cookie == NO_COOKIE)
offset = pc;
if (cookie == INVALID_COOKIE) {
- atomic_inc(&oprofile_stats.sample_lost_no_mapping);
+ seqnum32_inc_return(&oprofile_stats.sample_lost_no_mapping);
offset = pc;
}
if (cookie != last_cookie) {
@@ -391,14 +392,14 @@ add_sample(struct mm_struct *mm, struct op_sample *s, int in_kernel)
/* add userspace sample */
if (!mm) {
- atomic_inc(&oprofile_stats.sample_lost_no_mm);
+ seqnum32_inc_return(&oprofile_stats.sample_lost_no_mm);
return 0;
}
cookie = lookup_dcookie(mm, s->eip, &offset);
if (cookie == INVALID_COOKIE) {
- atomic_inc(&oprofile_stats.sample_lost_no_mapping);
+ seqnum32_inc_return(&oprofile_stats.sample_lost_no_mapping);
return 0;
}
@@ -556,7 +557,7 @@ void sync_buffer(int cpu)
/* ignore backtraces if failed to add a sample */
if (state == sb_bt_start) {
state = sb_bt_ignore;
- atomic_inc(&oprofile_stats.bt_lost_no_mapping);
+ seqnum32_inc_return(&oprofile_stats.bt_lost_no_mapping);
}
}
release_mm(mm);
diff --git a/drivers/oprofile/event_buffer.c b/drivers/oprofile/event_buffer.c
index 6c9edc8bbc95..2e2ff87b2a5d 100644
--- a/drivers/oprofile/event_buffer.c
+++ b/drivers/oprofile/event_buffer.c
@@ -19,6 +19,7 @@
#include <linux/dcookies.h>
#include <linux/fs.h>
#include <linux/uaccess.h>
+#include <linux/seqnum_ops.h>
#include "oprof.h"
#include "event_buffer.h"
@@ -53,7 +54,7 @@ void add_event_entry(unsigned long value)
}
if (buffer_pos == buffer_size) {
- atomic_inc(&oprofile_stats.event_lost_overflow);
+ seqnum32_inc_return(&oprofile_stats.event_lost_overflow);
return;
}
diff --git a/drivers/oprofile/oprof.c b/drivers/oprofile/oprof.c
index ed2c3ec07024..0f65235fb760 100644
--- a/drivers/oprofile/oprof.c
+++ b/drivers/oprofile/oprof.c
@@ -15,6 +15,7 @@
#include <linux/workqueue.h>
#include <linux/time.h>
#include <linux/mutex.h>
+#include <linux/seqnum_ops.h>
#include "oprof.h"
#include "event_buffer.h"
@@ -110,7 +111,7 @@ static void switch_worker(struct work_struct *work)
if (oprofile_ops.switch_events())
return;
- atomic_inc(&oprofile_stats.multiplex_counter);
+ seqnum32_inc_return(&oprofile_stats.multiplex_counter);
start_switch_worker();
}
diff --git a/drivers/oprofile/oprofile_stats.c b/drivers/oprofile/oprofile_stats.c
index 59659cea4582..5ad52a4ba5a2 100644
--- a/drivers/oprofile/oprofile_stats.c
+++ b/drivers/oprofile/oprofile_stats.c
@@ -11,6 +11,7 @@
#include <linux/smp.h>
#include <linux/cpumask.h>
#include <linux/threads.h>
+#include <linux/seqnum_ops.h>
#include "oprofile_stats.h"
#include "cpu_buffer.h"
@@ -30,11 +31,11 @@ void oprofile_reset_stats(void)
cpu_buf->sample_invalid_eip = 0;
}
- atomic_set(&oprofile_stats.sample_lost_no_mm, 0);
- atomic_set(&oprofile_stats.sample_lost_no_mapping, 0);
- atomic_set(&oprofile_stats.event_lost_overflow, 0);
- atomic_set(&oprofile_stats.bt_lost_no_mapping, 0);
- atomic_set(&oprofile_stats.multiplex_counter, 0);
+ seqnum32_init(&oprofile_stats.sample_lost_no_mm);
+ seqnum32_init(&oprofile_stats.sample_lost_no_mapping);
+ seqnum32_init(&oprofile_stats.event_lost_overflow);
+ seqnum32_init(&oprofile_stats.bt_lost_no_mapping);
+ seqnum32_init(&oprofile_stats.multiplex_counter);
}
diff --git a/drivers/oprofile/oprofile_stats.h b/drivers/oprofile/oprofile_stats.h
index 1fc622bd1834..229bcbb16527 100644
--- a/drivers/oprofile/oprofile_stats.h
+++ b/drivers/oprofile/oprofile_stats.h
@@ -11,13 +11,14 @@
#define OPROFILE_STATS_H
#include <linux/atomic.h>
+#include <linux/seqnum_ops.h>
struct oprofile_stat_struct {
- atomic_t sample_lost_no_mm;
- atomic_t sample_lost_no_mapping;
- atomic_t bt_lost_no_mapping;
- atomic_t event_lost_overflow;
- atomic_t multiplex_counter;
+ struct seqnum32 sample_lost_no_mm;
+ struct seqnum32 sample_lost_no_mapping;
+ struct seqnum32 bt_lost_no_mapping;
+ struct seqnum32 event_lost_overflow;
+ struct seqnum32 multiplex_counter;
};
extern struct oprofile_stat_struct oprofile_stats;
diff --git a/drivers/oprofile/oprofilefs.c b/drivers/oprofile/oprofilefs.c
index 0875f2f122b3..c5749b9aca11 100644
--- a/drivers/oprofile/oprofilefs.c
+++ b/drivers/oprofile/oprofilefs.c
@@ -17,6 +17,7 @@
#include <linux/fs_context.h>
#include <linux/pagemap.h>
#include <linux/uaccess.h>
+#include <linux/seqnum_ops.h>
#include "oprof.h"
@@ -193,7 +194,7 @@ static const struct file_operations atomic_ro_fops = {
int oprofilefs_create_ro_atomic(struct dentry *root,
- char const *name, atomic_t *val)
+ char const *name, struct seqnum32 *val)
{
return __oprofilefs_create_file(root, name,
&atomic_ro_fops, 0444, val);
diff --git a/include/linux/oprofile.h b/include/linux/oprofile.h
index b2a0f15f11fe..f770254a0c8a 100644
--- a/include/linux/oprofile.h
+++ b/include/linux/oprofile.h
@@ -19,6 +19,7 @@
#include <linux/errno.h>
#include <linux/printk.h>
#include <linux/atomic.h>
+#include <linux/seqnum_ops.h>
/* Each escaped entry is prefixed by ESCAPE_CODE
* then one of the following codes, then the
@@ -140,7 +141,7 @@ int oprofilefs_create_ro_ulong(struct dentry * root,
/** Create a file for read-only access to an atomic_t. */
int oprofilefs_create_ro_atomic(struct dentry * root,
- char const * name, atomic_t * val);
+ char const *name, struct seqnum32 *val);
/** create a directory */
struct dentry *oprofilefs_mkdir(struct dentry *parent, char const *name);
--
2.27.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 09/13] drivers/staging/rtl8723bs: convert stats to use seqnum_ops
2020-11-13 17:46 [PATCH v2 00/13] Introduce seqnum_ops Shuah Khan
@ 2020-11-13 17:46 ` Shuah Khan
2020-11-13 17:46 ` [PATCH v2 02/13] selftests: lib:test_seqnum_ops: add new test for seqnum_ops Shuah Khan
` (11 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw)
To: gregkh, keescook, peterz; +Cc: Shuah Khan, devel, linux-kernel
Sequence Number api provides interfaces for unsigned atomic up counters
leveraging atomic_t and atomic64_t ops underneath. Convert it to use
seqnum_ops.
atomic_t variables used for stats are atomic counters. Convert them to
use seqnum_ops.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
drivers/staging/rtl8723bs/core/rtw_cmd.c | 3 +-
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 33 +++++++++++++------
drivers/staging/rtl8723bs/include/rtw_cmd.h | 3 +-
.../staging/rtl8723bs/include/rtw_mlme_ext.h | 3 +-
4 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
index 2abe205e3453..a82a0d9af3b0 100644
--- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
@@ -10,6 +10,7 @@
#include <rtw_debug.h>
#include <hal_btcoex.h>
#include <linux/jiffies.h>
+#include <linux/seqnum_ops.h>
static struct _cmd_callback rtw_cmd_callback[] = {
{GEN_CMD_CODE(_Read_MACREG), NULL}, /*0*/
@@ -207,7 +208,7 @@ static void c2h_wk_callback(_workitem * work);
int rtw_init_evt_priv(struct evt_priv *pevtpriv)
{
/* allocate DMA-able/Non-Page memory for cmd_buf and rsp_buf */
- atomic_set(&pevtpriv->event_seq, 0);
+ seqnum32_init(&pevtpriv->event_seq);
pevtpriv->evt_done_cnt = 0;
_init_workitem(&pevtpriv->c2h_wk, c2h_wk_callback, NULL);
diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index b912ad2f4b72..48be6049b45c 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -11,6 +11,7 @@
#include <rtw_wifi_regd.h>
#include <hal_btcoex.h>
#include <linux/kernel.h>
+#include <linux/seqnum_ops.h>
#include <asm/unaligned.h>
static struct mlme_handler mlme_sta_tbl[] = {
@@ -281,7 +282,7 @@ static void init_mlme_ext_priv_value(struct adapter *padapter)
struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
struct mlme_ext_info *pmlmeinfo = &pmlmeext->mlmext_info;
- atomic_set(&pmlmeext->event_seq, 0);
+ seqnum32_init(&pmlmeext->event_seq);
pmlmeext->mgnt_seq = 0;/* reset to zero when disconnect at client mode */
pmlmeext->sa_query_seq = 0;
pmlmeext->mgnt_80211w_IPN = 0;
@@ -5051,7 +5052,9 @@ void report_survey_event(struct adapter *padapter, union recv_frame *precv_frame
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct survey_event);
pc2h_evt_hdr->ID = GEN_EVT_CODE(_Survey);
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
psurvey_evt = (struct survey_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
@@ -5104,7 +5107,9 @@ void report_surveydone_event(struct adapter *padapter)
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct surveydone_event);
pc2h_evt_hdr->ID = GEN_EVT_CODE(_SurveyDone);
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
psurveydone_evt = (struct surveydone_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
psurveydone_evt->bss_cnt = pmlmeext->sitesurvey_res.bss_cnt;
@@ -5151,7 +5156,9 @@ void report_join_res(struct adapter *padapter, int res)
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct joinbss_event);
pc2h_evt_hdr->ID = GEN_EVT_CODE(_JoinBss);
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
pjoinbss_evt = (struct joinbss_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
memcpy((unsigned char *)(&(pjoinbss_evt->network.network)), &(pmlmeinfo->network), sizeof(struct wlan_bssid_ex));
@@ -5202,7 +5209,9 @@ void report_wmm_edca_update(struct adapter *padapter)
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct wmm_event);
pc2h_evt_hdr->ID = GEN_EVT_CODE(_WMM);
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
pwmm_event = (struct wmm_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
pwmm_event->wmm = 0;
@@ -5249,7 +5258,9 @@ void report_del_sta_event(struct adapter *padapter, unsigned char *MacAddr, unsi
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct stadel_event);
pc2h_evt_hdr->ID = GEN_EVT_CODE(_DelSTA);
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
pdel_sta_evt = (struct stadel_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
memcpy((unsigned char *)(&(pdel_sta_evt->macaddr)), MacAddr, ETH_ALEN);
@@ -5302,7 +5313,9 @@ void report_add_sta_event(struct adapter *padapter, unsigned char *MacAddr, int
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct stassoc_event);
pc2h_evt_hdr->ID = GEN_EVT_CODE(_AddSTA);
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
padd_sta_evt = (struct stassoc_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
memcpy((unsigned char *)(&(padd_sta_evt->macaddr)), MacAddr, ETH_ALEN);
@@ -6620,10 +6633,10 @@ u8 mlme_evt_hdl(struct adapter *padapter, unsigned char *pbuf)
#ifdef CHECK_EVENT_SEQ
/* checking event sequence... */
- if (evt_seq != (atomic_read(&pevt_priv->event_seq) & 0x7f)) {
+ if (evt_seq != (seqnum32_fetch(&pevt_priv->event_seq) & 0x7f)) {
RT_TRACE(_module_rtl871x_cmd_c_, _drv_info_,
("Event Seq Error! %d vs %d\n", (evt_seq & 0x7f),
- (atomic_read(&pevt_priv->event_seq) & 0x7f)));
+ (seqnum32_fetch(&pevt_priv->event_seq) & 0x7f)));
pevt_priv->event_seq = (evt_seq+1)&0x7f;
@@ -6647,7 +6660,7 @@ u8 mlme_evt_hdl(struct adapter *padapter, unsigned char *pbuf)
}
- atomic_inc(&pevt_priv->event_seq);
+ seqnum32_inc_return(&pevt_priv->event_seq);
peventbuf += 2;
diff --git a/drivers/staging/rtl8723bs/include/rtw_cmd.h b/drivers/staging/rtl8723bs/include/rtw_cmd.h
index 56c77bc7ca81..cc0ea649388b 100644
--- a/drivers/staging/rtl8723bs/include/rtw_cmd.h
+++ b/drivers/staging/rtl8723bs/include/rtw_cmd.h
@@ -8,6 +8,7 @@
#define __RTW_CMD_H_
#include <linux/completion.h>
+#include <linux/seqnum_ops.h>
#define C2H_MEM_SZ (16*1024)
@@ -62,7 +63,7 @@
struct rtw_cbuf *c2h_queue;
#define C2H_QUEUE_MAX_LEN 10
- atomic_t event_seq;
+ struct seqnum32 event_seq;
u8 *evt_buf; /* shall be non-paged, and 4 bytes aligned */
u8 *evt_allocated_buf;
u32 evt_done_cnt;
diff --git a/drivers/staging/rtl8723bs/include/rtw_mlme_ext.h b/drivers/staging/rtl8723bs/include/rtw_mlme_ext.h
index 1567831caf91..537813c00670 100644
--- a/drivers/staging/rtl8723bs/include/rtw_mlme_ext.h
+++ b/drivers/staging/rtl8723bs/include/rtw_mlme_ext.h
@@ -7,6 +7,7 @@
#ifndef __RTW_MLME_EXT_H_
#define __RTW_MLME_EXT_H_
+#include <linux/seqnum_ops.h>
/* Commented by Albert 20101105 */
/* Increase the SURVEY_TO value from 100 to 150 (100ms to 150ms) */
@@ -461,7 +462,7 @@ struct p2p_oper_class_map {
struct mlme_ext_priv {
struct adapter *padapter;
u8 mlmeext_init;
- atomic_t event_seq;
+ struct seqnum32 event_seq;
u16 mgnt_seq;
u16 sa_query_seq;
u64 mgnt_80211w_IPN;
--
2.27.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 09/13] drivers/staging/rtl8723bs: convert stats to use seqnum_ops
@ 2020-11-13 17:46 ` Shuah Khan
0 siblings, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw)
To: gregkh, keescook, peterz; +Cc: devel, linux-kernel, Shuah Khan
Sequence Number api provides interfaces for unsigned atomic up counters
leveraging atomic_t and atomic64_t ops underneath. Convert it to use
seqnum_ops.
atomic_t variables used for stats are atomic counters. Convert them to
use seqnum_ops.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
drivers/staging/rtl8723bs/core/rtw_cmd.c | 3 +-
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 33 +++++++++++++------
drivers/staging/rtl8723bs/include/rtw_cmd.h | 3 +-
.../staging/rtl8723bs/include/rtw_mlme_ext.h | 3 +-
4 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
index 2abe205e3453..a82a0d9af3b0 100644
--- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
@@ -10,6 +10,7 @@
#include <rtw_debug.h>
#include <hal_btcoex.h>
#include <linux/jiffies.h>
+#include <linux/seqnum_ops.h>
static struct _cmd_callback rtw_cmd_callback[] = {
{GEN_CMD_CODE(_Read_MACREG), NULL}, /*0*/
@@ -207,7 +208,7 @@ static void c2h_wk_callback(_workitem * work);
int rtw_init_evt_priv(struct evt_priv *pevtpriv)
{
/* allocate DMA-able/Non-Page memory for cmd_buf and rsp_buf */
- atomic_set(&pevtpriv->event_seq, 0);
+ seqnum32_init(&pevtpriv->event_seq);
pevtpriv->evt_done_cnt = 0;
_init_workitem(&pevtpriv->c2h_wk, c2h_wk_callback, NULL);
diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index b912ad2f4b72..48be6049b45c 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -11,6 +11,7 @@
#include <rtw_wifi_regd.h>
#include <hal_btcoex.h>
#include <linux/kernel.h>
+#include <linux/seqnum_ops.h>
#include <asm/unaligned.h>
static struct mlme_handler mlme_sta_tbl[] = {
@@ -281,7 +282,7 @@ static void init_mlme_ext_priv_value(struct adapter *padapter)
struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
struct mlme_ext_info *pmlmeinfo = &pmlmeext->mlmext_info;
- atomic_set(&pmlmeext->event_seq, 0);
+ seqnum32_init(&pmlmeext->event_seq);
pmlmeext->mgnt_seq = 0;/* reset to zero when disconnect at client mode */
pmlmeext->sa_query_seq = 0;
pmlmeext->mgnt_80211w_IPN = 0;
@@ -5051,7 +5052,9 @@ void report_survey_event(struct adapter *padapter, union recv_frame *precv_frame
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct survey_event);
pc2h_evt_hdr->ID = GEN_EVT_CODE(_Survey);
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
psurvey_evt = (struct survey_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
@@ -5104,7 +5107,9 @@ void report_surveydone_event(struct adapter *padapter)
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct surveydone_event);
pc2h_evt_hdr->ID = GEN_EVT_CODE(_SurveyDone);
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
psurveydone_evt = (struct surveydone_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
psurveydone_evt->bss_cnt = pmlmeext->sitesurvey_res.bss_cnt;
@@ -5151,7 +5156,9 @@ void report_join_res(struct adapter *padapter, int res)
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct joinbss_event);
pc2h_evt_hdr->ID = GEN_EVT_CODE(_JoinBss);
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
pjoinbss_evt = (struct joinbss_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
memcpy((unsigned char *)(&(pjoinbss_evt->network.network)), &(pmlmeinfo->network), sizeof(struct wlan_bssid_ex));
@@ -5202,7 +5209,9 @@ void report_wmm_edca_update(struct adapter *padapter)
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct wmm_event);
pc2h_evt_hdr->ID = GEN_EVT_CODE(_WMM);
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
pwmm_event = (struct wmm_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
pwmm_event->wmm = 0;
@@ -5249,7 +5258,9 @@ void report_del_sta_event(struct adapter *padapter, unsigned char *MacAddr, unsi
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct stadel_event);
pc2h_evt_hdr->ID = GEN_EVT_CODE(_DelSTA);
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
pdel_sta_evt = (struct stadel_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
memcpy((unsigned char *)(&(pdel_sta_evt->macaddr)), MacAddr, ETH_ALEN);
@@ -5302,7 +5313,9 @@ void report_add_sta_event(struct adapter *padapter, unsigned char *MacAddr, int
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct stassoc_event);
pc2h_evt_hdr->ID = GEN_EVT_CODE(_AddSTA);
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
padd_sta_evt = (struct stassoc_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
memcpy((unsigned char *)(&(padd_sta_evt->macaddr)), MacAddr, ETH_ALEN);
@@ -6620,10 +6633,10 @@ u8 mlme_evt_hdl(struct adapter *padapter, unsigned char *pbuf)
#ifdef CHECK_EVENT_SEQ
/* checking event sequence... */
- if (evt_seq != (atomic_read(&pevt_priv->event_seq) & 0x7f)) {
+ if (evt_seq != (seqnum32_fetch(&pevt_priv->event_seq) & 0x7f)) {
RT_TRACE(_module_rtl871x_cmd_c_, _drv_info_,
("Event Seq Error! %d vs %d\n", (evt_seq & 0x7f),
- (atomic_read(&pevt_priv->event_seq) & 0x7f)));
+ (seqnum32_fetch(&pevt_priv->event_seq) & 0x7f)));
pevt_priv->event_seq = (evt_seq+1)&0x7f;
@@ -6647,7 +6660,7 @@ u8 mlme_evt_hdl(struct adapter *padapter, unsigned char *pbuf)
}
- atomic_inc(&pevt_priv->event_seq);
+ seqnum32_inc_return(&pevt_priv->event_seq);
peventbuf += 2;
diff --git a/drivers/staging/rtl8723bs/include/rtw_cmd.h b/drivers/staging/rtl8723bs/include/rtw_cmd.h
index 56c77bc7ca81..cc0ea649388b 100644
--- a/drivers/staging/rtl8723bs/include/rtw_cmd.h
+++ b/drivers/staging/rtl8723bs/include/rtw_cmd.h
@@ -8,6 +8,7 @@
#define __RTW_CMD_H_
#include <linux/completion.h>
+#include <linux/seqnum_ops.h>
#define C2H_MEM_SZ (16*1024)
@@ -62,7 +63,7 @@
struct rtw_cbuf *c2h_queue;
#define C2H_QUEUE_MAX_LEN 10
- atomic_t event_seq;
+ struct seqnum32 event_seq;
u8 *evt_buf; /* shall be non-paged, and 4 bytes aligned */
u8 *evt_allocated_buf;
u32 evt_done_cnt;
diff --git a/drivers/staging/rtl8723bs/include/rtw_mlme_ext.h b/drivers/staging/rtl8723bs/include/rtw_mlme_ext.h
index 1567831caf91..537813c00670 100644
--- a/drivers/staging/rtl8723bs/include/rtw_mlme_ext.h
+++ b/drivers/staging/rtl8723bs/include/rtw_mlme_ext.h
@@ -7,6 +7,7 @@
#ifndef __RTW_MLME_EXT_H_
#define __RTW_MLME_EXT_H_
+#include <linux/seqnum_ops.h>
/* Commented by Albert 20101105 */
/* Increase the SURVEY_TO value from 100 to 150 (100ms to 150ms) */
@@ -461,7 +462,7 @@ struct p2p_oper_class_map {
struct mlme_ext_priv {
struct adapter *padapter;
u8 mlmeext_init;
- atomic_t event_seq;
+ struct seqnum32 event_seq;
u16 mgnt_seq;
u16 sa_query_seq;
u64 mgnt_80211w_IPN;
--
2.27.0
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 10/13] usb: usbip/vhci: convert seqno to seqnum_ops
2020-11-13 17:46 [PATCH v2 00/13] Introduce seqnum_ops Shuah Khan
` (8 preceding siblings ...)
2020-11-13 17:46 ` Shuah Khan
@ 2020-11-13 17:46 ` Shuah Khan
2020-11-13 17:46 ` Shuah Khan
` (2 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw)
To: valentina.manea.m, shuah, gregkh, keescook, peterz
Cc: Shuah Khan, linux-usb, linux-kernel
Sequence Number api provides interfaces for unsigned atomic up counters
leveraging atomic_t and atomic64_t ops underneath. Convert it to use
seqnum_ops.
atomic_t variables used for stats are atomic counters. Convert them to
use seqnum_ops.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
drivers/usb/usbip/vhci.h | 3 ++-
drivers/usb/usbip/vhci_hcd.c | 7 ++++---
drivers/usb/usbip/vhci_rx.c | 5 +++--
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h
index 5659dce1526e..cb76747f423b 100644
--- a/drivers/usb/usbip/vhci.h
+++ b/drivers/usb/usbip/vhci.h
@@ -15,6 +15,7 @@
#include <linux/usb.h>
#include <linux/usb/hcd.h>
#include <linux/wait.h>
+#include <linux/seqnum_ops.h>
struct vhci_device {
struct usb_device *udev;
@@ -108,7 +109,7 @@ struct vhci_hcd {
unsigned resuming:1;
unsigned long re_timeout;
- atomic_t seqnum;
+ struct seqnum32 seqnum;
/*
* NOTE:
diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index 66cde5e5f796..57287165537c 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -11,6 +11,7 @@
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
+#include <linux/seqnum_ops.h>
#include "usbip_common.h"
#include "vhci.h"
@@ -665,7 +666,7 @@ static void vhci_tx_urb(struct urb *urb, struct vhci_device *vdev)
spin_lock_irqsave(&vdev->priv_lock, flags);
- priv->seqnum = atomic_inc_return(&vhci_hcd->seqnum);
+ priv->seqnum = seqnum32_inc_return(&vhci_hcd->seqnum);
if (priv->seqnum == 0xffff)
dev_info(&urb->dev->dev, "seqnum max\n");
@@ -921,7 +922,7 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
return -ENOMEM;
}
- unlink->seqnum = atomic_inc_return(&vhci_hcd->seqnum);
+ unlink->seqnum = seqnum32_inc_return(&vhci_hcd->seqnum);
if (unlink->seqnum == 0xffff)
pr_info("seqnum max\n");
@@ -1181,7 +1182,7 @@ static int vhci_start(struct usb_hcd *hcd)
vdev->rhport = rhport;
}
- atomic_set(&vhci_hcd->seqnum, 0);
+ seqnum32_init(&vhci_hcd->seqnum);
hcd->power_budget = 0; /* no limit */
hcd->uses_new_polling = 1;
diff --git a/drivers/usb/usbip/vhci_rx.c b/drivers/usb/usbip/vhci_rx.c
index 266024cbb64f..84620538093f 100644
--- a/drivers/usb/usbip/vhci_rx.c
+++ b/drivers/usb/usbip/vhci_rx.c
@@ -5,6 +5,7 @@
#include <linux/kthread.h>
#include <linux/slab.h>
+#include <linux/seqnum_ops.h>
#include "usbip_common.h"
#include "vhci.h"
@@ -66,9 +67,9 @@ static void vhci_recv_ret_submit(struct vhci_device *vdev,
spin_unlock_irqrestore(&vdev->priv_lock, flags);
if (!urb) {
- pr_err("cannot find a urb of seqnum %u max seqnum %d\n",
+ pr_err("cannot find a urb of seqnum %u max seqnum %u\n",
pdu->base.seqnum,
- atomic_read(&vhci_hcd->seqnum));
+ seqnum32_fetch(&vhci_hcd->seqnum));
usbip_event_add(ud, VDEV_EVENT_ERROR_TCP);
return;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 11/13] drivers/staging/rtl8188eu: convert stats to use seqnum_ops
2020-11-13 17:46 [PATCH v2 00/13] Introduce seqnum_ops Shuah Khan
@ 2020-11-13 17:46 ` Shuah Khan
2020-11-13 17:46 ` [PATCH v2 02/13] selftests: lib:test_seqnum_ops: add new test for seqnum_ops Shuah Khan
` (11 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw)
To: gregkh, keescook, peterz; +Cc: Shuah Khan, devel, linux-kernel
Sequence Number api provides interfaces for unsigned atomic up counters
leveraging atomic_t and atomic64_t ops underneath. Convert it to use
seqnum_ops.
atomic_t variables used for stats are atomic counters. Convert them to
use seqnum_ops.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 23 ++++++++++++++-----
.../staging/rtl8188eu/include/rtw_mlme_ext.h | 3 ++-
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
index b3eddcb83cd0..d8b5ab1dac26 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -7,6 +7,7 @@
#define _RTW_MLME_EXT_C_
#include <linux/ieee80211.h>
+#include <linux/seqnum_ops.h>
#include <asm/unaligned.h>
#include <osdep_service.h>
@@ -3860,7 +3861,7 @@ static void init_mlme_ext_priv_value(struct adapter *padapter)
_12M_RATE_, _24M_RATE_, 0xff,
};
- atomic_set(&pmlmeext->event_seq, 0);
+ seqnum32_init(&pmlmeext->event_seq);
pmlmeext->mgnt_seq = 0;/* reset to zero when disconnect at client mode */
pmlmeext->cur_channel = padapter->registrypriv.channel;
@@ -4189,7 +4190,9 @@ void report_survey_event(struct adapter *padapter,
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct survey_event);
pc2h_evt_hdr->ID = _Survey_EVT_;
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
psurvey_evt = (struct survey_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
@@ -4239,7 +4242,9 @@ void report_surveydone_event(struct adapter *padapter)
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct surveydone_event);
pc2h_evt_hdr->ID = _SurveyDone_EVT_;
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
psurveydone_evt = (struct surveydone_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
psurveydone_evt->bss_cnt = pmlmeext->sitesurvey_res.bss_cnt;
@@ -4283,7 +4288,9 @@ void report_join_res(struct adapter *padapter, int res)
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct joinbss_event);
pc2h_evt_hdr->ID = _JoinBss_EVT_;
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
pjoinbss_evt = (struct joinbss_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
memcpy((unsigned char *)(&pjoinbss_evt->network.network), &pmlmeinfo->network, sizeof(struct wlan_bssid_ex));
@@ -4333,7 +4340,9 @@ void report_del_sta_event(struct adapter *padapter, unsigned char *MacAddr,
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct stadel_event);
pc2h_evt_hdr->ID = _DelSTA_EVT_;
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
pdel_sta_evt = (struct stadel_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
ether_addr_copy((unsigned char *)(&pdel_sta_evt->macaddr), MacAddr);
@@ -4386,7 +4395,9 @@ void report_add_sta_event(struct adapter *padapter, unsigned char *MacAddr,
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct stassoc_event);
pc2h_evt_hdr->ID = _AddSTA_EVT_;
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
padd_sta_evt = (struct stassoc_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
ether_addr_copy((unsigned char *)(&padd_sta_evt->macaddr), MacAddr);
diff --git a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
index b11a6886a083..09b7e3bb2738 100644
--- a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
+++ b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
@@ -7,6 +7,7 @@
#ifndef __RTW_MLME_EXT_H_
#define __RTW_MLME_EXT_H_
+#include <linux/seqnum_ops.h>
#include <osdep_service.h>
#include <drv_types.h>
#include <wlan_bssdef.h>
@@ -393,7 +394,7 @@ struct p2p_oper_class_map {
struct mlme_ext_priv {
struct adapter *padapter;
u8 mlmeext_init;
- atomic_t event_seq;
+ struct seqnum32 event_seq;
u16 mgnt_seq;
unsigned char cur_channel;
--
2.27.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 11/13] drivers/staging/rtl8188eu: convert stats to use seqnum_ops
@ 2020-11-13 17:46 ` Shuah Khan
0 siblings, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw)
To: gregkh, keescook, peterz; +Cc: devel, linux-kernel, Shuah Khan
Sequence Number api provides interfaces for unsigned atomic up counters
leveraging atomic_t and atomic64_t ops underneath. Convert it to use
seqnum_ops.
atomic_t variables used for stats are atomic counters. Convert them to
use seqnum_ops.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 23 ++++++++++++++-----
.../staging/rtl8188eu/include/rtw_mlme_ext.h | 3 ++-
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
index b3eddcb83cd0..d8b5ab1dac26 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -7,6 +7,7 @@
#define _RTW_MLME_EXT_C_
#include <linux/ieee80211.h>
+#include <linux/seqnum_ops.h>
#include <asm/unaligned.h>
#include <osdep_service.h>
@@ -3860,7 +3861,7 @@ static void init_mlme_ext_priv_value(struct adapter *padapter)
_12M_RATE_, _24M_RATE_, 0xff,
};
- atomic_set(&pmlmeext->event_seq, 0);
+ seqnum32_init(&pmlmeext->event_seq);
pmlmeext->mgnt_seq = 0;/* reset to zero when disconnect at client mode */
pmlmeext->cur_channel = padapter->registrypriv.channel;
@@ -4189,7 +4190,9 @@ void report_survey_event(struct adapter *padapter,
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct survey_event);
pc2h_evt_hdr->ID = _Survey_EVT_;
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
psurvey_evt = (struct survey_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
@@ -4239,7 +4242,9 @@ void report_surveydone_event(struct adapter *padapter)
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct surveydone_event);
pc2h_evt_hdr->ID = _SurveyDone_EVT_;
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
psurveydone_evt = (struct surveydone_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
psurveydone_evt->bss_cnt = pmlmeext->sitesurvey_res.bss_cnt;
@@ -4283,7 +4288,9 @@ void report_join_res(struct adapter *padapter, int res)
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct joinbss_event);
pc2h_evt_hdr->ID = _JoinBss_EVT_;
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
pjoinbss_evt = (struct joinbss_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
memcpy((unsigned char *)(&pjoinbss_evt->network.network), &pmlmeinfo->network, sizeof(struct wlan_bssid_ex));
@@ -4333,7 +4340,9 @@ void report_del_sta_event(struct adapter *padapter, unsigned char *MacAddr,
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct stadel_event);
pc2h_evt_hdr->ID = _DelSTA_EVT_;
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
pdel_sta_evt = (struct stadel_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
ether_addr_copy((unsigned char *)(&pdel_sta_evt->macaddr), MacAddr);
@@ -4386,7 +4395,9 @@ void report_add_sta_event(struct adapter *padapter, unsigned char *MacAddr,
pc2h_evt_hdr = (struct C2HEvent_Header *)(pevtcmd);
pc2h_evt_hdr->len = sizeof(struct stassoc_event);
pc2h_evt_hdr->ID = _AddSTA_EVT_;
- pc2h_evt_hdr->seq = atomic_inc_return(&pmlmeext->event_seq);
+
+ /* seq is unsigned int seq:8 */
+ pc2h_evt_hdr->seq = seqnum32_inc_return(&pmlmeext->event_seq);
padd_sta_evt = (struct stassoc_event *)(pevtcmd + sizeof(struct C2HEvent_Header));
ether_addr_copy((unsigned char *)(&padd_sta_evt->macaddr), MacAddr);
diff --git a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
index b11a6886a083..09b7e3bb2738 100644
--- a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
+++ b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
@@ -7,6 +7,7 @@
#ifndef __RTW_MLME_EXT_H_
#define __RTW_MLME_EXT_H_
+#include <linux/seqnum_ops.h>
#include <osdep_service.h>
#include <drv_types.h>
#include <wlan_bssdef.h>
@@ -393,7 +394,7 @@ struct p2p_oper_class_map {
struct mlme_ext_priv {
struct adapter *padapter;
u8 mlmeext_init;
- atomic_t event_seq;
+ struct seqnum32 event_seq;
u16 mgnt_seq;
unsigned char cur_channel;
--
2.27.0
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 12/13] drivers/staging/unisys/visorhba: convert stats to use seqnum_ops
2020-11-13 17:46 [PATCH v2 00/13] Introduce seqnum_ops Shuah Khan
@ 2020-11-13 17:46 ` Shuah Khan
2020-11-13 17:46 ` [PATCH v2 02/13] selftests: lib:test_seqnum_ops: add new test for seqnum_ops Shuah Khan
` (11 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw)
To: david.kershner, gregkh, keescook, peterz; +Cc: Shuah Khan, devel, linux-kernel
Sequence Number api provides interfaces for unsigned atomic up counters
leveraging atomic_t and atomic64_t ops underneath. Convert it to use
seqnum_ops.
atomic_t variable used for error_count are atomic counters. Convert it to
use seqnum_ops.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
.../staging/unisys/visorhba/visorhba_main.c | 21 ++++++++++---------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c b/drivers/staging/unisys/visorhba/visorhba_main.c
index 7ae5306b92fe..7837eca83758 100644
--- a/drivers/staging/unisys/visorhba/visorhba_main.c
+++ b/drivers/staging/unisys/visorhba/visorhba_main.c
@@ -10,6 +10,7 @@
#include <linux/module.h>
#include <linux/seq_file.h>
#include <linux/visorbus.h>
+#include <linux/seqnum_ops.h>
#include <scsi/scsi.h>
#include <scsi/scsi_host.h>
#include <scsi/scsi_cmnd.h>
@@ -42,7 +43,7 @@ struct visordisk_info {
struct scsi_device *sdev;
u32 valid;
atomic_t ios_threshold;
- atomic_t error_count;
+ struct seqnum32 error_count;
struct visordisk_info *next;
};
@@ -374,8 +375,8 @@ static int visorhba_abort_handler(struct scsi_cmnd *scsicmd)
scsidev = scsicmd->device;
vdisk = scsidev->hostdata;
- if (atomic_read(&vdisk->error_count) < VISORHBA_ERROR_COUNT)
- atomic_inc(&vdisk->error_count);
+ if (seqnum32_fetch(&vdisk->error_count) < VISORHBA_ERROR_COUNT)
+ seqnum32_inc_return(&vdisk->error_count);
else
atomic_set(&vdisk->ios_threshold, IOS_ERROR_THRESHOLD);
rtn = forward_taskmgmt_command(TASK_MGMT_ABORT_TASK, scsidev);
@@ -401,8 +402,8 @@ static int visorhba_device_reset_handler(struct scsi_cmnd *scsicmd)
scsidev = scsicmd->device;
vdisk = scsidev->hostdata;
- if (atomic_read(&vdisk->error_count) < VISORHBA_ERROR_COUNT)
- atomic_inc(&vdisk->error_count);
+ if (seqnum32_fetch(&vdisk->error_count) < VISORHBA_ERROR_COUNT)
+ seqnum32_inc_return(&vdisk->error_count);
else
atomic_set(&vdisk->ios_threshold, IOS_ERROR_THRESHOLD);
rtn = forward_taskmgmt_command(TASK_MGMT_LUN_RESET, scsidev);
@@ -429,8 +430,8 @@ static int visorhba_bus_reset_handler(struct scsi_cmnd *scsicmd)
scsidev = scsicmd->device;
shost_for_each_device(scsidev, scsidev->host) {
vdisk = scsidev->hostdata;
- if (atomic_read(&vdisk->error_count) < VISORHBA_ERROR_COUNT)
- atomic_inc(&vdisk->error_count);
+ if (seqnum32_fetch(&vdisk->error_count) < VISORHBA_ERROR_COUNT)
+ seqnum32_inc_return(&vdisk->error_count);
else
atomic_set(&vdisk->ios_threshold, IOS_ERROR_THRESHOLD);
}
@@ -803,8 +804,8 @@ static void do_scsi_linuxstat(struct uiscmdrsp *cmdrsp,
return;
/* Okay see what our error_count is here.... */
vdisk = scsidev->hostdata;
- if (atomic_read(&vdisk->error_count) < VISORHBA_ERROR_COUNT) {
- atomic_inc(&vdisk->error_count);
+ if (seqnum32_fetch(&vdisk->error_count) < VISORHBA_ERROR_COUNT) {
+ seqnum32_inc_return(&vdisk->error_count);
atomic_set(&vdisk->ios_threshold, IOS_ERROR_THRESHOLD);
}
}
@@ -884,7 +885,7 @@ static void do_scsi_nolinuxstat(struct uiscmdrsp *cmdrsp,
if (atomic_read(&vdisk->ios_threshold) > 0) {
atomic_dec(&vdisk->ios_threshold);
if (atomic_read(&vdisk->ios_threshold) == 0)
- atomic_set(&vdisk->error_count, 0);
+ seqnum32_init(&vdisk->error_count);
}
}
}
--
2.27.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 12/13] drivers/staging/unisys/visorhba: convert stats to use seqnum_ops
@ 2020-11-13 17:46 ` Shuah Khan
0 siblings, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw)
To: david.kershner, gregkh, keescook, peterz; +Cc: devel, linux-kernel, Shuah Khan
Sequence Number api provides interfaces for unsigned atomic up counters
leveraging atomic_t and atomic64_t ops underneath. Convert it to use
seqnum_ops.
atomic_t variable used for error_count are atomic counters. Convert it to
use seqnum_ops.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
.../staging/unisys/visorhba/visorhba_main.c | 21 ++++++++++---------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c b/drivers/staging/unisys/visorhba/visorhba_main.c
index 7ae5306b92fe..7837eca83758 100644
--- a/drivers/staging/unisys/visorhba/visorhba_main.c
+++ b/drivers/staging/unisys/visorhba/visorhba_main.c
@@ -10,6 +10,7 @@
#include <linux/module.h>
#include <linux/seq_file.h>
#include <linux/visorbus.h>
+#include <linux/seqnum_ops.h>
#include <scsi/scsi.h>
#include <scsi/scsi_host.h>
#include <scsi/scsi_cmnd.h>
@@ -42,7 +43,7 @@ struct visordisk_info {
struct scsi_device *sdev;
u32 valid;
atomic_t ios_threshold;
- atomic_t error_count;
+ struct seqnum32 error_count;
struct visordisk_info *next;
};
@@ -374,8 +375,8 @@ static int visorhba_abort_handler(struct scsi_cmnd *scsicmd)
scsidev = scsicmd->device;
vdisk = scsidev->hostdata;
- if (atomic_read(&vdisk->error_count) < VISORHBA_ERROR_COUNT)
- atomic_inc(&vdisk->error_count);
+ if (seqnum32_fetch(&vdisk->error_count) < VISORHBA_ERROR_COUNT)
+ seqnum32_inc_return(&vdisk->error_count);
else
atomic_set(&vdisk->ios_threshold, IOS_ERROR_THRESHOLD);
rtn = forward_taskmgmt_command(TASK_MGMT_ABORT_TASK, scsidev);
@@ -401,8 +402,8 @@ static int visorhba_device_reset_handler(struct scsi_cmnd *scsicmd)
scsidev = scsicmd->device;
vdisk = scsidev->hostdata;
- if (atomic_read(&vdisk->error_count) < VISORHBA_ERROR_COUNT)
- atomic_inc(&vdisk->error_count);
+ if (seqnum32_fetch(&vdisk->error_count) < VISORHBA_ERROR_COUNT)
+ seqnum32_inc_return(&vdisk->error_count);
else
atomic_set(&vdisk->ios_threshold, IOS_ERROR_THRESHOLD);
rtn = forward_taskmgmt_command(TASK_MGMT_LUN_RESET, scsidev);
@@ -429,8 +430,8 @@ static int visorhba_bus_reset_handler(struct scsi_cmnd *scsicmd)
scsidev = scsicmd->device;
shost_for_each_device(scsidev, scsidev->host) {
vdisk = scsidev->hostdata;
- if (atomic_read(&vdisk->error_count) < VISORHBA_ERROR_COUNT)
- atomic_inc(&vdisk->error_count);
+ if (seqnum32_fetch(&vdisk->error_count) < VISORHBA_ERROR_COUNT)
+ seqnum32_inc_return(&vdisk->error_count);
else
atomic_set(&vdisk->ios_threshold, IOS_ERROR_THRESHOLD);
}
@@ -803,8 +804,8 @@ static void do_scsi_linuxstat(struct uiscmdrsp *cmdrsp,
return;
/* Okay see what our error_count is here.... */
vdisk = scsidev->hostdata;
- if (atomic_read(&vdisk->error_count) < VISORHBA_ERROR_COUNT) {
- atomic_inc(&vdisk->error_count);
+ if (seqnum32_fetch(&vdisk->error_count) < VISORHBA_ERROR_COUNT) {
+ seqnum32_inc_return(&vdisk->error_count);
atomic_set(&vdisk->ios_threshold, IOS_ERROR_THRESHOLD);
}
}
@@ -884,7 +885,7 @@ static void do_scsi_nolinuxstat(struct uiscmdrsp *cmdrsp,
if (atomic_read(&vdisk->ios_threshold) > 0) {
atomic_dec(&vdisk->ios_threshold);
if (atomic_read(&vdisk->ios_threshold) == 0)
- atomic_set(&vdisk->error_count, 0);
+ seqnum32_init(&vdisk->error_count);
}
}
}
--
2.27.0
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 13/13] security/integrity/ima: converts stats to seqnum_ops
2020-11-13 17:46 [PATCH v2 00/13] Introduce seqnum_ops Shuah Khan
` (11 preceding siblings ...)
2020-11-13 17:46 ` Shuah Khan
@ 2020-11-13 17:46 ` Shuah Khan
2020-11-14 16:11 ` kernel test robot
12 siblings, 1 reply; 30+ messages in thread
From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw)
To: zohar, dmitry.kasatkin, jmorris, serge, gregkh, keescook, peterz
Cc: Shuah Khan, linux-security-module, linux-integrity, linux-kernel
Sequence Number api provides interfaces for unsigned atomic up counters
leveraging atomic_t and atomic64_t ops underneath. Convert it to use
seqnum_ops.
atomic_t variables used for ima_htable.violations and number of stored
measurements and ios_threshold are atomic counters. Convert them to
seqnum_ops.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
security/integrity/ima/ima.h | 5 +++--
security/integrity/ima/ima_api.c | 3 ++-
security/integrity/ima/ima_fs.c | 5 +++--
security/integrity/ima/ima_queue.c | 7 ++++---
4 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 6ebefec616e4..55fe1d14c67a 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -21,6 +21,7 @@
#include <linux/tpm.h>
#include <linux/audit.h>
#include <crypto/hash_info.h>
+#include <linux/seqnum_ops.h>
#include "../integrity.h"
@@ -174,8 +175,8 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
extern spinlock_t ima_queue_lock;
struct ima_h_table {
- atomic_long_t len; /* number of stored measurements in the list */
- atomic_long_t violations;
+ struct seqnum64 len; /* number of stored measurements in the list */
+ struct seqnum64 violations;
struct hlist_head queue[IMA_MEASURE_HTABLE_SIZE];
};
extern struct ima_h_table ima_htable;
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 4f39fb93f278..c6c442b93ce3 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -14,6 +14,7 @@
#include <linux/xattr.h>
#include <linux/evm.h>
#include <linux/iversion.h>
+#include <linux/seqnum_ops.h>
#include "ima.h"
@@ -144,7 +145,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
int result;
/* can overflow, only indicator */
- atomic_long_inc(&ima_htable.violations);
+ seqnum64_inc_return(&ima_htable.violations);
result = ima_alloc_init_template(&event_data, &entry, NULL);
if (result < 0) {
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index ea8ff8a07b36..83a0d33e6f70 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -21,6 +21,7 @@
#include <linux/rcupdate.h>
#include <linux/parser.h>
#include <linux/vmalloc.h>
+#include <linux/seqnum_ops.h>
#include "ima.h"
@@ -39,12 +40,12 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
static int valid_policy = 1;
static ssize_t ima_show_htable_value(char __user *buf, size_t count,
- loff_t *ppos, atomic_long_t *val)
+ loff_t *ppos, struct seqnum64 *val)
{
char tmpbuf[32]; /* greater than largest 'long' string value */
ssize_t len;
- len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", atomic_long_read(val));
+ len = scnprintf(tmpbuf, sizeof(tmpbuf), "%llu\n", seqnum64_fetch(val));
return simple_read_from_buffer(buf, count, ppos, tmpbuf, len);
}
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index c096ef8945c7..38c31bc62358 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -17,6 +17,7 @@
#include <linux/rculist.h>
#include <linux/slab.h>
+#include <linux/seqnum_ops.h>
#include "ima.h"
#define AUDIT_CAUSE_LEN_MAX 32
@@ -33,8 +34,8 @@ static unsigned long binary_runtime_size = ULONG_MAX;
/* key: inode (before secure-hashing a file) */
struct ima_h_table ima_htable = {
- .len = ATOMIC_LONG_INIT(0),
- .violations = ATOMIC_LONG_INIT(0),
+ .len = SEQNUM_INIT(0),
+ .violations = SEQNUM_INIT(0),
.queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
};
@@ -106,7 +107,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
INIT_LIST_HEAD(&qe->later);
list_add_tail_rcu(&qe->later, &ima_measurements);
- atomic_long_inc(&ima_htable.len);
+ seqnum64_inc_return(&ima_htable.len);
if (update_htable) {
key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
--
2.27.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 13/13] security/integrity/ima: converts stats to seqnum_ops
2020-11-13 17:46 ` [PATCH v2 13/13] security/integrity/ima: converts stats to seqnum_ops Shuah Khan
@ 2020-11-14 16:11 ` kernel test robot
0 siblings, 0 replies; 30+ messages in thread
From: kernel test robot @ 2020-11-14 16:11 UTC (permalink / raw)
To: Shuah Khan, zohar, dmitry.kasatkin, jmorris, serge, gregkh,
keescook, peterz
Cc: kbuild-all, Shuah Khan, linux-security-module, linux-integrity
[-- Attachment #1: Type: text/plain, Size: 7214 bytes --]
Hi Shuah,
I love your patch! Yet something to improve:
[auto build test ERROR on staging/staging-testing]
[also build test ERROR on integrity/next-integrity char-misc/char-misc-testing usb/usb-testing linus/master v5.10-rc3 next-20201113]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Shuah-Khan/Introduce-seqnum_ops/20201114-014959
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git f4acd33c446b2ba97f1552a4da90050109d01ca7
config: nios2-randconfig-r023-20201114 (attached as .config)
compiler: nios2-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/b86077d3629fe6d16070d95b8331344258dcaed2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Shuah-Khan/Introduce-seqnum_ops/20201114-014959
git checkout b86077d3629fe6d16070d95b8331344258dcaed2
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nios2
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
In file included from security/integrity/ima/ima_fs.c:26:
security/integrity/ima/ima.h:178:18: error: field 'len' has incomplete type
178 | struct seqnum64 len; /* number of stored measurements in the list */
| ^~~
security/integrity/ima/ima.h:179:18: error: field 'violations' has incomplete type
179 | struct seqnum64 violations;
| ^~~~~~~~~~
security/integrity/ima/ima_fs.c: In function 'ima_show_htable_value':
>> security/integrity/ima/ima_fs.c:48:52: error: implicit declaration of function 'seqnum64_fetch'; did you mean 'seqnum32_fetch'? [-Werror=implicit-function-declaration]
48 | len = scnprintf(tmpbuf, sizeof(tmpbuf), "%llu\n", seqnum64_fetch(val));
| ^~~~~~~~~~~~~~
| seqnum32_fetch
>> security/integrity/ima/ima_fs.c:48:46: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 4 has type 'int' [-Wformat=]
48 | len = scnprintf(tmpbuf, sizeof(tmpbuf), "%llu\n", seqnum64_fetch(val));
| ~~~^ ~~~~~~~~~~~~~~~~~~~
| | |
| | int
| long long unsigned int
| %u
security/integrity/ima/ima_fs.c: In function 'ima_show_htable_violations':
security/integrity/ima/ima_fs.c:57:1: error: control reaches end of non-void function [-Werror=return-type]
57 | }
| ^
security/integrity/ima/ima_fs.c: In function 'ima_show_measurements_count':
security/integrity/ima/ima_fs.c:70:1: error: control reaches end of non-void function [-Werror=return-type]
70 | }
| ^
cc1: some warnings being treated as errors
--
In file included from security/integrity/ima/ima_queue.c:21:
security/integrity/ima/ima.h:178:18: error: field 'len' has incomplete type
178 | struct seqnum64 len; /* number of stored measurements in the list */
| ^~~
security/integrity/ima/ima.h:179:18: error: field 'violations' has incomplete type
179 | struct seqnum64 violations;
| ^~~~~~~~~~
In file included from security/integrity/ima/ima_queue.c:20:
include/linux/seqnum_ops.h:40:27: error: field name not in record or union initializer
40 | #define SEQNUM_INIT(i) { .seqnum = ATOMIC_INIT(i) }
| ^
security/integrity/ima/ima_queue.c:37:9: note: in expansion of macro 'SEQNUM_INIT'
37 | .len = SEQNUM_INIT(0),
| ^~~~~~~~~~~
include/linux/seqnum_ops.h:40:27: note: (near initialization for 'ima_htable.len')
40 | #define SEQNUM_INIT(i) { .seqnum = ATOMIC_INIT(i) }
| ^
security/integrity/ima/ima_queue.c:37:9: note: in expansion of macro 'SEQNUM_INIT'
37 | .len = SEQNUM_INIT(0),
| ^~~~~~~~~~~
include/linux/seqnum_ops.h:40:27: error: field name not in record or union initializer
40 | #define SEQNUM_INIT(i) { .seqnum = ATOMIC_INIT(i) }
| ^
security/integrity/ima/ima_queue.c:38:16: note: in expansion of macro 'SEQNUM_INIT'
38 | .violations = SEQNUM_INIT(0),
| ^~~~~~~~~~~
include/linux/seqnum_ops.h:40:27: note: (near initialization for 'ima_htable.violations')
40 | #define SEQNUM_INIT(i) { .seqnum = ATOMIC_INIT(i) }
| ^
security/integrity/ima/ima_queue.c:38:16: note: in expansion of macro 'SEQNUM_INIT'
38 | .violations = SEQNUM_INIT(0),
| ^~~~~~~~~~~
security/integrity/ima/ima_queue.c: In function 'ima_add_digest_entry':
>> security/integrity/ima/ima_queue.c:110:2: error: implicit declaration of function 'seqnum64_inc_return'; did you mean 'seqnum32_inc_return'? [-Werror=implicit-function-declaration]
110 | seqnum64_inc_return(&ima_htable.len);
| ^~~~~~~~~~~~~~~~~~~
| seqnum32_inc_return
cc1: some warnings being treated as errors
--
In file included from security/integrity/ima/ima_api.c:19:
security/integrity/ima/ima.h:178:18: error: field 'len' has incomplete type
178 | struct seqnum64 len; /* number of stored measurements in the list */
| ^~~
security/integrity/ima/ima.h:179:18: error: field 'violations' has incomplete type
179 | struct seqnum64 violations;
| ^~~~~~~~~~
security/integrity/ima/ima_api.c: In function 'ima_add_violation':
>> security/integrity/ima/ima_api.c:148:2: error: implicit declaration of function 'seqnum64_inc_return'; did you mean 'seqnum32_inc_return'? [-Werror=implicit-function-declaration]
148 | seqnum64_inc_return(&ima_htable.violations);
| ^~~~~~~~~~~~~~~~~~~
| seqnum32_inc_return
cc1: some warnings being treated as errors
vim +48 security/integrity/ima/ima_fs.c
41
42 static ssize_t ima_show_htable_value(char __user *buf, size_t count,
43 loff_t *ppos, struct seqnum64 *val)
44 {
45 char tmpbuf[32]; /* greater than largest 'long' string value */
46 ssize_t len;
47
> 48 len = scnprintf(tmpbuf, sizeof(tmpbuf), "%llu\n", seqnum64_fetch(val));
49 return simple_read_from_buffer(buf, count, ppos, tmpbuf, len);
50 }
51
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24371 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 13/13] security/integrity/ima: converts stats to seqnum_ops
@ 2020-11-14 16:11 ` kernel test robot
0 siblings, 0 replies; 30+ messages in thread
From: kernel test robot @ 2020-11-14 16:11 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 7344 bytes --]
Hi Shuah,
I love your patch! Yet something to improve:
[auto build test ERROR on staging/staging-testing]
[also build test ERROR on integrity/next-integrity char-misc/char-misc-testing usb/usb-testing linus/master v5.10-rc3 next-20201113]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Shuah-Khan/Introduce-seqnum_ops/20201114-014959
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git f4acd33c446b2ba97f1552a4da90050109d01ca7
config: nios2-randconfig-r023-20201114 (attached as .config)
compiler: nios2-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/b86077d3629fe6d16070d95b8331344258dcaed2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Shuah-Khan/Introduce-seqnum_ops/20201114-014959
git checkout b86077d3629fe6d16070d95b8331344258dcaed2
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nios2
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
In file included from security/integrity/ima/ima_fs.c:26:
security/integrity/ima/ima.h:178:18: error: field 'len' has incomplete type
178 | struct seqnum64 len; /* number of stored measurements in the list */
| ^~~
security/integrity/ima/ima.h:179:18: error: field 'violations' has incomplete type
179 | struct seqnum64 violations;
| ^~~~~~~~~~
security/integrity/ima/ima_fs.c: In function 'ima_show_htable_value':
>> security/integrity/ima/ima_fs.c:48:52: error: implicit declaration of function 'seqnum64_fetch'; did you mean 'seqnum32_fetch'? [-Werror=implicit-function-declaration]
48 | len = scnprintf(tmpbuf, sizeof(tmpbuf), "%llu\n", seqnum64_fetch(val));
| ^~~~~~~~~~~~~~
| seqnum32_fetch
>> security/integrity/ima/ima_fs.c:48:46: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 4 has type 'int' [-Wformat=]
48 | len = scnprintf(tmpbuf, sizeof(tmpbuf), "%llu\n", seqnum64_fetch(val));
| ~~~^ ~~~~~~~~~~~~~~~~~~~
| | |
| | int
| long long unsigned int
| %u
security/integrity/ima/ima_fs.c: In function 'ima_show_htable_violations':
security/integrity/ima/ima_fs.c:57:1: error: control reaches end of non-void function [-Werror=return-type]
57 | }
| ^
security/integrity/ima/ima_fs.c: In function 'ima_show_measurements_count':
security/integrity/ima/ima_fs.c:70:1: error: control reaches end of non-void function [-Werror=return-type]
70 | }
| ^
cc1: some warnings being treated as errors
--
In file included from security/integrity/ima/ima_queue.c:21:
security/integrity/ima/ima.h:178:18: error: field 'len' has incomplete type
178 | struct seqnum64 len; /* number of stored measurements in the list */
| ^~~
security/integrity/ima/ima.h:179:18: error: field 'violations' has incomplete type
179 | struct seqnum64 violations;
| ^~~~~~~~~~
In file included from security/integrity/ima/ima_queue.c:20:
include/linux/seqnum_ops.h:40:27: error: field name not in record or union initializer
40 | #define SEQNUM_INIT(i) { .seqnum = ATOMIC_INIT(i) }
| ^
security/integrity/ima/ima_queue.c:37:9: note: in expansion of macro 'SEQNUM_INIT'
37 | .len = SEQNUM_INIT(0),
| ^~~~~~~~~~~
include/linux/seqnum_ops.h:40:27: note: (near initialization for 'ima_htable.len')
40 | #define SEQNUM_INIT(i) { .seqnum = ATOMIC_INIT(i) }
| ^
security/integrity/ima/ima_queue.c:37:9: note: in expansion of macro 'SEQNUM_INIT'
37 | .len = SEQNUM_INIT(0),
| ^~~~~~~~~~~
include/linux/seqnum_ops.h:40:27: error: field name not in record or union initializer
40 | #define SEQNUM_INIT(i) { .seqnum = ATOMIC_INIT(i) }
| ^
security/integrity/ima/ima_queue.c:38:16: note: in expansion of macro 'SEQNUM_INIT'
38 | .violations = SEQNUM_INIT(0),
| ^~~~~~~~~~~
include/linux/seqnum_ops.h:40:27: note: (near initialization for 'ima_htable.violations')
40 | #define SEQNUM_INIT(i) { .seqnum = ATOMIC_INIT(i) }
| ^
security/integrity/ima/ima_queue.c:38:16: note: in expansion of macro 'SEQNUM_INIT'
38 | .violations = SEQNUM_INIT(0),
| ^~~~~~~~~~~
security/integrity/ima/ima_queue.c: In function 'ima_add_digest_entry':
>> security/integrity/ima/ima_queue.c:110:2: error: implicit declaration of function 'seqnum64_inc_return'; did you mean 'seqnum32_inc_return'? [-Werror=implicit-function-declaration]
110 | seqnum64_inc_return(&ima_htable.len);
| ^~~~~~~~~~~~~~~~~~~
| seqnum32_inc_return
cc1: some warnings being treated as errors
--
In file included from security/integrity/ima/ima_api.c:19:
security/integrity/ima/ima.h:178:18: error: field 'len' has incomplete type
178 | struct seqnum64 len; /* number of stored measurements in the list */
| ^~~
security/integrity/ima/ima.h:179:18: error: field 'violations' has incomplete type
179 | struct seqnum64 violations;
| ^~~~~~~~~~
security/integrity/ima/ima_api.c: In function 'ima_add_violation':
>> security/integrity/ima/ima_api.c:148:2: error: implicit declaration of function 'seqnum64_inc_return'; did you mean 'seqnum32_inc_return'? [-Werror=implicit-function-declaration]
148 | seqnum64_inc_return(&ima_htable.violations);
| ^~~~~~~~~~~~~~~~~~~
| seqnum32_inc_return
cc1: some warnings being treated as errors
vim +48 security/integrity/ima/ima_fs.c
41
42 static ssize_t ima_show_htable_value(char __user *buf, size_t count,
43 loff_t *ppos, struct seqnum64 *val)
44 {
45 char tmpbuf[32]; /* greater than largest 'long' string value */
46 ssize_t len;
47
> 48 len = scnprintf(tmpbuf, sizeof(tmpbuf), "%llu\n", seqnum64_fetch(val));
49 return simple_read_from_buffer(buf, count, ppos, tmpbuf, len);
50 }
51
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 24371 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread