[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCH 4/4] include/uk: merge two bitops functions



Hi Yuri,

looking good, just some typos that can be fixed on committing (see below), otherwise:

Reviewed-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>

On 10/10/2018 02:43 PM, Yuri Volchkov wrote:
Unikraft currently has two sets of bit operations. This patch merges
them together

Add a full stop at the end of the sentence.


Signed-off-by: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>
---
  include/uk/arch/atomic.h | 106 ----------------------------------
  include/uk/bitops.h      | 120 +++++++++++++++++++++++++++++++++------
  2 files changed, 103 insertions(+), 123 deletions(-)

diff --git a/include/uk/arch/atomic.h b/include/uk/arch/atomic.h
index 44a71e6..ce8f6e5 100644
--- a/include/uk/arch/atomic.h
+++ b/include/uk/arch/atomic.h
@@ -83,112 +83,6 @@ extern "C" {
                    : old;                                                     \
        })
-/**
- * test_and_clear_bit - Clear a bit and return its old value
- * @nr: Bit to clear
- * @addr: Address to count from
- *
- * Note that @nr may be almost arbitrarily large; this function is not
- * restricted to acting on a single-word quantity.
- *
- * This operation is atomic.
- * If you need a memory barrier, use synch_test_and_clear_bit instead.
- */
-static inline int ukarch_test_and_clr_bit(unsigned int nr, volatile void *byte)
-{
-       volatile __u8 *addr = ((__u8 *)byte) + (nr >> 3);
-       __u8 bit = 1 << (nr & 7);
-       __u8 orig;
-
-       orig = __atomic_fetch_and(addr, ~bit, __ATOMIC_RELAXED);
-
-       return (orig & bit) != 0;
-}
-
-/**
- * Atomically set a bit and return the old value.
- * Similar to test_and_clear_bit.
- */
-static inline int ukarch_test_and_set_bit(unsigned int nr, volatile void *byte)
-{
-       volatile __u8 *addr = ((__u8 *)byte) + (nr >> 3);
-       __u8 bit = 1 << (nr & 7);
-       __u8 orig;
-
-       orig = __atomic_fetch_or(addr, bit, __ATOMIC_RELAXED);
-
-       return (orig & bit) != 0;
-}
-
-/**
- * Test whether a bit is set.
- */
-static inline int ukarch_test_bit(unsigned int nr,
-                                       const volatile unsigned long *byte)
-{
-       const volatile __u8 *ptr = (const __u8 *)byte;
-       int ret =  ((1 << (nr & 7)) & (ptr[nr >> 3])) != 0;
-
-       return ret;
-}
-
-/**
- * Atomically set a bit in memory (like test_and_set_bit but discards result).
- */
-static inline void ukarch_set_bit(unsigned int nr,
-                                       volatile unsigned long *byte)
-{
-       ukarch_test_and_set_bit(nr, byte);
-}
-
-/**
- * Atomically clear a bit in memory (like test_and_clear_bit but discards
- * result).
- */
-static inline void ukarch_clr_bit(unsigned int nr,
-                                       volatile unsigned long *byte)
-{
-       ukarch_test_and_clr_bit(nr, byte);
-}
-
-/* As test_and_clear_bit, but using __ATOMIC_SEQ_CST */
-static inline int ukarch_test_and_clr_bit_sync(unsigned int nr,
-                                               volatile void *byte)
-{
-       volatile __u8 *addr = ((__u8 *)byte) + (nr >> 3);
-       __u8 bit = 1 << (nr & 7);
-       __u8 orig;
-
-       orig = __atomic_fetch_and(addr, ~bit, __ATOMIC_SEQ_CST);
-
-       return (orig & bit) != 0;
-}
-
-/* As test_and_set_bit, but using __ATOMIC_SEQ_CST */
-static inline int ukarch_test_and_set_bit_sync(unsigned int nr,
-                                               volatile void *byte)
-{
-       volatile __u8 *addr = ((__u8 *)byte) + (nr >> 3);
-       __u8 bit = 1 << (nr & 7);
-       __u8 orig;
-
-       orig = __atomic_fetch_or(addr, bit, __ATOMIC_SEQ_CST);
-
-       return (orig & bit) != 0;
-}
-
-/* As set_bit, but using __ATOMIC_SEQ_CST */
-static inline void ukarch_set_bit_sync(unsigned int nr, volatile void *byte)
-{
-       ukarch_test_and_set_bit_sync(nr, byte);
-}
-
-/* As clear_bit, but using __ATOMIC_SEQ_CST */
-static inline void ukarch_clr_bit_sync(unsigned int nr, volatile void *byte)
-{
-       ukarch_test_and_clr_bit_sync(nr, byte);
-}
-
  #ifdef __cplusplus
  }
  #endif
diff --git a/include/uk/bitops.h b/include/uk/bitops.h
index 5a28410..bca9c74 100644
--- a/include/uk/bitops.h
+++ b/include/uk/bitops.h
@@ -242,38 +242,92 @@ uk_find_next_zero_bit(const unsigned long *addr, unsigned 
long size,
        return (bit);
  }
-/* uk_set_bit and uk_clear_bit are atomic and protected against
- * reordering (do barriers), while the underscored (__*) versions of
- * them don't (not atomic).
+/**
+ * uk_test_and_clear_bit - Atomically clear a bit and return its old value
+ * @nr: Bit to clear
+ * @addr: Address to count from
+ *
+ * Note that @nr may be almost arbitrarily large; this function is not
+ * restricted to acting on a single-word quantity.
   */
-#define __uk_set_bit(i, a)        ukarch_set_bit(i, a)
-#define uk_set_bit(i, a)          ukarch_set_bit_sync(i, a)
-#define __uk_clear_bit(i, a)      ukarch_clr_bit(i, a)
-#define uk_clear_bit(i, a)        ukarch_clr_bit_sync(i, a)
-#define uk_test_bit(i, a)         ukarch_test_bit(i, a)
-
  static inline int
-uk_test_and_clear_bit(long bit, volatile unsigned long *var)
+uk_test_and_clear_bit(long nr, volatile unsigned long *addr)
  {
-       return ukarch_test_and_clr_bit_sync(bit, (volatile void *) var);
+       volatile __u8 *ptr = ((__u8 *) addr) + (nr >> 3);
+       __u8 mask = 1 << (nr & 7);
+       __u8 orig;
+
+       orig = __atomic_fetch_and(ptr, ~mask, __ATOMIC_SEQ_CST);
+
+       return (orig & mask) != 0;
  }
+/**
+ * __uk_test_and_clear_bit - Clear a bit and return its old value
+ * @nr: Bit to clear
+ * @addr: Address to count from
+ *
+ * Note that @nr may be almost arbitrarily large; this function is not
+ * restricted to acting on a single-word quantity.
+ *
+ * This operation is not atomic and can be reordered. If two
+ * __uk_test_and_clear_bit are executing in parallel, it could be that
+ * only one of them will be successful

Add a full stop at the end of the sentence.

+ */
  static inline int
-__uk_test_and_clear_bit(long bit, volatile unsigned long *var)
+__uk_test_and_clear_bit(long nr, volatile unsigned long *addr)
  {
-       return ukarch_test_and_clr_bit(bit, (volatile void *) var);
+       volatile __u8 *ptr = ((__u8 *) addr) + (nr >> 3);
+       __u8 mask = 1 << (nr & 7);
+       __u8 orig;
+
+       orig = __atomic_fetch_and(ptr, ~mask, __ATOMIC_RELAXED);
+
+       return (orig & mask) != 0;
  }
+/**
+ * uk_test_and_set_bit - Atomically set a bit and return its old value
+ * @nr: Bit to clear
+ * @addr: Address to count from
+ *
+ * Note that @nr may be almost arbitrarily large; this function is not
+ * restricted to acting on a single-word quantity.
+ */
  static inline int
-uk_test_and_set_bit(long bit, volatile unsigned long *var)
+uk_test_and_set_bit(long nr, volatile unsigned long *addr)
  {
-       return ukarch_test_and_set_bit_sync(bit, (volatile void *) var);
+       volatile __u8 *ptr = ((__u8 *) addr) + (nr >> 3);
+       __u8 mask = 1 << (nr & 7);
+       __u8 orig;
+
+       orig = __atomic_fetch_or(ptr, mask, __ATOMIC_SEQ_CST);
+
+       return (orig & mask) != 0;
  }
+/**
+ * __uk_test_and_set_bit - Set a bit and return its old value
+ * @nr: Bit to clear
+ * @addr: Address to count from
+ *
+ * Note that @nr may be almost arbitrarily large; this function is not
+ * restricted to acting on a single-word quantity.
+ *
+ * This operation is not atomic and can be reordered. If two
+ * __uk_test_and_set_bit are executing in parallel, it could be that
+ * only one of them will be successful

Add a full stop at the end of the sentence.

+ */
  static inline int
-__uk_test_and_set_bit(long bit, volatile unsigned long *var)
+__uk_test_and_set_bit(long nr, volatile unsigned long *addr)
  {
-       return ukarch_test_and_set_bit(bit, (volatile void *) var);
+       volatile __u8 *ptr = ((__u8 *) addr) + (nr >> 3);
+       __u8 mask = 1 << (nr & 7);
+       __u8 orig;
+
+       orig = __atomic_fetch_or(ptr, mask, __ATOMIC_RELAXED);
+
+       return (orig & mask) != 0;
  }
enum {
@@ -282,6 +336,38 @@ enum {
        REG_OP_RELEASE,
  };
+/* uk_set_bit and uk_clear_bit are atomic and protected against
+ * reordering (do barriers), while the underscored (__*) versions of
+ * them are not (not atomic).
+ */
+static inline void uk_set_bit(long nr, volatile unsigned long *addr)
+{
+       uk_test_and_set_bit(nr, addr);
+}
+
+static inline void __uk_set_bit(long nr, volatile unsigned long *addr)
+{
+       __uk_test_and_set_bit(nr, addr);
+}
+
+static inline void uk_clear_bit(long nr, volatile unsigned long *addr)
+{
+       uk_test_and_clear_bit(nr, addr);
+}
+
+static inline void __uk_clear_bit(long nr, volatile unsigned long *addr)
+{
+       __uk_test_and_clear_bit(nr, addr);
+}
+
+static inline int uk_test_bit(int nr, const volatile unsigned long *addr)
+{
+       const volatile __u8 *ptr = (const __u8 *) addr;
+       int ret =  ((1 << (nr & 7)) & (ptr[nr >> 3])) != 0;
+
+       return ret;
+}
+
  static inline int
  __uk_bitopts_reg_op(unsigned long *bitmap, int pos, int order, int reg_op)
  {


--
Dr. Florian Schmidt
フローリアン・シュミット
Research Scientist,
Systems and Machine Learning Group
NEC Laboratories Europe
Kurfürsten-Anlage 36, D-69115 Heidelberg
Tel.     +49 (0)6221 4342-265
Fax:     +49 (0)6221 4342-155
e-mail:  florian.schmidt@xxxxxxxxx
============================================================
Registered at Amtsgericht Mannheim, Germany, HRB728558

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.