Re: [RCU PATCH 06/14] net: sched: fw use RCU

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Wed, Mar 12, 2014 at 10:01:13AM -0700, Eric Dumazet wrote:
> On Wed, 2014-03-12 at 09:41 -0700, John Fastabend wrote:
> 
> > Rearranging the rcu_head placement above results in this error:
> > 
> > net/sched/cls_fw.c: In function ‘fw_destroy’:
> > net/sched/cls_fw.c:169:279: error: call to ‘__compiletime_assert_169’ 
> > declared with attribute error: BUILD_BUG_ON failed: 
> > !__is_kfree_rcu_offset(__builtin_offsetof(typeof(*(head)),rcu))
> > 
> > 
> > However it seems best to have the rcu_head at the end of the struct
> > so I'll just convert the kfree_rcu() to call_rcu().
> 
> 
> I missed HTSIZE was so big.
> 
> Its unfortunate because it makes struct fw_head slightly larger than one
> page.

Hmmm...  You know, the offset that kfree uses is a byte offset.  I see
no reason why any of the rcu_head structures should be misaligned.  If
HTSIZE is too big by only a factor of four or smaller, would the following
(untested) patch to RCU be appropriate?

							Thanx, Paul

------------------------------------------------------------------------

rcu: Increase kfree_rcu() offset range

The kfree_rcu() function operates by placing an offset into the rcu_head
structure in place of the function pointer that is normally there.
This offset cannot exceed 4095, on the theory that no Linux kernel will
ever have executable code in page zero.  However, the rcu_head structure
contains a pair of function pointers, and should never be misaligned.
This commit therefore divides the offset by the size of a pointer when
being placed into the rcu_head structure, and multiplies it by this
same constant when pulling it out.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 3cea28c64ebe..2dc4ecf923d0 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -1023,8 +1023,13 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
  * 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))
+#define kfree_rcu(ptr, rcu_head) \
+do { \
+	unsigned long ___offset = offsetof(typeof(*(ptr)), rcu_head); \
+	\
+	BUILD_BUG_ON(___offset & (sizeof(void *) - 1)); \
+	__kfree_rcu(&((ptr)->rcu_head), ___offset / sizeof(void *)); \
+} while (0)
 
 #if defined(CONFIG_TINY_RCU) || defined(CONFIG_RCU_NOCB_CPU_ALL)
 static inline int rcu_needs_cpu(int cpu, unsigned long *delta_jiffies)
diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
index bfda2726ca45..93dbd98ad6da 100644
--- a/kernel/rcu/rcu.h
+++ b/kernel/rcu/rcu.h
@@ -106,7 +106,7 @@ static inline bool __rcu_reclaim(const char *rn, struct rcu_head *head)
 	rcu_lock_acquire(&rcu_callback_map);
 	if (__is_kfree_rcu_offset(offset)) {
 		RCU_TRACE(trace_rcu_invoke_kfree_callback(rn, head, offset));
-		kfree((void *)head - offset);
+		kfree((void *)head - offset * sizeof(void *));
 		rcu_lock_release(&rcu_callback_map);
 		return 1;
 	} else {