[PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[rao . shoaib]

From: Rao Shoaib 

kfree_call_rcu does not belong in linux/rcupdate.h and should be moved to
slab_common.c

Signed-off-by: Rao Shoaib 
---
 include/linux/rcupdate.h | 43 +++
 include/linux/rcutree.h  |  2 --
 include/linux/slab.h | 42 ++
 kernel/rcu/tree.c| 24 ++--
 mm/slab_common.c | 10 ++
 5 files changed, 65 insertions(+), 56 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 043d047..6338fb6 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -55,6 +55,9 @@ void call_rcu(struct rcu_head *head, rcu_callback_t func);
 #definecall_rcucall_rcu_sched
 #endif /* #else #ifdef CONFIG_PREEMPT_RCU */
 
+/* only for use by kfree_call_rcu() */
+void call_rcu_lazy(struct rcu_head *head, rcu_callback_t func);
+
 void call_rcu_bh(struct rcu_head *head, rcu_callback_t func);
 void call_rcu_sched(struct rcu_head *head, rcu_callback_t func);
 void synchronize_sched(void);
@@ -837,45 +840,6 @@ static inline notrace void 
rcu_read_unlock_sched_notrace(void)
 #define __is_kfree_rcu_offset(offset) ((offset) < 4096)
 
 /*
- * Helper macro for kfree_rcu() to prevent argument-expansion eyestrain.
- */
-#define __kfree_rcu(head, offset) \
-   do { \
-   BUILD_BUG_ON(!__is_kfree_rcu_offset(offset)); \
-   kfree_call_rcu(head, (rcu_callback_t)(unsigned long)(offset)); \
-   } while (0)
-
-/**
- * kfree_rcu() - kfree an object after a grace period.
- * @ptr:   pointer to kfree
- * @rcu_head:  the name of the struct rcu_head within the type of @ptr.
- *
- * Many rcu callbacks functions just call kfree() on the base structure.
- * These functions are trivial, but their size adds up, and furthermore
- * when they are used in a kernel module, that module must invoke the
- * high-latency rcu_barrier() function at module-unload time.
- *
- * The kfree_rcu() function handles this issue.  Rather than encoding a
- * function address in the embedded rcu_head structure, kfree_rcu() instead
- * encodes the offset of the rcu_head structure within the base structure.
- * Because the functions are not allowed in the low-order 4096 bytes of
- * kernel virtual memory, offsets up to 4095 bytes can be accommodated.
- * If the offset is larger than 4095 bytes, a compile-time error will
- * be generated in __kfree_rcu().  If this error is triggered, you can
- * either fall back to use of call_rcu() or rearrange the structure to
- * position the rcu_head structure into the first 4096 bytes.
- *
- * Note that the allowable offset might decrease in the future, for example,
- * to allow something like kmem_cache_free_rcu().
- *
- * The BUILD_BUG_ON check must not involve any function calls, hence the
- * checks are done in macros here.
- */
-#define kfree_rcu(ptr, rcu_head)   \
-   __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
-
-
-/*
  * Place this after a lock-acquisition primitive to guarantee that
  * an UNLOCK+LOCK pair acts as a full barrier.  This guarantee applies
  * if the UNLOCK and LOCK are executed by the same CPU or if the
@@ -887,5 +851,4 @@ static inline notrace void 
rcu_read_unlock_sched_notrace(void)
 #define smp_mb__after_unlock_lock()do { } while (0)
 #endif /* #else #ifdef CONFIG_ARCH_WEAK_RELEASE_ACQUIRE */
 
-
 #endif /* __LINUX_RCUPDATE_H */
diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
index fd996cd..567ef58 100644
--- a/include/linux/rcutree.h
+++ b/include/linux/rcutree.h
@@ -48,8 +48,6 @@ void synchronize_rcu_bh(void);
 void synchronize_sched_expedited(void);
 void synchronize_rcu_expedited(void);
 
-void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
-
 /**
  * synchronize_rcu_bh_expedited - Brute-force RCU-bh grace period
  *
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 231abc8..116e870 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -355,6 +355,48 @@ void *__kmalloc(size_t size, gfp_t flags) 
__assume_kmalloc_alignment __malloc;
 void *kmem_cache_alloc(struct kmem_cache *, gfp_t flags) 
__assume_slab_alignment __malloc;
 void kmem_cache_free(struct kmem_cache *, void *);
 
+void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
+
+/* Helper macro for kfree_rcu() to prevent argument-expansion eyestrain. */
+#define __kfree_rcu(head, offset) \
+   do { \
+   unsigned long __of = (unsigned long)offset; \
+   BUILD_BUG_ON(!__is_kfree_rcu_offset(__of)); \
+   kfree_call_rcu(head, (rcu_callback_t)(__of));   \
+   } while (0)
+
+/**
+ * kfree_rcu() - kfree an object after a grace period.
+ * @ptr:   pointer to kfree
+ * @rcu_name:  the name of the struct rcu_head within the type of @ptr.
+ *
+ * Many rcu callbacks functions just call kfree() on the base structure.
+ 

[PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[rao . shoaib]

From: Rao Shoaib 

kfree_call_rcu does not belong in linux/rcupdate.h and should be moved to
slab_common.c

Signed-off-by: Rao Shoaib 
---
 include/linux/rcupdate.h | 43 +++
 include/linux/rcutree.h  |  2 --
 include/linux/slab.h | 42 ++
 kernel/rcu/tree.c| 24 ++--
 mm/slab_common.c | 10 ++
 5 files changed, 65 insertions(+), 56 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 043d047..6338fb6 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -55,6 +55,9 @@ void call_rcu(struct rcu_head *head, rcu_callback_t func);
 #definecall_rcucall_rcu_sched
 #endif /* #else #ifdef CONFIG_PREEMPT_RCU */
 
+/* only for use by kfree_call_rcu() */
+void call_rcu_lazy(struct rcu_head *head, rcu_callback_t func);
+
 void call_rcu_bh(struct rcu_head *head, rcu_callback_t func);
 void call_rcu_sched(struct rcu_head *head, rcu_callback_t func);
 void synchronize_sched(void);
@@ -837,45 +840,6 @@ static inline notrace void 
rcu_read_unlock_sched_notrace(void)
 #define __is_kfree_rcu_offset(offset) ((offset) < 4096)
 
 /*
- * Helper macro for kfree_rcu() to prevent argument-expansion eyestrain.
- */
-#define __kfree_rcu(head, offset) \
-   do { \
-   BUILD_BUG_ON(!__is_kfree_rcu_offset(offset)); \
-   kfree_call_rcu(head, (rcu_callback_t)(unsigned long)(offset)); \
-   } while (0)
-
-/**
- * kfree_rcu() - kfree an object after a grace period.
- * @ptr:   pointer to kfree
- * @rcu_head:  the name of the struct rcu_head within the type of @ptr.
- *
- * Many rcu callbacks functions just call kfree() on the base structure.
- * These functions are trivial, but their size adds up, and furthermore
- * when they are used in a kernel module, that module must invoke the
- * high-latency rcu_barrier() function at module-unload time.
- *
- * The kfree_rcu() function handles this issue.  Rather than encoding a
- * function address in the embedded rcu_head structure, kfree_rcu() instead
- * encodes the offset of the rcu_head structure within the base structure.
- * Because the functions are not allowed in the low-order 4096 bytes of
- * kernel virtual memory, offsets up to 4095 bytes can be accommodated.
- * If the offset is larger than 4095 bytes, a compile-time error will
- * be generated in __kfree_rcu().  If this error is triggered, you can
- * either fall back to use of call_rcu() or rearrange the structure to
- * position the rcu_head structure into the first 4096 bytes.
- *
- * Note that the allowable offset might decrease in the future, for example,
- * to allow something like kmem_cache_free_rcu().
- *
- * The BUILD_BUG_ON check must not involve any function calls, hence the
- * checks are done in macros here.
- */
-#define kfree_rcu(ptr, rcu_head)   \
-   __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
-
-
-/*
  * Place this after a lock-acquisition primitive to guarantee that
  * an UNLOCK+LOCK pair acts as a full barrier.  This guarantee applies
  * if the UNLOCK and LOCK are executed by the same CPU or if the
@@ -887,5 +851,4 @@ static inline notrace void 
rcu_read_unlock_sched_notrace(void)
 #define smp_mb__after_unlock_lock()do { } while (0)
 #endif /* #else #ifdef CONFIG_ARCH_WEAK_RELEASE_ACQUIRE */
 
-
 #endif /* __LINUX_RCUPDATE_H */
diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
index fd996cd..567ef58 100644
--- a/include/linux/rcutree.h
+++ b/include/linux/rcutree.h
@@ -48,8 +48,6 @@ void synchronize_rcu_bh(void);
 void synchronize_sched_expedited(void);
 void synchronize_rcu_expedited(void);
 
-void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
-
 /**
  * synchronize_rcu_bh_expedited - Brute-force RCU-bh grace period
  *
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 231abc8..116e870 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -355,6 +355,48 @@ void *__kmalloc(size_t size, gfp_t flags) 
__assume_kmalloc_alignment __malloc;
 void *kmem_cache_alloc(struct kmem_cache *, gfp_t flags) 
__assume_slab_alignment __malloc;
 void kmem_cache_free(struct kmem_cache *, void *);
 
+void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
+
+/* Helper macro for kfree_rcu() to prevent argument-expansion eyestrain. */
+#define __kfree_rcu(head, offset) \
+   do { \
+   unsigned long __of = (unsigned long)offset; \
+   BUILD_BUG_ON(!__is_kfree_rcu_offset(__of)); \
+   kfree_call_rcu(head, (rcu_callback_t)(__of));   \
+   } while (0)
+
+/**
+ * kfree_rcu() - kfree an object after a grace period.
+ * @ptr:   pointer to kfree
+ * @rcu_name:  the name of the struct rcu_head within the type of @ptr.
+ *
+ * Many rcu callbacks functions just call kfree() on the base structure.
+ * These functions are trivial, but their size 

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Paul E. McKenney]

On Mon, Apr 02, 2018 at 05:45:48PM +0800, kbuild test robot wrote:
> Hi Rao,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on rcu/rcu/next]
> [also build test WARNING on v4.16 next-20180329]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]

I have no idea what is going on with this one, but the other two are due
to not having Tiny RCU support for new-age kfree_rcu().

Thanx, Paul

> url:
> https://github.com/0day-ci/linux/commits/rao-shoaib-oracle-com/Move-kfree_rcu-out-of-rcu-code-and-use-kfree_bulk/20180402-135939
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git 
> rcu/next
> reproduce:
> # apt-get install sparse
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
> 
> 
> sparse warnings: (new ones prefixed by >>)
> 
>include/linux/init.h:134:6: sparse: attribute 'indirect_branch': unknown 
> attribute
>include/linux/init.h:135:5: sparse: attribute 'indirect_branch': unknown 
> attribute
>include/linux/init.h:268:6: sparse: attribute 'indirect_branch': unknown 
> attribute
>include/linux/init.h:269:6: sparse: attribute 'indirect_branch': unknown 
> attribute
>include/linux/printk.h:200:6: sparse: attribute 'indirect_branch': unknown 
> attribute
>arch/x86/include/asm/mem_encrypt.h:32:6: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:34:6: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:37:6: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:38:6: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:40:6: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:42:6: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:43:6: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:45:5: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:46:5: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:49:6: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/qspinlock.h:53:32: sparse: attribute 
> 'indirect_branch': unknown attribute
>include/linux/workqueue.h:646:5: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/workqueue.h:647:5: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/wait_bit.h:41:13: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/numa.h:34:12: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/numa.h:35:13: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/numa.h:62:13: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/vmalloc.h:64:13: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/vmalloc.h:173:8: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/vmalloc.h:174:8: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/fixmap.h:174:6: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/fixmap.h:176:6: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/fixmap.h:178:6: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/fixmap.h:180:6: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/apic.h:254:13: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/apic.h:430:13: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/io_apic.h:184:13: sparse: attribute 
> 'indirect_branch': unknown attribute
>include/linux/smp.h:113:6: sparse: attribute 'indirect_branch': unknown 
> attribute
>include/linux/smp.h:125:13: sparse: attribute 'indirect_branch': unknown 
> attribute
>include/linux/smp.h:126:13: sparse: attribute 'indirect_branch': unknown 
> attribute
>include/linux/percpu.h:110:33: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/percpu.h:112:13: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/percpu.h:114:12: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/percpu.h:118:12: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/percpu.h:126:12: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/fs.h:63:13: sparse: attribute 'indirect_branch': unknown 
> attribute
>include/linux/fs.h:64:13: sparse: attribute 

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Paul E. McKenney]

On Mon, Apr 02, 2018 at 05:45:48PM +0800, kbuild test robot wrote:
> Hi Rao,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on rcu/rcu/next]
> [also build test WARNING on v4.16 next-20180329]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]

I have no idea what is going on with this one, but the other two are due
to not having Tiny RCU support for new-age kfree_rcu().

Thanx, Paul

> url:
> https://github.com/0day-ci/linux/commits/rao-shoaib-oracle-com/Move-kfree_rcu-out-of-rcu-code-and-use-kfree_bulk/20180402-135939
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git 
> rcu/next
> reproduce:
> # apt-get install sparse
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
> 
> 
> sparse warnings: (new ones prefixed by >>)
> 
>include/linux/init.h:134:6: sparse: attribute 'indirect_branch': unknown 
> attribute
>include/linux/init.h:135:5: sparse: attribute 'indirect_branch': unknown 
> attribute
>include/linux/init.h:268:6: sparse: attribute 'indirect_branch': unknown 
> attribute
>include/linux/init.h:269:6: sparse: attribute 'indirect_branch': unknown 
> attribute
>include/linux/printk.h:200:6: sparse: attribute 'indirect_branch': unknown 
> attribute
>arch/x86/include/asm/mem_encrypt.h:32:6: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:34:6: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:37:6: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:38:6: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:40:6: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:42:6: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:43:6: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:45:5: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:46:5: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/mem_encrypt.h:49:6: sparse: attribute 
> 'indirect_branch': unknown attribute
>arch/x86/include/asm/qspinlock.h:53:32: sparse: attribute 
> 'indirect_branch': unknown attribute
>include/linux/workqueue.h:646:5: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/workqueue.h:647:5: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/wait_bit.h:41:13: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/numa.h:34:12: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/numa.h:35:13: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/numa.h:62:13: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/vmalloc.h:64:13: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/vmalloc.h:173:8: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/vmalloc.h:174:8: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/fixmap.h:174:6: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/fixmap.h:176:6: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/fixmap.h:178:6: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/fixmap.h:180:6: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/apic.h:254:13: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/apic.h:430:13: sparse: attribute 'indirect_branch': 
> unknown attribute
>arch/x86/include/asm/io_apic.h:184:13: sparse: attribute 
> 'indirect_branch': unknown attribute
>include/linux/smp.h:113:6: sparse: attribute 'indirect_branch': unknown 
> attribute
>include/linux/smp.h:125:13: sparse: attribute 'indirect_branch': unknown 
> attribute
>include/linux/smp.h:126:13: sparse: attribute 'indirect_branch': unknown 
> attribute
>include/linux/percpu.h:110:33: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/percpu.h:112:13: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/percpu.h:114:12: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/percpu.h:118:12: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/percpu.h:126:12: sparse: attribute 'indirect_branch': 
> unknown attribute
>include/linux/fs.h:63:13: sparse: attribute 'indirect_branch': unknown 
> attribute
>include/linux/fs.h:64:13: sparse: attribute 

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[kbuild test robot]

Hi Rao,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on rcu/rcu/next]
[also build test WARNING on v4.16 next-20180329]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/rao-shoaib-oracle-com/Move-kfree_rcu-out-of-rcu-code-and-use-kfree_bulk/20180402-135939
base:   https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git 
rcu/next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   include/linux/init.h:134:6: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/init.h:135:5: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/init.h:268:6: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/init.h:269:6: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/printk.h:200:6: sparse: attribute 'indirect_branch': unknown 
attribute
   arch/x86/include/asm/mem_encrypt.h:32:6: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:34:6: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:37:6: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:38:6: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:40:6: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:42:6: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:43:6: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:45:5: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:46:5: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:49:6: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/qspinlock.h:53:32: sparse: attribute 'indirect_branch': 
unknown attribute
   include/linux/workqueue.h:646:5: sparse: attribute 'indirect_branch': 
unknown attribute
   include/linux/workqueue.h:647:5: sparse: attribute 'indirect_branch': 
unknown attribute
   include/linux/wait_bit.h:41:13: sparse: attribute 'indirect_branch': unknown 
attribute
   arch/x86/include/asm/numa.h:34:12: sparse: attribute 'indirect_branch': 
unknown attribute
   arch/x86/include/asm/numa.h:35:13: sparse: attribute 'indirect_branch': 
unknown attribute
   arch/x86/include/asm/numa.h:62:13: sparse: attribute 'indirect_branch': 
unknown attribute
   include/linux/vmalloc.h:64:13: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/vmalloc.h:173:8: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/vmalloc.h:174:8: sparse: attribute 'indirect_branch': unknown 
attribute
   arch/x86/include/asm/fixmap.h:174:6: sparse: attribute 'indirect_branch': 
unknown attribute
   arch/x86/include/asm/fixmap.h:176:6: sparse: attribute 'indirect_branch': 
unknown attribute
   arch/x86/include/asm/fixmap.h:178:6: sparse: attribute 'indirect_branch': 
unknown attribute
   arch/x86/include/asm/fixmap.h:180:6: sparse: attribute 'indirect_branch': 
unknown attribute
   arch/x86/include/asm/apic.h:254:13: sparse: attribute 'indirect_branch': 
unknown attribute
   arch/x86/include/asm/apic.h:430:13: sparse: attribute 'indirect_branch': 
unknown attribute
   arch/x86/include/asm/io_apic.h:184:13: sparse: attribute 'indirect_branch': 
unknown attribute
   include/linux/smp.h:113:6: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/smp.h:125:13: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/smp.h:126:13: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/percpu.h:110:33: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/percpu.h:112:13: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/percpu.h:114:12: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/percpu.h:118:12: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/percpu.h:126:12: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/fs.h:63:13: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/fs.h:64:13: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/fs.h:65:13: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/fs.h:66:13: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/memory_hotplug.h:221:13: sparse: attribute 'indirect_branch': 
unknown attribute
   include/linux/mmzone.h:1292:15: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/fs.h:2421:13: sparse: attribute 'indirect_branch': 

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[kbuild test robot]

Hi Rao,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on rcu/rcu/next]
[also build test WARNING on v4.16 next-20180329]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/rao-shoaib-oracle-com/Move-kfree_rcu-out-of-rcu-code-and-use-kfree_bulk/20180402-135939
base:   https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git 
rcu/next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   include/linux/init.h:134:6: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/init.h:135:5: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/init.h:268:6: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/init.h:269:6: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/printk.h:200:6: sparse: attribute 'indirect_branch': unknown 
attribute
   arch/x86/include/asm/mem_encrypt.h:32:6: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:34:6: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:37:6: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:38:6: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:40:6: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:42:6: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:43:6: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:45:5: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:46:5: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/mem_encrypt.h:49:6: sparse: attribute 
'indirect_branch': unknown attribute
   arch/x86/include/asm/qspinlock.h:53:32: sparse: attribute 'indirect_branch': 
unknown attribute
   include/linux/workqueue.h:646:5: sparse: attribute 'indirect_branch': 
unknown attribute
   include/linux/workqueue.h:647:5: sparse: attribute 'indirect_branch': 
unknown attribute
   include/linux/wait_bit.h:41:13: sparse: attribute 'indirect_branch': unknown 
attribute
   arch/x86/include/asm/numa.h:34:12: sparse: attribute 'indirect_branch': 
unknown attribute
   arch/x86/include/asm/numa.h:35:13: sparse: attribute 'indirect_branch': 
unknown attribute
   arch/x86/include/asm/numa.h:62:13: sparse: attribute 'indirect_branch': 
unknown attribute
   include/linux/vmalloc.h:64:13: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/vmalloc.h:173:8: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/vmalloc.h:174:8: sparse: attribute 'indirect_branch': unknown 
attribute
   arch/x86/include/asm/fixmap.h:174:6: sparse: attribute 'indirect_branch': 
unknown attribute
   arch/x86/include/asm/fixmap.h:176:6: sparse: attribute 'indirect_branch': 
unknown attribute
   arch/x86/include/asm/fixmap.h:178:6: sparse: attribute 'indirect_branch': 
unknown attribute
   arch/x86/include/asm/fixmap.h:180:6: sparse: attribute 'indirect_branch': 
unknown attribute
   arch/x86/include/asm/apic.h:254:13: sparse: attribute 'indirect_branch': 
unknown attribute
   arch/x86/include/asm/apic.h:430:13: sparse: attribute 'indirect_branch': 
unknown attribute
   arch/x86/include/asm/io_apic.h:184:13: sparse: attribute 'indirect_branch': 
unknown attribute
   include/linux/smp.h:113:6: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/smp.h:125:13: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/smp.h:126:13: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/percpu.h:110:33: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/percpu.h:112:13: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/percpu.h:114:12: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/percpu.h:118:12: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/percpu.h:126:12: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/fs.h:63:13: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/fs.h:64:13: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/fs.h:65:13: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/fs.h:66:13: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/memory_hotplug.h:221:13: sparse: attribute 'indirect_branch': 
unknown attribute
   include/linux/mmzone.h:1292:15: sparse: attribute 'indirect_branch': unknown 
attribute
   include/linux/fs.h:2421:13: sparse: attribute 'indirect_branch': 

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[kbuild test robot]

Hi Rao,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on rcu/rcu/next]
[also build test ERROR on v4.16 next-20180329]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/rao-shoaib-oracle-com/Move-kfree_rcu-out-of-rcu-code-and-use-kfree_bulk/20180402-135939
base:   https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git 
rcu/next
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

>> mm/slab_common.c:1531:6: error: redefinition of 'kfree_call_rcu'
void kfree_call_rcu(struct rcu_head *head,
 ^~
   In file included from include/linux/rcupdate.h:214:0,
from include/linux/srcu.h:33,
from include/linux/notifier.h:16,
from include/linux/memory_hotplug.h:7,
from include/linux/mmzone.h:775,
from include/linux/gfp.h:6,
from include/linux/slab.h:15,
from mm/slab_common.c:7:
   include/linux/rcutiny.h:87:20: note: previous definition of 'kfree_call_rcu' 
was here
static inline void kfree_call_rcu(struct rcu_head *head,
   ^~

vim +/kfree_call_rcu +1531 mm/slab_common.c

  1527  
  1528  /*
  1529   * Queue Memory to be freed by RCU after a grace period.
  1530   */
> 1531  void kfree_call_rcu(struct rcu_head *head,
  1532  rcu_callback_t func)
  1533  {
  1534  call_rcu_lazy(head, func);
  1535  }
  1536  EXPORT_SYMBOL_GPL(kfree_call_rcu);
  1537  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[kbuild test robot]

Hi Rao,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on rcu/rcu/next]
[also build test ERROR on v4.16 next-20180329]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/rao-shoaib-oracle-com/Move-kfree_rcu-out-of-rcu-code-and-use-kfree_bulk/20180402-135939
base:   https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git 
rcu/next
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

>> mm/slab_common.c:1531:6: error: redefinition of 'kfree_call_rcu'
void kfree_call_rcu(struct rcu_head *head,
 ^~
   In file included from include/linux/rcupdate.h:214:0,
from include/linux/srcu.h:33,
from include/linux/notifier.h:16,
from include/linux/memory_hotplug.h:7,
from include/linux/mmzone.h:775,
from include/linux/gfp.h:6,
from include/linux/slab.h:15,
from mm/slab_common.c:7:
   include/linux/rcutiny.h:87:20: note: previous definition of 'kfree_call_rcu' 
was here
static inline void kfree_call_rcu(struct rcu_head *head,
   ^~

vim +/kfree_call_rcu +1531 mm/slab_common.c

  1527  
  1528  /*
  1529   * Queue Memory to be freed by RCU after a grace period.
  1530   */
> 1531  void kfree_call_rcu(struct rcu_head *head,
  1532  rcu_callback_t func)
  1533  {
  1534  call_rcu_lazy(head, func);
  1535  }
  1536  EXPORT_SYMBOL_GPL(kfree_call_rcu);
  1537  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

[PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[rao . shoaib]

From: Rao Shoaib 

kfree_call_rcu does not belong in linux/rcupdate.h and should be moved to
slab_common.c

Signed-off-by: Rao Shoaib 
---
 include/linux/rcupdate.h | 43 +++
 include/linux/rcutree.h  |  2 --
 include/linux/slab.h | 42 ++
 kernel/rcu/tree.c| 24 ++--
 mm/slab_common.c | 10 ++
 5 files changed, 65 insertions(+), 56 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 043d047..6338fb6 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -55,6 +55,9 @@ void call_rcu(struct rcu_head *head, rcu_callback_t func);
 #definecall_rcucall_rcu_sched
 #endif /* #else #ifdef CONFIG_PREEMPT_RCU */
 
+/* only for use by kfree_call_rcu() */
+void call_rcu_lazy(struct rcu_head *head, rcu_callback_t func);
+
 void call_rcu_bh(struct rcu_head *head, rcu_callback_t func);
 void call_rcu_sched(struct rcu_head *head, rcu_callback_t func);
 void synchronize_sched(void);
@@ -837,45 +840,6 @@ static inline notrace void 
rcu_read_unlock_sched_notrace(void)
 #define __is_kfree_rcu_offset(offset) ((offset) < 4096)
 
 /*
- * Helper macro for kfree_rcu() to prevent argument-expansion eyestrain.
- */
-#define __kfree_rcu(head, offset) \
-   do { \
-   BUILD_BUG_ON(!__is_kfree_rcu_offset(offset)); \
-   kfree_call_rcu(head, (rcu_callback_t)(unsigned long)(offset)); \
-   } while (0)
-
-/**
- * kfree_rcu() - kfree an object after a grace period.
- * @ptr:   pointer to kfree
- * @rcu_head:  the name of the struct rcu_head within the type of @ptr.
- *
- * Many rcu callbacks functions just call kfree() on the base structure.
- * These functions are trivial, but their size adds up, and furthermore
- * when they are used in a kernel module, that module must invoke the
- * high-latency rcu_barrier() function at module-unload time.
- *
- * The kfree_rcu() function handles this issue.  Rather than encoding a
- * function address in the embedded rcu_head structure, kfree_rcu() instead
- * encodes the offset of the rcu_head structure within the base structure.
- * Because the functions are not allowed in the low-order 4096 bytes of
- * kernel virtual memory, offsets up to 4095 bytes can be accommodated.
- * If the offset is larger than 4095 bytes, a compile-time error will
- * be generated in __kfree_rcu().  If this error is triggered, you can
- * either fall back to use of call_rcu() or rearrange the structure to
- * position the rcu_head structure into the first 4096 bytes.
- *
- * Note that the allowable offset might decrease in the future, for example,
- * to allow something like kmem_cache_free_rcu().
- *
- * The BUILD_BUG_ON check must not involve any function calls, hence the
- * checks are done in macros here.
- */
-#define kfree_rcu(ptr, rcu_head)   \
-   __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
-
-
-/*
  * Place this after a lock-acquisition primitive to guarantee that
  * an UNLOCK+LOCK pair acts as a full barrier.  This guarantee applies
  * if the UNLOCK and LOCK are executed by the same CPU or if the
@@ -887,5 +851,4 @@ static inline notrace void 
rcu_read_unlock_sched_notrace(void)
 #define smp_mb__after_unlock_lock()do { } while (0)
 #endif /* #else #ifdef CONFIG_ARCH_WEAK_RELEASE_ACQUIRE */
 
-
 #endif /* __LINUX_RCUPDATE_H */
diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
index fd996cd..567ef58 100644
--- a/include/linux/rcutree.h
+++ b/include/linux/rcutree.h
@@ -48,8 +48,6 @@ void synchronize_rcu_bh(void);
 void synchronize_sched_expedited(void);
 void synchronize_rcu_expedited(void);
 
-void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
-
 /**
  * synchronize_rcu_bh_expedited - Brute-force RCU-bh grace period
  *
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 231abc8..116e870 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -355,6 +355,48 @@ void *__kmalloc(size_t size, gfp_t flags) 
__assume_kmalloc_alignment __malloc;
 void *kmem_cache_alloc(struct kmem_cache *, gfp_t flags) 
__assume_slab_alignment __malloc;
 void kmem_cache_free(struct kmem_cache *, void *);
 
+void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
+
+/* Helper macro for kfree_rcu() to prevent argument-expansion eyestrain. */
+#define __kfree_rcu(head, offset) \
+   do { \
+   unsigned long __of = (unsigned long)offset; \
+   BUILD_BUG_ON(!__is_kfree_rcu_offset(__of)); \
+   kfree_call_rcu(head, (rcu_callback_t)(__of));   \
+   } while (0)
+
+/**
+ * kfree_rcu() - kfree an object after a grace period.
+ * @ptr:   pointer to kfree
+ * @rcu_name:  the name of the struct rcu_head within the type of @ptr.
+ *
+ * Many rcu callbacks functions just call kfree() on the base structure.
+ 

[PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[rao . shoaib]

From: Rao Shoaib 

kfree_call_rcu does not belong in linux/rcupdate.h and should be moved to
slab_common.c

Signed-off-by: Rao Shoaib 
---
 include/linux/rcupdate.h | 43 +++
 include/linux/rcutree.h  |  2 --
 include/linux/slab.h | 42 ++
 kernel/rcu/tree.c| 24 ++--
 mm/slab_common.c | 10 ++
 5 files changed, 65 insertions(+), 56 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 043d047..6338fb6 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -55,6 +55,9 @@ void call_rcu(struct rcu_head *head, rcu_callback_t func);
 #definecall_rcucall_rcu_sched
 #endif /* #else #ifdef CONFIG_PREEMPT_RCU */
 
+/* only for use by kfree_call_rcu() */
+void call_rcu_lazy(struct rcu_head *head, rcu_callback_t func);
+
 void call_rcu_bh(struct rcu_head *head, rcu_callback_t func);
 void call_rcu_sched(struct rcu_head *head, rcu_callback_t func);
 void synchronize_sched(void);
@@ -837,45 +840,6 @@ static inline notrace void 
rcu_read_unlock_sched_notrace(void)
 #define __is_kfree_rcu_offset(offset) ((offset) < 4096)
 
 /*
- * Helper macro for kfree_rcu() to prevent argument-expansion eyestrain.
- */
-#define __kfree_rcu(head, offset) \
-   do { \
-   BUILD_BUG_ON(!__is_kfree_rcu_offset(offset)); \
-   kfree_call_rcu(head, (rcu_callback_t)(unsigned long)(offset)); \
-   } while (0)
-
-/**
- * kfree_rcu() - kfree an object after a grace period.
- * @ptr:   pointer to kfree
- * @rcu_head:  the name of the struct rcu_head within the type of @ptr.
- *
- * Many rcu callbacks functions just call kfree() on the base structure.
- * These functions are trivial, but their size adds up, and furthermore
- * when they are used in a kernel module, that module must invoke the
- * high-latency rcu_barrier() function at module-unload time.
- *
- * The kfree_rcu() function handles this issue.  Rather than encoding a
- * function address in the embedded rcu_head structure, kfree_rcu() instead
- * encodes the offset of the rcu_head structure within the base structure.
- * Because the functions are not allowed in the low-order 4096 bytes of
- * kernel virtual memory, offsets up to 4095 bytes can be accommodated.
- * If the offset is larger than 4095 bytes, a compile-time error will
- * be generated in __kfree_rcu().  If this error is triggered, you can
- * either fall back to use of call_rcu() or rearrange the structure to
- * position the rcu_head structure into the first 4096 bytes.
- *
- * Note that the allowable offset might decrease in the future, for example,
- * to allow something like kmem_cache_free_rcu().
- *
- * The BUILD_BUG_ON check must not involve any function calls, hence the
- * checks are done in macros here.
- */
-#define kfree_rcu(ptr, rcu_head)   \
-   __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
-
-
-/*
  * Place this after a lock-acquisition primitive to guarantee that
  * an UNLOCK+LOCK pair acts as a full barrier.  This guarantee applies
  * if the UNLOCK and LOCK are executed by the same CPU or if the
@@ -887,5 +851,4 @@ static inline notrace void 
rcu_read_unlock_sched_notrace(void)
 #define smp_mb__after_unlock_lock()do { } while (0)
 #endif /* #else #ifdef CONFIG_ARCH_WEAK_RELEASE_ACQUIRE */
 
-
 #endif /* __LINUX_RCUPDATE_H */
diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
index fd996cd..567ef58 100644
--- a/include/linux/rcutree.h
+++ b/include/linux/rcutree.h
@@ -48,8 +48,6 @@ void synchronize_rcu_bh(void);
 void synchronize_sched_expedited(void);
 void synchronize_rcu_expedited(void);
 
-void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
-
 /**
  * synchronize_rcu_bh_expedited - Brute-force RCU-bh grace period
  *
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 231abc8..116e870 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -355,6 +355,48 @@ void *__kmalloc(size_t size, gfp_t flags) 
__assume_kmalloc_alignment __malloc;
 void *kmem_cache_alloc(struct kmem_cache *, gfp_t flags) 
__assume_slab_alignment __malloc;
 void kmem_cache_free(struct kmem_cache *, void *);
 
+void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
+
+/* Helper macro for kfree_rcu() to prevent argument-expansion eyestrain. */
+#define __kfree_rcu(head, offset) \
+   do { \
+   unsigned long __of = (unsigned long)offset; \
+   BUILD_BUG_ON(!__is_kfree_rcu_offset(__of)); \
+   kfree_call_rcu(head, (rcu_callback_t)(__of));   \
+   } while (0)
+
+/**
+ * kfree_rcu() - kfree an object after a grace period.
+ * @ptr:   pointer to kfree
+ * @rcu_name:  the name of the struct rcu_head within the type of @ptr.
+ *
+ * Many rcu callbacks functions just call kfree() on the base structure.
+ * These functions are trivial, but their size 

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Joe Perches]

On Mon, 2018-03-26 at 18:56 -0700, Rao Shoaib wrote:
> Folks,
> 
> Is anyone working on resolving the check patch issue as I am waiting to 
> resubmit my patch. Will it be fine if I submitted the patch with the 
> original macro as the check is in-correct.

Yes.  Of course.  Anytime a person knows better,
checkpatch output should be ignored.

> I do not speak perl but I can do the process work. If folks think Joe's 
> fix is fine I can submit it and perhaps someone can review it ?

I think it's fine too ;)
Submit away...

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Joe Perches]

On Mon, 2018-03-26 at 18:56 -0700, Rao Shoaib wrote:
> Folks,
> 
> Is anyone working on resolving the check patch issue as I am waiting to 
> resubmit my patch. Will it be fine if I submitted the patch with the 
> original macro as the check is in-correct.

Yes.  Of course.  Anytime a person knows better,
checkpatch output should be ignored.

> I do not speak perl but I can do the process work. If folks think Joe's 
> fix is fine I can submit it and perhaps someone can review it ?

I think it's fine too ;)
Submit away...

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Rao Shoaib]

Folks,

Is anyone working on resolving the check patch issue as I am waiting to 
resubmit my patch. Will it be fine if I submitted the patch with the 
original macro as the check is in-correct.


I do not speak perl but I can do the process work. If folks think Joe's 
fix is fine I can submit it and perhaps someone can review it ?


Regards,

Shoaib


On 01/04/2018 10:46 PM, Joe Perches wrote:

On Thu, 2018-01-04 at 16:07 -0800, Matthew Wilcox wrote:

On Thu, Jan 04, 2018 at 03:47:32PM -0800, Paul E. McKenney wrote:

I was under the impression that typeof did not actually evaluate its
argument, but rather only returned its type.  And there are a few macros
with this pattern in mainline.

Or am I confused about what typeof does?

I think checkpatch is confused by the '*' in the typeof argument:

$ git diff |./scripts/checkpatch.pl --strict
CHECK: Macro argument reuse 'ptr' - possible side-effects?
#29: FILE: include/linux/rcupdate.h:896:
+#define kfree_rcu(ptr, rcu_head)\
+   __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))

If one removes the '*', the warning goes away.

I'm no perlista, but Joe, would this regexp modification make sense?

+++ b/scripts/checkpatch.pl
@@ -4957,7 +4957,7 @@ sub process {
 next if ($arg =~ /\.\.\./);
 next if ($arg =~ /^type$/i);
 my $tmp_stmt = $define_stmt;
-   $tmp_stmt =~ 
s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g;
+   $tmp_stmt =~ 
s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\**\(*\s*$arg\s*\)*\b//g;

I supposed ideally it'd be more like

$tmp_stmt =~ 
s/\b(?:typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*(?:\s*\*\s*)*\s*\(*\s*$arg\s*\)*\b//g;

Adding ?: at the start to not capture and
(?:\s*\*\s*)* for any number of * with any
surrounding spacings.

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Rao Shoaib]

Folks,

Is anyone working on resolving the check patch issue as I am waiting to 
resubmit my patch. Will it be fine if I submitted the patch with the 
original macro as the check is in-correct.


I do not speak perl but I can do the process work. If folks think Joe's 
fix is fine I can submit it and perhaps someone can review it ?


Regards,

Shoaib


On 01/04/2018 10:46 PM, Joe Perches wrote:

On Thu, 2018-01-04 at 16:07 -0800, Matthew Wilcox wrote:

On Thu, Jan 04, 2018 at 03:47:32PM -0800, Paul E. McKenney wrote:

I was under the impression that typeof did not actually evaluate its
argument, but rather only returned its type.  And there are a few macros
with this pattern in mainline.

Or am I confused about what typeof does?

I think checkpatch is confused by the '*' in the typeof argument:

$ git diff |./scripts/checkpatch.pl --strict
CHECK: Macro argument reuse 'ptr' - possible side-effects?
#29: FILE: include/linux/rcupdate.h:896:
+#define kfree_rcu(ptr, rcu_head)\
+   __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))

If one removes the '*', the warning goes away.

I'm no perlista, but Joe, would this regexp modification make sense?

+++ b/scripts/checkpatch.pl
@@ -4957,7 +4957,7 @@ sub process {
 next if ($arg =~ /\.\.\./);
 next if ($arg =~ /^type$/i);
 my $tmp_stmt = $define_stmt;
-   $tmp_stmt =~ 
s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g;
+   $tmp_stmt =~ 
s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\**\(*\s*$arg\s*\)*\b//g;

I supposed ideally it'd be more like

$tmp_stmt =~ 
s/\b(?:typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*(?:\s*\*\s*)*\s*\(*\s*$arg\s*\)*\b//g;

Adding ?: at the start to not capture and
(?:\s*\*\s*)* for any number of * with any
surrounding spacings.

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Joe Perches]

On Thu, 2018-01-04 at 16:07 -0800, Matthew Wilcox wrote:
> On Thu, Jan 04, 2018 at 03:47:32PM -0800, Paul E. McKenney wrote:
> > I was under the impression that typeof did not actually evaluate its
> > argument, but rather only returned its type.  And there are a few macros
> > with this pattern in mainline.
> > 
> > Or am I confused about what typeof does?
> 
> I think checkpatch is confused by the '*' in the typeof argument:
> 
> $ git diff |./scripts/checkpatch.pl --strict
> CHECK: Macro argument reuse 'ptr' - possible side-effects?
> #29: FILE: include/linux/rcupdate.h:896:
> +#define kfree_rcu(ptr, rcu_head)\
> + __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
> 
> If one removes the '*', the warning goes away.
> 
> I'm no perlista, but Joe, would this regexp modification make sense?
> 
> +++ b/scripts/checkpatch.pl
> @@ -4957,7 +4957,7 @@ sub process {
> next if ($arg =~ /\.\.\./);
> next if ($arg =~ /^type$/i);
> my $tmp_stmt = $define_stmt;
> -   $tmp_stmt =~ 
> s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g;
> +   $tmp_stmt =~ 
> s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\**\(*\s*$arg\s*\)*\b//g;

I supposed ideally it'd be more like

$tmp_stmt =~ 
s/\b(?:typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*(?:\s*\*\s*)*\s*\(*\s*$arg\s*\)*\b//g;

Adding ?: at the start to not capture and
(?:\s*\*\s*)* for any number of * with any
surrounding spacings.

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Joe Perches]

On Thu, 2018-01-04 at 16:07 -0800, Matthew Wilcox wrote:
> On Thu, Jan 04, 2018 at 03:47:32PM -0800, Paul E. McKenney wrote:
> > I was under the impression that typeof did not actually evaluate its
> > argument, but rather only returned its type.  And there are a few macros
> > with this pattern in mainline.
> > 
> > Or am I confused about what typeof does?
> 
> I think checkpatch is confused by the '*' in the typeof argument:
> 
> $ git diff |./scripts/checkpatch.pl --strict
> CHECK: Macro argument reuse 'ptr' - possible side-effects?
> #29: FILE: include/linux/rcupdate.h:896:
> +#define kfree_rcu(ptr, rcu_head)\
> + __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
> 
> If one removes the '*', the warning goes away.
> 
> I'm no perlista, but Joe, would this regexp modification make sense?
> 
> +++ b/scripts/checkpatch.pl
> @@ -4957,7 +4957,7 @@ sub process {
> next if ($arg =~ /\.\.\./);
> next if ($arg =~ /^type$/i);
> my $tmp_stmt = $define_stmt;
> -   $tmp_stmt =~ 
> s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g;
> +   $tmp_stmt =~ 
> s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\**\(*\s*$arg\s*\)*\b//g;

I supposed ideally it'd be more like

$tmp_stmt =~ 
s/\b(?:typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*(?:\s*\*\s*)*\s*\(*\s*$arg\s*\)*\b//g;

Adding ?: at the start to not capture and
(?:\s*\*\s*)* for any number of * with any
surrounding spacings.

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Rao Shoaib]

On 01/04/2018 04:07 PM, Matthew Wilcox wrote:

On Thu, Jan 04, 2018 at 03:47:32PM -0800, Paul E. McKenney wrote:

I was under the impression that typeof did not actually evaluate its
argument, but rather only returned its type.  And there are a few macros
with this pattern in mainline.

Or am I confused about what typeof does?

I think checkpatch is confused by the '*' in the typeof argument:

Yup.


$ git diff |./scripts/checkpatch.pl --strict
CHECK: Macro argument reuse 'ptr' - possible side-effects?
#29: FILE: include/linux/rcupdate.h:896:
+#define kfree_rcu(ptr, rcu_head)\
+   __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))

If one removes the '*', the warning goes away.

I'm no perlista, but Joe, would this regexp modification make sense?

+++ b/scripts/checkpatch.pl
@@ -4957,7 +4957,7 @@ sub process {
 next if ($arg =~ /\.\.\./);
 next if ($arg =~ /^type$/i);
 my $tmp_stmt = $define_stmt;
-   $tmp_stmt =~ 
s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g;
+   $tmp_stmt =~ 
s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\**\(*\s*$arg\s*\)*\b//g;
 $tmp_stmt =~ s/\#+\s*$arg\b//g;
 $tmp_stmt =~ s/\b$arg\s*\#\#//g;
 my $use_cnt = $tmp_stmt =~ s/\b$arg\b//g;

Thanks a lot for digging into this. I had to try several variations for 
the warning to go away and don't remember the reason for each change. I 
am not perl literate and the regular expression sacred me ;-).


Shoaib

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Rao Shoaib]

On 01/04/2018 04:07 PM, Matthew Wilcox wrote:

On Thu, Jan 04, 2018 at 03:47:32PM -0800, Paul E. McKenney wrote:

I was under the impression that typeof did not actually evaluate its
argument, but rather only returned its type.  And there are a few macros
with this pattern in mainline.

Or am I confused about what typeof does?

I think checkpatch is confused by the '*' in the typeof argument:

Yup.


$ git diff |./scripts/checkpatch.pl --strict
CHECK: Macro argument reuse 'ptr' - possible side-effects?
#29: FILE: include/linux/rcupdate.h:896:
+#define kfree_rcu(ptr, rcu_head)\
+   __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))

If one removes the '*', the warning goes away.

I'm no perlista, but Joe, would this regexp modification make sense?

+++ b/scripts/checkpatch.pl
@@ -4957,7 +4957,7 @@ sub process {
 next if ($arg =~ /\.\.\./);
 next if ($arg =~ /^type$/i);
 my $tmp_stmt = $define_stmt;
-   $tmp_stmt =~ 
s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g;
+   $tmp_stmt =~ 
s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\**\(*\s*$arg\s*\)*\b//g;
 $tmp_stmt =~ s/\#+\s*$arg\b//g;
 $tmp_stmt =~ s/\b$arg\s*\#\#//g;
 my $use_cnt = $tmp_stmt =~ s/\b$arg\b//g;

Thanks a lot for digging into this. I had to try several variations for 
the warning to go away and don't remember the reason for each change. I 
am not perl literate and the regular expression sacred me ;-).


Shoaib

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Matthew Wilcox]

On Thu, Jan 04, 2018 at 03:47:32PM -0800, Paul E. McKenney wrote:
> I was under the impression that typeof did not actually evaluate its
> argument, but rather only returned its type.  And there are a few macros
> with this pattern in mainline.
> 
> Or am I confused about what typeof does?

I think checkpatch is confused by the '*' in the typeof argument:

$ git diff |./scripts/checkpatch.pl --strict
CHECK: Macro argument reuse 'ptr' - possible side-effects?
#29: FILE: include/linux/rcupdate.h:896:
+#define kfree_rcu(ptr, rcu_head)\
+   __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))

If one removes the '*', the warning goes away.

I'm no perlista, but Joe, would this regexp modification make sense?

+++ b/scripts/checkpatch.pl
@@ -4957,7 +4957,7 @@ sub process {
next if ($arg =~ /\.\.\./);
next if ($arg =~ /^type$/i);
my $tmp_stmt = $define_stmt;
-   $tmp_stmt =~ 
s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g;
+   $tmp_stmt =~ 
s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\**\(*\s*$arg\s*\)*\b//g;
$tmp_stmt =~ s/\#+\s*$arg\b//g;
$tmp_stmt =~ s/\b$arg\s*\#\#//g;
my $use_cnt = $tmp_stmt =~ s/\b$arg\b//g;

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Matthew Wilcox]

On Thu, Jan 04, 2018 at 03:47:32PM -0800, Paul E. McKenney wrote:
> I was under the impression that typeof did not actually evaluate its
> argument, but rather only returned its type.  And there are a few macros
> with this pattern in mainline.
> 
> Or am I confused about what typeof does?

I think checkpatch is confused by the '*' in the typeof argument:

$ git diff |./scripts/checkpatch.pl --strict
CHECK: Macro argument reuse 'ptr' - possible side-effects?
#29: FILE: include/linux/rcupdate.h:896:
+#define kfree_rcu(ptr, rcu_head)\
+   __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))

If one removes the '*', the warning goes away.

I'm no perlista, but Joe, would this regexp modification make sense?

+++ b/scripts/checkpatch.pl
@@ -4957,7 +4957,7 @@ sub process {
next if ($arg =~ /\.\.\./);
next if ($arg =~ /^type$/i);
my $tmp_stmt = $define_stmt;
-   $tmp_stmt =~ 
s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g;
+   $tmp_stmt =~ 
s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\**\(*\s*$arg\s*\)*\b//g;
$tmp_stmt =~ s/\#+\s*$arg\b//g;
$tmp_stmt =~ s/\b$arg\s*\#\#//g;
my $use_cnt = $tmp_stmt =~ s/\b$arg\b//g;

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Paul E. McKenney]

On Thu, Jan 04, 2018 at 03:13:07PM -0800, Matthew Wilcox wrote:
> On Thu, Jan 04, 2018 at 02:18:50PM -0800, Rao Shoaib wrote:
> > > > > > +#define kfree_rcu(ptr, rcu_head_name) \
> > > > > > +    do { \
> > > > > > +    typeof(ptr) __ptr = ptr;    \
> > > > > > +    unsigned long __off = offsetof(typeof(*(__ptr)), \
> > > > > > +  rcu_head_name); \
> > > > > > +    struct rcu_head *__rptr = (void *)__ptr + __off; \
> > > > > > +    __kfree_rcu(__rptr, __off); \
> > > > > > +    } while (0)
> > > > > why do you want to open code this?
> > > But why are you changing this macro at all?  If it was to avoid the
> > > double-mention of "ptr", then you haven't done that.
> > I have -- I do not get the error because ptr is being assigned only one. If
> > you have a better way than let me know and I will be happy to make the
> > change.
> 
> But look at the original:
> 
> #define kfree_rcu(ptr, rcu_head)\
> __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
>^^^^^^
> 
> versus your version:
> 
> +#define kfree_rcu(ptr, rcu_head_name) \
> +    do { \
> +    typeof(ptr) __ptr = ptr;    \
> ^^^  ^^^
> +    unsigned long __off = offsetof(typeof(*(__ptr)), \
> +  rcu_head_name); \
> +    struct rcu_head *__rptr = (void *)__ptr + __off; \
> +    __kfree_rcu(__rptr, __off); \
> +    } while (0)
> 
> I don't see the difference.

I was under the impression that typeof did not actually evaluate its
argument, but rather only returned its type.  And there are a few macros
with this pattern in mainline.

Or am I confused about what typeof does?

Thanx, Paul

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Paul E. McKenney]

On Thu, Jan 04, 2018 at 03:13:07PM -0800, Matthew Wilcox wrote:
> On Thu, Jan 04, 2018 at 02:18:50PM -0800, Rao Shoaib wrote:
> > > > > > +#define kfree_rcu(ptr, rcu_head_name) \
> > > > > > +    do { \
> > > > > > +    typeof(ptr) __ptr = ptr;    \
> > > > > > +    unsigned long __off = offsetof(typeof(*(__ptr)), \
> > > > > > +  rcu_head_name); \
> > > > > > +    struct rcu_head *__rptr = (void *)__ptr + __off; \
> > > > > > +    __kfree_rcu(__rptr, __off); \
> > > > > > +    } while (0)
> > > > > why do you want to open code this?
> > > But why are you changing this macro at all?  If it was to avoid the
> > > double-mention of "ptr", then you haven't done that.
> > I have -- I do not get the error because ptr is being assigned only one. If
> > you have a better way than let me know and I will be happy to make the
> > change.
> 
> But look at the original:
> 
> #define kfree_rcu(ptr, rcu_head)\
> __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
>^^^^^^
> 
> versus your version:
> 
> +#define kfree_rcu(ptr, rcu_head_name) \
> +    do { \
> +    typeof(ptr) __ptr = ptr;    \
> ^^^  ^^^
> +    unsigned long __off = offsetof(typeof(*(__ptr)), \
> +  rcu_head_name); \
> +    struct rcu_head *__rptr = (void *)__ptr + __off; \
> +    __kfree_rcu(__rptr, __off); \
> +    } while (0)
> 
> I don't see the difference.

I was under the impression that typeof did not actually evaluate its
argument, but rather only returned its type.  And there are a few macros
with this pattern in mainline.

Or am I confused about what typeof does?

Thanx, Paul

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Matthew Wilcox]

On Thu, Jan 04, 2018 at 02:18:50PM -0800, Rao Shoaib wrote:
> > > > > +#define kfree_rcu(ptr, rcu_head_name) \
> > > > > +    do { \
> > > > > +    typeof(ptr) __ptr = ptr;    \
> > > > > +    unsigned long __off = offsetof(typeof(*(__ptr)), \
> > > > > +  rcu_head_name); \
> > > > > +    struct rcu_head *__rptr = (void *)__ptr + __off; \
> > > > > +    __kfree_rcu(__rptr, __off); \
> > > > > +    } while (0)
> > > > why do you want to open code this?
> > But why are you changing this macro at all?  If it was to avoid the
> > double-mention of "ptr", then you haven't done that.
> I have -- I do not get the error because ptr is being assigned only one. If
> you have a better way than let me know and I will be happy to make the
> change.

But look at the original:

#define kfree_rcu(ptr, rcu_head)\
__kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
   ^^^^^^

versus your version:

+#define kfree_rcu(ptr, rcu_head_name) \
+    do { \
+    typeof(ptr) __ptr = ptr;    \
^^^  ^^^
+    unsigned long __off = offsetof(typeof(*(__ptr)), \
+  rcu_head_name); \
+    struct rcu_head *__rptr = (void *)__ptr + __off; \
+    __kfree_rcu(__rptr, __off); \
+    } while (0)

I don't see the difference.

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Matthew Wilcox]

On Thu, Jan 04, 2018 at 02:18:50PM -0800, Rao Shoaib wrote:
> > > > > +#define kfree_rcu(ptr, rcu_head_name) \
> > > > > +    do { \
> > > > > +    typeof(ptr) __ptr = ptr;    \
> > > > > +    unsigned long __off = offsetof(typeof(*(__ptr)), \
> > > > > +  rcu_head_name); \
> > > > > +    struct rcu_head *__rptr = (void *)__ptr + __off; \
> > > > > +    __kfree_rcu(__rptr, __off); \
> > > > > +    } while (0)
> > > > why do you want to open code this?
> > But why are you changing this macro at all?  If it was to avoid the
> > double-mention of "ptr", then you haven't done that.
> I have -- I do not get the error because ptr is being assigned only one. If
> you have a better way than let me know and I will be happy to make the
> change.

But look at the original:

#define kfree_rcu(ptr, rcu_head)\
__kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
   ^^^^^^

versus your version:

+#define kfree_rcu(ptr, rcu_head_name) \
+    do { \
+    typeof(ptr) __ptr = ptr;    \
^^^  ^^^
+    unsigned long __off = offsetof(typeof(*(__ptr)), \
+  rcu_head_name); \
+    struct rcu_head *__rptr = (void *)__ptr + __off; \
+    __kfree_rcu(__rptr, __off); \
+    } while (0)

I don't see the difference.

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Rao Shoaib]

On 01/04/2018 01:46 PM, Matthew Wilcox wrote:

On Thu, Jan 04, 2018 at 01:27:49PM -0800, Rao Shoaib wrote:

On 01/04/2018 12:35 PM, Rao Shoaib wrote:

As far as your previous comments are concerned, only the following one
has not been addressed. Can you please elaborate as I do not understand
the comment. The code was expanded because the new macro expansion check
fails. Based on Matthew Wilcox's comment I have reverted rcu_head_name
back to rcu_head.

It turns out I did not remember the real reason for the change. With the
macro rewritten, using rcu_head as a macro argument does not work because it
conflicts with the name of the type 'struct rcu_head' used in the macro. I
have renamed the macro argument to rcu_name.

Shoaib

+#define kfree_rcu(ptr, rcu_head_name) \
+    do { \
+    typeof(ptr) __ptr = ptr;    \
+    unsigned long __off = offsetof(typeof(*(__ptr)), \
+  rcu_head_name); \
+    struct rcu_head *__rptr = (void *)__ptr + __off; \
+    __kfree_rcu(__rptr, __off); \
+    } while (0)

why do you want to open code this?

But why are you changing this macro at all?  If it was to avoid the
double-mention of "ptr", then you haven't done that.
I have -- I do not get the error because ptr is being assigned only one. 
If you have a better way than let me know and I will be happy to make 
the change.


Shoaib.

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Rao Shoaib]

On 01/04/2018 01:46 PM, Matthew Wilcox wrote:

On Thu, Jan 04, 2018 at 01:27:49PM -0800, Rao Shoaib wrote:

On 01/04/2018 12:35 PM, Rao Shoaib wrote:

As far as your previous comments are concerned, only the following one
has not been addressed. Can you please elaborate as I do not understand
the comment. The code was expanded because the new macro expansion check
fails. Based on Matthew Wilcox's comment I have reverted rcu_head_name
back to rcu_head.

It turns out I did not remember the real reason for the change. With the
macro rewritten, using rcu_head as a macro argument does not work because it
conflicts with the name of the type 'struct rcu_head' used in the macro. I
have renamed the macro argument to rcu_name.

Shoaib

+#define kfree_rcu(ptr, rcu_head_name) \
+    do { \
+    typeof(ptr) __ptr = ptr;    \
+    unsigned long __off = offsetof(typeof(*(__ptr)), \
+  rcu_head_name); \
+    struct rcu_head *__rptr = (void *)__ptr + __off; \
+    __kfree_rcu(__rptr, __off); \
+    } while (0)

why do you want to open code this?

But why are you changing this macro at all?  If it was to avoid the
double-mention of "ptr", then you haven't done that.
I have -- I do not get the error because ptr is being assigned only one. 
If you have a better way than let me know and I will be happy to make 
the change.


Shoaib.

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Matthew Wilcox]

On Thu, Jan 04, 2018 at 01:27:49PM -0800, Rao Shoaib wrote:
> On 01/04/2018 12:35 PM, Rao Shoaib wrote:
> > As far as your previous comments are concerned, only the following one
> > has not been addressed. Can you please elaborate as I do not understand
> > the comment. The code was expanded because the new macro expansion check
> > fails. Based on Matthew Wilcox's comment I have reverted rcu_head_name
> > back to rcu_head.
> It turns out I did not remember the real reason for the change. With the
> macro rewritten, using rcu_head as a macro argument does not work because it
> conflicts with the name of the type 'struct rcu_head' used in the macro. I
> have renamed the macro argument to rcu_name.
> 
> Shoaib
> > 
> > > +#define kfree_rcu(ptr, rcu_head_name) \
> > > +    do { \
> > > +    typeof(ptr) __ptr = ptr;    \
> > > +    unsigned long __off = offsetof(typeof(*(__ptr)), \
> > > +  rcu_head_name); \
> > > +    struct rcu_head *__rptr = (void *)__ptr + __off; \
> > > +    __kfree_rcu(__rptr, __off); \
> > > +    } while (0)
> > 
> > why do you want to open code this?

But why are you changing this macro at all?  If it was to avoid the
double-mention of "ptr", then you haven't done that.

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Matthew Wilcox]

On Thu, Jan 04, 2018 at 01:27:49PM -0800, Rao Shoaib wrote:
> On 01/04/2018 12:35 PM, Rao Shoaib wrote:
> > As far as your previous comments are concerned, only the following one
> > has not been addressed. Can you please elaborate as I do not understand
> > the comment. The code was expanded because the new macro expansion check
> > fails. Based on Matthew Wilcox's comment I have reverted rcu_head_name
> > back to rcu_head.
> It turns out I did not remember the real reason for the change. With the
> macro rewritten, using rcu_head as a macro argument does not work because it
> conflicts with the name of the type 'struct rcu_head' used in the macro. I
> have renamed the macro argument to rcu_name.
> 
> Shoaib
> > 
> > > +#define kfree_rcu(ptr, rcu_head_name) \
> > > +    do { \
> > > +    typeof(ptr) __ptr = ptr;    \
> > > +    unsigned long __off = offsetof(typeof(*(__ptr)), \
> > > +  rcu_head_name); \
> > > +    struct rcu_head *__rptr = (void *)__ptr + __off; \
> > > +    __kfree_rcu(__rptr, __off); \
> > > +    } while (0)
> > 
> > why do you want to open code this?

But why are you changing this macro at all?  If it was to avoid the
double-mention of "ptr", then you haven't done that.

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Rao Shoaib]

On 01/04/2018 12:35 PM, Rao Shoaib wrote:

Hi Boqun,

Thanks a lot for all your guidance and for catching the cut and paster 
error. Please see inline.



On 01/03/2018 05:38 PM, Boqun Feng wrote:


But you introduced a bug here, you should use rcu_state_p instead of
_sched_state as the third parameter for __call_rcu().

Please re-read:

https://marc.info/?l=linux-mm=151390529209639

, and there are other comments, which you still haven't resolved in this
version. You may want to write a better commit log to explain the
reasons of each modifcation and fix bugs or typos in your previous
version. That's how review process works ;-)

Regards,
Boqun


This is definitely a serious error. Thanks for catching this.

As far as your previous comments are concerned, only the following one 
has not been addressed. Can you please elaborate as I do not 
understand the comment. The code was expanded because the new macro 
expansion check fails. Based on Matthew Wilcox's comment I have 
reverted rcu_head_name back to rcu_head.
It turns out I did not remember the real reason for the change. With the 
macro rewritten, using rcu_head as a macro argument does not work 
because it conflicts with the name of the type 'struct rcu_head' used in 
the macro. I have renamed the macro argument to rcu_name.


Shoaib



+#define kfree_rcu(ptr, rcu_head_name) \
+    do { \
+    typeof(ptr) __ptr = ptr;    \
+    unsigned long __off = offsetof(typeof(*(__ptr)), \
+  rcu_head_name); \
+    struct rcu_head *__rptr = (void *)__ptr + __off; \
+    __kfree_rcu(__rptr, __off); \
+    } while (0)


why do you want to open code this?

Does the following text for the commit log looks better.

kfree_rcu() should use the new kfree_bulk() interface for freeing rcu 
structures


The newly implemented kfree_bulk() interfaces are more efficient, 
using the interfaces for freeing rcu structures has shown performance 
improvements in synthetic benchmarks that allocate and free rcu 
structures at a high rate.


Shoaib

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majord...@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Rao Shoaib]

On 01/04/2018 12:35 PM, Rao Shoaib wrote:

Hi Boqun,

Thanks a lot for all your guidance and for catching the cut and paster 
error. Please see inline.



On 01/03/2018 05:38 PM, Boqun Feng wrote:


But you introduced a bug here, you should use rcu_state_p instead of
_sched_state as the third parameter for __call_rcu().

Please re-read:

https://marc.info/?l=linux-mm=151390529209639

, and there are other comments, which you still haven't resolved in this
version. You may want to write a better commit log to explain the
reasons of each modifcation and fix bugs or typos in your previous
version. That's how review process works ;-)

Regards,
Boqun


This is definitely a serious error. Thanks for catching this.

As far as your previous comments are concerned, only the following one 
has not been addressed. Can you please elaborate as I do not 
understand the comment. The code was expanded because the new macro 
expansion check fails. Based on Matthew Wilcox's comment I have 
reverted rcu_head_name back to rcu_head.
It turns out I did not remember the real reason for the change. With the 
macro rewritten, using rcu_head as a macro argument does not work 
because it conflicts with the name of the type 'struct rcu_head' used in 
the macro. I have renamed the macro argument to rcu_name.


Shoaib



+#define kfree_rcu(ptr, rcu_head_name) \
+    do { \
+    typeof(ptr) __ptr = ptr;    \
+    unsigned long __off = offsetof(typeof(*(__ptr)), \
+  rcu_head_name); \
+    struct rcu_head *__rptr = (void *)__ptr + __off; \
+    __kfree_rcu(__rptr, __off); \
+    } while (0)


why do you want to open code this?

Does the following text for the commit log looks better.

kfree_rcu() should use the new kfree_bulk() interface for freeing rcu 
structures


The newly implemented kfree_bulk() interfaces are more efficient, 
using the interfaces for freeing rcu structures has shown performance 
improvements in synthetic benchmarks that allocate and free rcu 
structures at a high rate.


Shoaib

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majord...@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Rao Shoaib]

Hi Boqun,

Thanks a lot for all your guidance and for catching the cut and paster 
error. Please see inline.



On 01/03/2018 05:38 PM, Boqun Feng wrote:


But you introduced a bug here, you should use rcu_state_p instead of
_sched_state as the third parameter for __call_rcu().

Please re-read:

https://marc.info/?l=linux-mm=151390529209639

, and there are other comments, which you still haven't resolved in this
version. You may want to write a better commit log to explain the
reasons of each modifcation and fix bugs or typos in your previous
version. That's how review process works ;-)

Regards,
Boqun


This is definitely a serious error. Thanks for catching this.

As far as your previous comments are concerned, only the following one 
has not been addressed. Can you please elaborate as I do not understand 
the comment. The code was expanded because the new macro expansion check 
fails. Based on Matthew Wilcox's comment I have reverted rcu_head_name 
back to rcu_head.



+#define kfree_rcu(ptr, rcu_head_name)  \
+   do { \
+   typeof(ptr) __ptr = ptr;\
+   unsigned long __off = offsetof(typeof(*(__ptr)), \
+ rcu_head_name); \
+   struct rcu_head *__rptr = (void *)__ptr + __off; \
+   __kfree_rcu(__rptr, __off); \
+   } while (0)


why do you want to open code this?

Does the following text for the commit log looks better.

kfree_rcu() should use the new kfree_bulk() interface for freeing rcu 
structures


The newly implemented kfree_bulk() interfaces are more efficient, using 
the interfaces for freeing rcu structures has shown performance 
improvements in synthetic benchmarks that allocate and free rcu 
structures at a high rate.


Shoaib

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Rao Shoaib]

Hi Boqun,

Thanks a lot for all your guidance and for catching the cut and paster 
error. Please see inline.



On 01/03/2018 05:38 PM, Boqun Feng wrote:


But you introduced a bug here, you should use rcu_state_p instead of
_sched_state as the third parameter for __call_rcu().

Please re-read:

https://marc.info/?l=linux-mm=151390529209639

, and there are other comments, which you still haven't resolved in this
version. You may want to write a better commit log to explain the
reasons of each modifcation and fix bugs or typos in your previous
version. That's how review process works ;-)

Regards,
Boqun


This is definitely a serious error. Thanks for catching this.

As far as your previous comments are concerned, only the following one 
has not been addressed. Can you please elaborate as I do not understand 
the comment. The code was expanded because the new macro expansion check 
fails. Based on Matthew Wilcox's comment I have reverted rcu_head_name 
back to rcu_head.



+#define kfree_rcu(ptr, rcu_head_name)  \
+   do { \
+   typeof(ptr) __ptr = ptr;\
+   unsigned long __off = offsetof(typeof(*(__ptr)), \
+ rcu_head_name); \
+   struct rcu_head *__rptr = (void *)__ptr + __off; \
+   __kfree_rcu(__rptr, __off); \
+   } while (0)


why do you want to open code this?

Does the following text for the commit log looks better.

kfree_rcu() should use the new kfree_bulk() interface for freeing rcu 
structures


The newly implemented kfree_bulk() interfaces are more efficient, using 
the interfaces for freeing rcu structures has shown performance 
improvements in synthetic benchmarks that allocate and free rcu 
structures at a high rate.


Shoaib

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Boqun Feng]

Hi Shoaib,

Good to see you set out a patchset ;-)

On Tue, Jan 02, 2018 at 02:49:25PM -0800, Rao Shoaib wrote:
> 
> 
> On 01/02/2018 02:23 PM, Matthew Wilcox wrote:
> > On Tue, Jan 02, 2018 at 12:11:37PM -0800, rao.sho...@oracle.com wrote:
> > > -#define kfree_rcu(ptr, rcu_head) \
> > > - __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
> > > +#define kfree_rcu(ptr, rcu_head_name)\
> > > + do { \
> > > + typeof(ptr) __ptr = ptr;\
> > > + unsigned long __off = offsetof(typeof(*(__ptr)), \
> > > +   rcu_head_name); \
> > > + struct rcu_head *__rptr = (void *)__ptr + __off; \
> > > + __kfree_rcu(__rptr, __off); \
> > > + } while (0)
> > I feel like you're trying to help people understand the code better,
> > but using longer names can really work against that.  Reverting to
> > calling the parameter 'rcu_head' lets you not split the line:
> I think it is a matter of preference, what is the issue with line splitting
> ?
> Coming from a background other than Linux I find it very annoying that Linux
> allows variables names that are meaning less. Linux does not even enforce
> adding a prefix for structure members, so trying to find out where a member
> is used or set is impossible using cscope.
> I can not change the Linux requirements so I will go ahead and make the
> change in the next rev.
> 
> > 
> > +#define kfree_rcu(ptr, rcu_head)   \
> > +   do { \
> > +   typeof(ptr) __ptr = ptr;\
> > +   unsigned long __off = offsetof(typeof(*(__ptr)), rcu_head); \
> > +   struct rcu_head *__rptr = (void *)__ptr + __off; \
> > +   __kfree_rcu(__rptr, __off); \
> > +   } while (0)
> > 
> > Also, I don't understand why you're bothering to create __ptr here.
> > I understand the desire to not mention the same argument more than once,
> > but you have 'ptr' twice anyway.
> > 
> > And it's good practice to enclose macro arguments in parentheses in case
> > the user has done something really tricksy like pass in "p + 1".
> > 
> > In summary, I don't see anything fundamentally better in your rewrite
> > of kfree_rcu().  The previous version is more succinct, and to my
> > mind, easier to understand.
> I did not want to make thins change but it is required due to the new tests
> added for macro expansion where the same name as in the macro can not be
> used twice. It takes care of the 'p + 1' hazard that you refer to above.
> > 
> > > +void call_rcu_lazy(struct rcu_head *head, rcu_callback_t func)
> > > +{
> > > + __call_rcu(head, func, _sched_state, -1, 1);
> > > +}
> > > -void kfree_call_rcu(struct rcu_head *head,
> > > - rcu_callback_t func)
> > > -{
> > > - __call_rcu(head, func, rcu_state_p, -1, 1);
> > > -}
> > You've silently changed this.  Why?  It might well be the right change,
> > but it at least merits mentioning in the changelog.
> This was to address a comment about me not changing the tiny implementation
> to be same as the tree implementation.
> 

But you introduced a bug here, you should use rcu_state_p instead of
_sched_state as the third parameter for __call_rcu().

Please re-read:

https://marc.info/?l=linux-mm=151390529209639

, and there are other comments, which you still haven't resolved in this
version. You may want to write a better commit log to explain the
reasons of each modifcation and fix bugs or typos in your previous
version. That's how review process works ;-)

Regards,
Boqun

> Shoaib
> > 
> > --
> > To unsubscribe, send a message with 'unsubscribe linux-mm' in
> > the body to majord...@kvack.org.  For more info on Linux MM,
> > see: http://www.linux-mm.org/ .
> > Don't email: mailto:"d...@kvack.org;> em...@kvack.org 
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 


signature.asc
Description: PGP signature

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Boqun Feng]

Hi Shoaib,

Good to see you set out a patchset ;-)

On Tue, Jan 02, 2018 at 02:49:25PM -0800, Rao Shoaib wrote:
> 
> 
> On 01/02/2018 02:23 PM, Matthew Wilcox wrote:
> > On Tue, Jan 02, 2018 at 12:11:37PM -0800, rao.sho...@oracle.com wrote:
> > > -#define kfree_rcu(ptr, rcu_head) \
> > > - __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
> > > +#define kfree_rcu(ptr, rcu_head_name)\
> > > + do { \
> > > + typeof(ptr) __ptr = ptr;\
> > > + unsigned long __off = offsetof(typeof(*(__ptr)), \
> > > +   rcu_head_name); \
> > > + struct rcu_head *__rptr = (void *)__ptr + __off; \
> > > + __kfree_rcu(__rptr, __off); \
> > > + } while (0)
> > I feel like you're trying to help people understand the code better,
> > but using longer names can really work against that.  Reverting to
> > calling the parameter 'rcu_head' lets you not split the line:
> I think it is a matter of preference, what is the issue with line splitting
> ?
> Coming from a background other than Linux I find it very annoying that Linux
> allows variables names that are meaning less. Linux does not even enforce
> adding a prefix for structure members, so trying to find out where a member
> is used or set is impossible using cscope.
> I can not change the Linux requirements so I will go ahead and make the
> change in the next rev.
> 
> > 
> > +#define kfree_rcu(ptr, rcu_head)   \
> > +   do { \
> > +   typeof(ptr) __ptr = ptr;\
> > +   unsigned long __off = offsetof(typeof(*(__ptr)), rcu_head); \
> > +   struct rcu_head *__rptr = (void *)__ptr + __off; \
> > +   __kfree_rcu(__rptr, __off); \
> > +   } while (0)
> > 
> > Also, I don't understand why you're bothering to create __ptr here.
> > I understand the desire to not mention the same argument more than once,
> > but you have 'ptr' twice anyway.
> > 
> > And it's good practice to enclose macro arguments in parentheses in case
> > the user has done something really tricksy like pass in "p + 1".
> > 
> > In summary, I don't see anything fundamentally better in your rewrite
> > of kfree_rcu().  The previous version is more succinct, and to my
> > mind, easier to understand.
> I did not want to make thins change but it is required due to the new tests
> added for macro expansion where the same name as in the macro can not be
> used twice. It takes care of the 'p + 1' hazard that you refer to above.
> > 
> > > +void call_rcu_lazy(struct rcu_head *head, rcu_callback_t func)
> > > +{
> > > + __call_rcu(head, func, _sched_state, -1, 1);
> > > +}
> > > -void kfree_call_rcu(struct rcu_head *head,
> > > - rcu_callback_t func)
> > > -{
> > > - __call_rcu(head, func, rcu_state_p, -1, 1);
> > > -}
> > You've silently changed this.  Why?  It might well be the right change,
> > but it at least merits mentioning in the changelog.
> This was to address a comment about me not changing the tiny implementation
> to be same as the tree implementation.
> 

But you introduced a bug here, you should use rcu_state_p instead of
_sched_state as the third parameter for __call_rcu().

Please re-read:

https://marc.info/?l=linux-mm=151390529209639

, and there are other comments, which you still haven't resolved in this
version. You may want to write a better commit log to explain the
reasons of each modifcation and fix bugs or typos in your previous
version. That's how review process works ;-)

Regards,
Boqun

> Shoaib
> > 
> > --
> > To unsubscribe, send a message with 'unsubscribe linux-mm' in
> > the body to majord...@kvack.org.  For more info on Linux MM,
> > see: http://www.linux-mm.org/ .
> > Don't email: mailto:"d...@kvack.org;> em...@kvack.org 
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 


signature.asc
Description: PGP signature

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Rao Shoaib]

On 01/02/2018 02:23 PM, Matthew Wilcox wrote:

On Tue, Jan 02, 2018 at 12:11:37PM -0800, rao.sho...@oracle.com wrote:

-#define kfree_rcu(ptr, rcu_head)   \
-   __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
+#define kfree_rcu(ptr, rcu_head_name)  \
+   do { \
+   typeof(ptr) __ptr = ptr;\
+   unsigned long __off = offsetof(typeof(*(__ptr)), \
+ rcu_head_name); \
+   struct rcu_head *__rptr = (void *)__ptr + __off; \
+   __kfree_rcu(__rptr, __off); \
+   } while (0)

I feel like you're trying to help people understand the code better,
but using longer names can really work against that.  Reverting to
calling the parameter 'rcu_head' lets you not split the line:
I think it is a matter of preference, what is the issue with line 
splitting ?
Coming from a background other than Linux I find it very annoying that 
Linux allows variables names that are meaning less. Linux does not even 
enforce adding a prefix for structure members, so trying to find out 
where a member is used or set is impossible using cscope.
I can not change the Linux requirements so I will go ahead and make the 
change in the next rev.




+#define kfree_rcu(ptr, rcu_head)   \
+   do { \
+   typeof(ptr) __ptr = ptr;\
+   unsigned long __off = offsetof(typeof(*(__ptr)), rcu_head); \
+   struct rcu_head *__rptr = (void *)__ptr + __off; \
+   __kfree_rcu(__rptr, __off); \
+   } while (0)

Also, I don't understand why you're bothering to create __ptr here.
I understand the desire to not mention the same argument more than once,
but you have 'ptr' twice anyway.

And it's good practice to enclose macro arguments in parentheses in case
the user has done something really tricksy like pass in "p + 1".

In summary, I don't see anything fundamentally better in your rewrite
of kfree_rcu().  The previous version is more succinct, and to my
mind, easier to understand.
I did not want to make thins change but it is required due to the new 
tests added for macro expansion where the same name as in the macro can 
not be used twice. It takes care of the 'p + 1' hazard that you refer to 
above.



+void call_rcu_lazy(struct rcu_head *head, rcu_callback_t func)
+{
+   __call_rcu(head, func, _sched_state, -1, 1);
+}
-void kfree_call_rcu(struct rcu_head *head,
-   rcu_callback_t func)
-{
-   __call_rcu(head, func, rcu_state_p, -1, 1);
-}

You've silently changed this.  Why?  It might well be the right change,
but it at least merits mentioning in the changelog.
This was to address a comment about me not changing the tiny 
implementation to be same as the tree implementation.


Shoaib


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majord...@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Rao Shoaib]

On 01/02/2018 02:23 PM, Matthew Wilcox wrote:

On Tue, Jan 02, 2018 at 12:11:37PM -0800, rao.sho...@oracle.com wrote:

-#define kfree_rcu(ptr, rcu_head)   \
-   __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
+#define kfree_rcu(ptr, rcu_head_name)  \
+   do { \
+   typeof(ptr) __ptr = ptr;\
+   unsigned long __off = offsetof(typeof(*(__ptr)), \
+ rcu_head_name); \
+   struct rcu_head *__rptr = (void *)__ptr + __off; \
+   __kfree_rcu(__rptr, __off); \
+   } while (0)

I feel like you're trying to help people understand the code better,
but using longer names can really work against that.  Reverting to
calling the parameter 'rcu_head' lets you not split the line:
I think it is a matter of preference, what is the issue with line 
splitting ?
Coming from a background other than Linux I find it very annoying that 
Linux allows variables names that are meaning less. Linux does not even 
enforce adding a prefix for structure members, so trying to find out 
where a member is used or set is impossible using cscope.
I can not change the Linux requirements so I will go ahead and make the 
change in the next rev.




+#define kfree_rcu(ptr, rcu_head)   \
+   do { \
+   typeof(ptr) __ptr = ptr;\
+   unsigned long __off = offsetof(typeof(*(__ptr)), rcu_head); \
+   struct rcu_head *__rptr = (void *)__ptr + __off; \
+   __kfree_rcu(__rptr, __off); \
+   } while (0)

Also, I don't understand why you're bothering to create __ptr here.
I understand the desire to not mention the same argument more than once,
but you have 'ptr' twice anyway.

And it's good practice to enclose macro arguments in parentheses in case
the user has done something really tricksy like pass in "p + 1".

In summary, I don't see anything fundamentally better in your rewrite
of kfree_rcu().  The previous version is more succinct, and to my
mind, easier to understand.
I did not want to make thins change but it is required due to the new 
tests added for macro expansion where the same name as in the macro can 
not be used twice. It takes care of the 'p + 1' hazard that you refer to 
above.



+void call_rcu_lazy(struct rcu_head *head, rcu_callback_t func)
+{
+   __call_rcu(head, func, _sched_state, -1, 1);
+}
-void kfree_call_rcu(struct rcu_head *head,
-   rcu_callback_t func)
-{
-   __call_rcu(head, func, rcu_state_p, -1, 1);
-}

You've silently changed this.  Why?  It might well be the right change,
but it at least merits mentioning in the changelog.
This was to address a comment about me not changing the tiny 
implementation to be same as the tree implementation.


Shoaib


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majord...@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Matthew Wilcox]

On Tue, Jan 02, 2018 at 12:11:37PM -0800, rao.sho...@oracle.com wrote:
> -#define kfree_rcu(ptr, rcu_head) \
> - __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))

> +#define kfree_rcu(ptr, rcu_head_name)\
> + do { \
> + typeof(ptr) __ptr = ptr;\
> + unsigned long __off = offsetof(typeof(*(__ptr)), \
> +   rcu_head_name); \
> + struct rcu_head *__rptr = (void *)__ptr + __off; \
> + __kfree_rcu(__rptr, __off); \
> + } while (0)

I feel like you're trying to help people understand the code better,
but using longer names can really work against that.  Reverting to
calling the parameter 'rcu_head' lets you not split the line:

+#define kfree_rcu(ptr, rcu_head)   \
+   do { \
+   typeof(ptr) __ptr = ptr;\
+   unsigned long __off = offsetof(typeof(*(__ptr)), rcu_head); \
+   struct rcu_head *__rptr = (void *)__ptr + __off; \
+   __kfree_rcu(__rptr, __off); \
+   } while (0)

Also, I don't understand why you're bothering to create __ptr here.
I understand the desire to not mention the same argument more than once,
but you have 'ptr' twice anyway.

And it's good practice to enclose macro arguments in parentheses in case
the user has done something really tricksy like pass in "p + 1".

In summary, I don't see anything fundamentally better in your rewrite
of kfree_rcu().  The previous version is more succinct, and to my
mind, easier to understand.

> +void call_rcu_lazy(struct rcu_head *head, rcu_callback_t func)
> +{
> + __call_rcu(head, func, _sched_state, -1, 1);
> +}

> -void kfree_call_rcu(struct rcu_head *head,
> - rcu_callback_t func)
> -{
> - __call_rcu(head, func, rcu_state_p, -1, 1);
> -}

You've silently changed this.  Why?  It might well be the right change,
but it at least merits mentioning in the changelog.

Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[Matthew Wilcox]

On Tue, Jan 02, 2018 at 12:11:37PM -0800, rao.sho...@oracle.com wrote:
> -#define kfree_rcu(ptr, rcu_head) \
> - __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))

> +#define kfree_rcu(ptr, rcu_head_name)\
> + do { \
> + typeof(ptr) __ptr = ptr;\
> + unsigned long __off = offsetof(typeof(*(__ptr)), \
> +   rcu_head_name); \
> + struct rcu_head *__rptr = (void *)__ptr + __off; \
> + __kfree_rcu(__rptr, __off); \
> + } while (0)

I feel like you're trying to help people understand the code better,
but using longer names can really work against that.  Reverting to
calling the parameter 'rcu_head' lets you not split the line:

+#define kfree_rcu(ptr, rcu_head)   \
+   do { \
+   typeof(ptr) __ptr = ptr;\
+   unsigned long __off = offsetof(typeof(*(__ptr)), rcu_head); \
+   struct rcu_head *__rptr = (void *)__ptr + __off; \
+   __kfree_rcu(__rptr, __off); \
+   } while (0)

Also, I don't understand why you're bothering to create __ptr here.
I understand the desire to not mention the same argument more than once,
but you have 'ptr' twice anyway.

And it's good practice to enclose macro arguments in parentheses in case
the user has done something really tricksy like pass in "p + 1".

In summary, I don't see anything fundamentally better in your rewrite
of kfree_rcu().  The previous version is more succinct, and to my
mind, easier to understand.

> +void call_rcu_lazy(struct rcu_head *head, rcu_callback_t func)
> +{
> + __call_rcu(head, func, _sched_state, -1, 1);
> +}

> -void kfree_call_rcu(struct rcu_head *head,
> - rcu_callback_t func)
> -{
> - __call_rcu(head, func, rcu_state_p, -1, 1);
> -}

You've silently changed this.  Why?  It might well be the right change,
but it at least merits mentioning in the changelog.

[PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[rao . shoaib]

From: Rao Shoaib 

Signed-off-by: Rao Shoaib 
---
 include/linux/rcupdate.h | 43 +++
 include/linux/rcutree.h  |  2 --
 include/linux/slab.h | 44 
 kernel/rcu/tree.c| 24 ++--
 mm/slab_common.c | 10 ++
 5 files changed, 67 insertions(+), 56 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index a6ddc42..23ed728 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -55,6 +55,9 @@ void call_rcu(struct rcu_head *head, rcu_callback_t func);
 #definecall_rcucall_rcu_sched
 #endif /* #else #ifdef CONFIG_PREEMPT_RCU */
 
+/* only for use by kfree_call_rcu() */
+void call_rcu_lazy(struct rcu_head *head, rcu_callback_t func);
+
 void call_rcu_bh(struct rcu_head *head, rcu_callback_t func);
 void call_rcu_sched(struct rcu_head *head, rcu_callback_t func);
 void synchronize_sched(void);
@@ -838,45 +841,6 @@ static inline notrace void 
rcu_read_unlock_sched_notrace(void)
 #define __is_kfree_rcu_offset(offset) ((offset) < 4096)
 
 /*
- * Helper macro for kfree_rcu() to prevent argument-expansion eyestrain.
- */
-#define __kfree_rcu(head, offset) \
-   do { \
-   BUILD_BUG_ON(!__is_kfree_rcu_offset(offset)); \
-   kfree_call_rcu(head, (rcu_callback_t)(unsigned long)(offset)); \
-   } while (0)
-
-/**
- * kfree_rcu() - kfree an object after a grace period.
- * @ptr:   pointer to kfree
- * @rcu_head:  the name of the struct rcu_head within the type of @ptr.
- *
- * Many rcu callbacks functions just call kfree() on the base structure.
- * These functions are trivial, but their size adds up, and furthermore
- * when they are used in a kernel module, that module must invoke the
- * high-latency rcu_barrier() function at module-unload time.
- *
- * The kfree_rcu() function handles this issue.  Rather than encoding a
- * function address in the embedded rcu_head structure, kfree_rcu() instead
- * encodes the offset of the rcu_head structure within the base structure.
- * Because the functions are not allowed in the low-order 4096 bytes of
- * kernel virtual memory, offsets up to 4095 bytes can be accommodated.
- * If the offset is larger than 4095 bytes, a compile-time error will
- * be generated in __kfree_rcu().  If this error is triggered, you can
- * either fall back to use of call_rcu() or rearrange the structure to
- * position the rcu_head structure into the first 4096 bytes.
- *
- * Note that the allowable offset might decrease in the future, for example,
- * to allow something like kmem_cache_free_rcu().
- *
- * The BUILD_BUG_ON check must not involve any function calls, hence the
- * checks are done in macros here.
- */
-#define kfree_rcu(ptr, rcu_head)   \
-   __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
-
-
-/*
  * Place this after a lock-acquisition primitive to guarantee that
  * an UNLOCK+LOCK pair acts as a full barrier.  This guarantee applies
  * if the UNLOCK and LOCK are executed by the same CPU or if the
@@ -888,5 +852,4 @@ static inline notrace void 
rcu_read_unlock_sched_notrace(void)
 #define smp_mb__after_unlock_lock()do { } while (0)
 #endif /* #else #ifdef CONFIG_ARCH_WEAK_RELEASE_ACQUIRE */
 
-
 #endif /* __LINUX_RCUPDATE_H */
diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
index 37d6fd3..7746b19 100644
--- a/include/linux/rcutree.h
+++ b/include/linux/rcutree.h
@@ -48,8 +48,6 @@ void synchronize_rcu_bh(void);
 void synchronize_sched_expedited(void);
 void synchronize_rcu_expedited(void);
 
-void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
-
 /**
  * synchronize_rcu_bh_expedited - Brute-force RCU-bh grace period
  *
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 50697a1..a71f6a78 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -342,6 +342,50 @@ void *__kmalloc(size_t size, gfp_t flags) 
__assume_kmalloc_alignment __malloc;
 void *kmem_cache_alloc(struct kmem_cache *, gfp_t flags) 
__assume_slab_alignment __malloc;
 void kmem_cache_free(struct kmem_cache *, void *);
 
+void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
+
+/* Helper macro for kfree_rcu() to prevent argument-expansion eyestrain. */
+#define __kfree_rcu(head, offset) \
+   do { \
+   unsigned long __o = (unsigned long)offset; \
+   BUILD_BUG_ON(!__is_kfree_rcu_offset(__o)); \
+   kfree_call_rcu(head, (rcu_callback_t)(__o)); \
+   } while (0)
+
+/**
+ * kfree_rcu() - kfree an object after a grace period.
+ * @ptr:   pointer to kfree
+ * @rcu_head:  the name of the struct rcu_head within the type of @ptr.
+ *
+ * Many rcu callbacks functions just call kfree() on the base structure.
+ * These functions are trivial, but their size adds up, and furthermore
+ * when they are used 

[PATCH 1/2] Move kfree_call_rcu() to slab_common.c

[rao . shoaib]

From: Rao Shoaib 

Signed-off-by: Rao Shoaib 
---
 include/linux/rcupdate.h | 43 +++
 include/linux/rcutree.h  |  2 --
 include/linux/slab.h | 44 
 kernel/rcu/tree.c| 24 ++--
 mm/slab_common.c | 10 ++
 5 files changed, 67 insertions(+), 56 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index a6ddc42..23ed728 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -55,6 +55,9 @@ void call_rcu(struct rcu_head *head, rcu_callback_t func);
 #definecall_rcucall_rcu_sched
 #endif /* #else #ifdef CONFIG_PREEMPT_RCU */
 
+/* only for use by kfree_call_rcu() */
+void call_rcu_lazy(struct rcu_head *head, rcu_callback_t func);
+
 void call_rcu_bh(struct rcu_head *head, rcu_callback_t func);
 void call_rcu_sched(struct rcu_head *head, rcu_callback_t func);
 void synchronize_sched(void);
@@ -838,45 +841,6 @@ static inline notrace void 
rcu_read_unlock_sched_notrace(void)
 #define __is_kfree_rcu_offset(offset) ((offset) < 4096)
 
 /*
- * Helper macro for kfree_rcu() to prevent argument-expansion eyestrain.
- */
-#define __kfree_rcu(head, offset) \
-   do { \
-   BUILD_BUG_ON(!__is_kfree_rcu_offset(offset)); \
-   kfree_call_rcu(head, (rcu_callback_t)(unsigned long)(offset)); \
-   } while (0)
-
-/**
- * kfree_rcu() - kfree an object after a grace period.
- * @ptr:   pointer to kfree
- * @rcu_head:  the name of the struct rcu_head within the type of @ptr.
- *
- * Many rcu callbacks functions just call kfree() on the base structure.
- * These functions are trivial, but their size adds up, and furthermore
- * when they are used in a kernel module, that module must invoke the
- * high-latency rcu_barrier() function at module-unload time.
- *
- * The kfree_rcu() function handles this issue.  Rather than encoding a
- * function address in the embedded rcu_head structure, kfree_rcu() instead
- * encodes the offset of the rcu_head structure within the base structure.
- * Because the functions are not allowed in the low-order 4096 bytes of
- * kernel virtual memory, offsets up to 4095 bytes can be accommodated.
- * If the offset is larger than 4095 bytes, a compile-time error will
- * be generated in __kfree_rcu().  If this error is triggered, you can
- * either fall back to use of call_rcu() or rearrange the structure to
- * position the rcu_head structure into the first 4096 bytes.
- *
- * Note that the allowable offset might decrease in the future, for example,
- * to allow something like kmem_cache_free_rcu().
- *
- * The BUILD_BUG_ON check must not involve any function calls, hence the
- * checks are done in macros here.
- */
-#define kfree_rcu(ptr, rcu_head)   \
-   __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
-
-
-/*
  * Place this after a lock-acquisition primitive to guarantee that
  * an UNLOCK+LOCK pair acts as a full barrier.  This guarantee applies
  * if the UNLOCK and LOCK are executed by the same CPU or if the
@@ -888,5 +852,4 @@ static inline notrace void 
rcu_read_unlock_sched_notrace(void)
 #define smp_mb__after_unlock_lock()do { } while (0)
 #endif /* #else #ifdef CONFIG_ARCH_WEAK_RELEASE_ACQUIRE */
 
-
 #endif /* __LINUX_RCUPDATE_H */
diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
index 37d6fd3..7746b19 100644
--- a/include/linux/rcutree.h
+++ b/include/linux/rcutree.h
@@ -48,8 +48,6 @@ void synchronize_rcu_bh(void);
 void synchronize_sched_expedited(void);
 void synchronize_rcu_expedited(void);
 
-void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
-
 /**
  * synchronize_rcu_bh_expedited - Brute-force RCU-bh grace period
  *
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 50697a1..a71f6a78 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -342,6 +342,50 @@ void *__kmalloc(size_t size, gfp_t flags) 
__assume_kmalloc_alignment __malloc;
 void *kmem_cache_alloc(struct kmem_cache *, gfp_t flags) 
__assume_slab_alignment __malloc;
 void kmem_cache_free(struct kmem_cache *, void *);
 
+void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
+
+/* Helper macro for kfree_rcu() to prevent argument-expansion eyestrain. */
+#define __kfree_rcu(head, offset) \
+   do { \
+   unsigned long __o = (unsigned long)offset; \
+   BUILD_BUG_ON(!__is_kfree_rcu_offset(__o)); \
+   kfree_call_rcu(head, (rcu_callback_t)(__o)); \
+   } while (0)
+
+/**
+ * kfree_rcu() - kfree an object after a grace period.
+ * @ptr:   pointer to kfree
+ * @rcu_head:  the name of the struct rcu_head within the type of @ptr.
+ *
+ * Many rcu callbacks functions just call kfree() on the base structure.
+ * These functions are trivial, but their size adds up, and furthermore
+ * when they are used in a kernel module, that module must invoke the