All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/13] Introduce seqnum_ops
@ 2020-11-13 17:46 Shuah Khan
  2020-11-13 17:46 ` [PATCH v2 01/13] seqnum_ops: Introduce Sequence Number Ops Shuah Khan
                   ` (12 more replies)
  0 siblings, 13 replies; 30+ messages in thread
From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw)
  To: corbet, keescook, gregkh, peterz, rafael, lenb, james.morse,
	tony.luck, bp, minyard, arnd, mchehab, rric, valentina.manea.m,
	shuah, zohar, dmitry.kasatkin, jmorris, serge
  Cc: Shuah Khan, linux-doc, linux-kernel, linux-kselftest, linux-acpi,
	openipmi-developer, linux-edac, linux-usb, linux-integrity,
	linux-security-module

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. It becomes 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.

Changes since v1:
- Removed dec based on Greg KH's comments
- Removed read/set/inc based on the discussion with Peter Zijlstra
- Interfaces are restricted to init, increment and return new value,
  and fetch current value.
- Interfaces return u32 and u64 - a few reviewers suggested unsigned.
  After reviewing a few use-cases, I determined this is a good path
  forward. It adds unsigned atomic support that doesn't exist now,
  and simplifies code in drivers that currently convert atomic_t return
  values to unsigned. All the drivers changes included in this series
  used to convert atomic_t returns to unsigned.

Patch v1 thread:
https://lore.kernel.org/lkml/cover.1605027593.git.skhan@linuxfoundation.org/

Counters thread:
lore.kernel.org/lkml/cover.1602209970.git.skhan@linuxfoundation.org

Shuah Khan (13):
  seqnum_ops: Introduce Sequence Number Ops
  selftests: lib:test_seqnum_ops: add new test for seqnum_ops
  drivers/acpi: convert seqno seqnum_ops
  drivers/acpi/apei: convert seqno to seqnum_ops
  drivers/base/test/test_async_driver_probe: convert to use seqnum_ops
  drivers/char/ipmi: convert stats to use seqnum_ops
  drivers/edac: convert pci counters to seqnum_ops
  drivers/oprofile: convert stats to use seqnum_ops
  drivers/staging/rtl8723bs: convert stats to use seqnum_ops
  usb: usbip/vhci: convert seqno to seqnum_ops
  drivers/staging/rtl8188eu: convert stats to use seqnum_ops
  drivers/staging/unisys/visorhba: convert stats to use seqnum_ops
  security/integrity/ima: converts stats to seqnum_ops

 Documentation/core-api/atomic_ops.rst         |   4 +
 Documentation/core-api/index.rst              |   1 +
 Documentation/core-api/seqnum_ops.rst         |  89 +++++++++++++
 MAINTAINERS                                   |   8 ++
 drivers/acpi/acpi_extlog.c                    |   8 +-
 drivers/acpi/apei/ghes.c                      |   8 +-
 drivers/base/test/test_async_driver_probe.c   |  28 +++--
 drivers/char/ipmi/ipmi_msghandler.c           |   9 +-
 drivers/char/ipmi/ipmi_si_intf.c              |   9 +-
 drivers/char/ipmi/ipmi_ssif.c                 |   9 +-
 drivers/edac/edac_pci.h                       |   5 +-
 drivers/edac/edac_pci_sysfs.c                 |  30 ++---
 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 +-
 drivers/staging/rtl8188eu/core/rtw_mlme_ext.c |  23 +++-
 .../staging/rtl8188eu/include/rtw_mlme_ext.h  |   3 +-
 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 +-
 .../staging/unisys/visorhba/visorhba_main.c   |  21 ++--
 drivers/usb/usbip/vhci.h                      |   3 +-
 drivers/usb/usbip/vhci_hcd.c                  |   7 +-
 drivers/usb/usbip/vhci_rx.c                   |   5 +-
 include/linux/oprofile.h                      |   3 +-
 include/linux/seqnum_ops.h                    | 118 +++++++++++++++++
 lib/Kconfig                                   |   9 ++
 lib/Makefile                                  |   1 +
 lib/test_seqnum_ops.c                         | 119 ++++++++++++++++++
 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 +-
 tools/testing/selftests/lib/Makefile          |   1 +
 tools/testing/selftests/lib/config            |   1 +
 .../testing/selftests/lib/test_seqnum_ops.sh  |  10 ++
 40 files changed, 524 insertions(+), 110 deletions(-)
 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
 create mode 100755 tools/testing/selftests/lib/test_seqnum_ops.sh

-- 
2.27.0


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [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

* [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 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 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

* 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-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: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-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

* 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

end of thread, other threads:[~2020-11-17 18:24 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 21:03   ` Matthew Wilcox
2020-11-16 14:49     ` Peter Zijlstra
2020-11-16 14:58       ` Peter Zijlstra
2020-11-17 14:50         ` Shuah Khan
2020-11-17 15:21           ` Peter Zijlstra
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
2020-11-16 14:53   ` Peter Zijlstra
2020-11-17 16:15     ` 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 ` [PATCH v2 03/13] drivers/acpi: convert seqno seqnum_ops Shuah Khan
2020-11-13 17:46 ` [PATCH v2 04/13] drivers/acpi/apei: convert seqno to seqnum_ops 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
2020-11-13 17:46 ` [PATCH v2 06/13] drivers/char/ipmi: convert stats " Shuah Khan
2020-11-13 17:46 ` [PATCH v2 07/13] drivers/edac: convert pci counters to seqnum_ops Shuah Khan
2020-11-13 17:46 ` [PATCH v2 08/13] drivers/oprofile: convert stats to use seqnum_ops Shuah Khan
2020-11-13 17:46 ` [PATCH v2 09/13] drivers/staging/rtl8723bs: " Shuah Khan
2020-11-13 17:46   ` Shuah Khan
2020-11-13 17:46 ` [PATCH v2 10/13] usb: usbip/vhci: convert seqno to seqnum_ops Shuah Khan
2020-11-13 17:46 ` [PATCH v2 11/13] drivers/staging/rtl8188eu: convert stats to use seqnum_ops Shuah Khan
2020-11-13 17:46   ` Shuah Khan
2020-11-13 17:46 ` [PATCH v2 12/13] drivers/staging/unisys/visorhba: " Shuah Khan
2020-11-13 17:46   ` Shuah Khan
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
2020-11-14 16:11     ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.