[PATCH] RCU: Reduce size of rcu_head 1 of 2

[PATCH] RCU: Reduce size of rcu_head 1 of 2

[Dipankar Sarma]

I have merged Rusty's original patch for reducing the size of
rcu_heads by splitting the two main changes into two patches.
This first patch just changes the rcu_heads to use a singly linked
list for internal per-CPU lists. I also added the tail pointer
so that the rcu_heads can be queued in the same order (and
hence invoked) as it was done earlier. That way we are not
messing with callback semantics at all.

Thanks
Dipankar




This reduces the RCU head size by using a singly linked to maintain
them. The ordering of the callbacks is still maintained as before
by using a tail pointer for the next list. Rusty wrote the originial
patch which I merged and changed to make it use a tail pointer so
that ordering of callbacks is not changed.


 include/linux/rcupdate.h |   21 ++++++++++-----------
 kernel/rcupdate.c        |   40 ++++++++++++++++++++--------------------
 2 files changed, 30 insertions(+), 31 deletions(-)

diff -puN include/linux/rcupdate.h~rcu-singly-link include/linux/rcupdate.h
--- linux-2.6.0-test2-rcu/include/linux/rcupdate.h~rcu-singly-link	2003-07-31 16:12:45.000000000 +0530
+++ linux-2.6.0-test2-rcu-dipankar/include/linux/rcupdate.h	2003-07-31 16:12:45.000000000 +0530
@@ -36,28 +36,26 @@
 #ifdef __KERNEL__
 
 #include <linux/cache.h>
-#include <linux/list.h>
 #include <linux/spinlock.h>
 #include <linux/threads.h>
 #include <linux/percpu.h>
 
 /**
  * struct rcu_head - callback structure for use with RCU
- * @list: list_head to queue the update requests
+ * @next: next update requests in a list
  * @func: actual update function to call after the grace period.
  * @arg: argument to be passed to the actual update function.
  */
 struct rcu_head {
-	struct list_head list;
+	struct rcu_head *next;
 	void (*func)(void *obj);
 	void *arg;
 };
 
-#define RCU_HEAD_INIT(head) \
-		{ list: LIST_HEAD_INIT(head.list), func: NULL, arg: NULL }
+#define RCU_HEAD_INIT(head) { .next = NULL, .func = NULL, .arg = NULL }
 #define RCU_HEAD(head) struct rcu_head head = RCU_HEAD_INIT(head)
 #define INIT_RCU_HEAD(ptr) do { \
-       INIT_LIST_HEAD(&(ptr)->list); (ptr)->func = NULL; (ptr)->arg = NULL; \
+       (ptr)->next = NULL; (ptr)->func = NULL; (ptr)->arg = NULL; \
 } while (0)
 
 
@@ -93,8 +91,9 @@ struct rcu_data {
         long            last_qsctr;	 /* value of qsctr at beginning */
                                          /* of rcu grace period */
         long  	       	batch;           /* Batch # for current RCU batch */
-        struct list_head  nxtlist;
-        struct list_head  curlist;
+        struct rcu_head *nxtlist;
+	struct rcu_head **nxttail;
+        struct rcu_head *curlist;
 };
 
 DECLARE_PER_CPU(struct rcu_data, rcu_data);
@@ -104,16 +103,16 @@ extern struct rcu_ctrlblk rcu_ctrlblk;
 #define RCU_last_qsctr(cpu) 	(per_cpu(rcu_data, (cpu)).last_qsctr)
 #define RCU_batch(cpu) 		(per_cpu(rcu_data, (cpu)).batch)
 #define RCU_nxtlist(cpu) 	(per_cpu(rcu_data, (cpu)).nxtlist)
+#define RCU_nxttail(cpu) 	(per_cpu(rcu_data, (cpu)).nxttail)
 #define RCU_curlist(cpu) 	(per_cpu(rcu_data, (cpu)).curlist)
 
 #define RCU_QSCTR_INVALID	0
 
 static inline int rcu_pending(int cpu) 
 {
-	if ((!list_empty(&RCU_curlist(cpu)) &&
+	if ((RCU_curlist(cpu) &&
 	     rcu_batch_before(RCU_batch(cpu), rcu_ctrlblk.curbatch)) ||
-	    (list_empty(&RCU_curlist(cpu)) &&
-			 !list_empty(&RCU_nxtlist(cpu))) ||
+	    (!RCU_curlist(cpu) && RCU_nxtlist(cpu)) ||
 	    test_bit(cpu, &rcu_ctrlblk.rcu_cpu_mask))
 		return 1;
 	else
diff -puN kernel/rcupdate.c~rcu-singly-link kernel/rcupdate.c
--- linux-2.6.0-test2-rcu/kernel/rcupdate.c~rcu-singly-link	2003-07-31 16:12:45.000000000 +0530
+++ linux-2.6.0-test2-rcu-dipankar/kernel/rcupdate.c	2003-07-31 16:12:45.000000000 +0530
@@ -73,9 +73,11 @@ void call_rcu(struct rcu_head *head, voi
 
 	head->func = func;
 	head->arg = arg;
+	head->next = NULL;
 	local_irq_save(flags);
 	cpu = smp_processor_id();
-	list_add_tail(&head->list, &RCU_nxtlist(cpu));
+	*RCU_nxttail(cpu) = head;
+	RCU_nxttail(cpu) = &head->next;
 	local_irq_restore(flags);
 }
 
@@ -83,16 +85,14 @@ void call_rcu(struct rcu_head *head, voi
  * Invoke the completed RCU callbacks. They are expected to be in
  * a per-cpu list.
  */
-static void rcu_do_batch(struct list_head *list)
+static void rcu_do_batch(struct rcu_head *list)
 {
-	struct list_head *entry;
-	struct rcu_head *head;
+	struct rcu_head *next;
 
-	while (!list_empty(list)) {
-		entry = list->next;
-		list_del(entry);
-		head = list_entry(entry, struct rcu_head, list);
-		head->func(head->arg);
+	while (list) {
+		next = list->next;
+		list->func(list->arg);
+		list = next;
 	}
 }
 
@@ -160,18 +160,19 @@ out_unlock:
 static void rcu_process_callbacks(unsigned long unused)
 {
 	int cpu = smp_processor_id();
-	LIST_HEAD(list);
+	struct rcu_head *rcu_list = NULL;
 
-	if (!list_empty(&RCU_curlist(cpu)) &&
+	if (RCU_curlist(cpu) &&
 	    rcu_batch_after(rcu_ctrlblk.curbatch, RCU_batch(cpu))) {
-		list_splice(&RCU_curlist(cpu), &list);
-		INIT_LIST_HEAD(&RCU_curlist(cpu));
+		rcu_list = RCU_curlist(cpu);
+		RCU_curlist(cpu) = NULL;
 	}
 
 	local_irq_disable();
-	if (!list_empty(&RCU_nxtlist(cpu)) && list_empty(&RCU_curlist(cpu))) {
-		list_splice(&RCU_nxtlist(cpu), &RCU_curlist(cpu));
-		INIT_LIST_HEAD(&RCU_nxtlist(cpu));
+	if (RCU_nxtlist(cpu) && !RCU_curlist(cpu)) {
+		RCU_curlist(cpu) = RCU_nxtlist(cpu);
+		RCU_nxtlist(cpu) = NULL;
+		RCU_nxttail(cpu) = &RCU_nxtlist(cpu);
 		local_irq_enable();
 
 		/*
@@ -185,8 +186,8 @@ static void rcu_process_callbacks(unsign
 		local_irq_enable();
 	}
 	rcu_check_quiescent_state();
-	if (!list_empty(&list))
-		rcu_do_batch(&list);
+	if (rcu_list)
+		rcu_do_batch(rcu_list);
 }
 
 void rcu_check_callbacks(int cpu, int user)
@@ -202,8 +203,7 @@ static void __devinit rcu_online_cpu(int
 {
 	memset(&per_cpu(rcu_data, cpu), 0, sizeof(struct rcu_data));
 	tasklet_init(&RCU_tasklet(cpu), rcu_process_callbacks, 0UL);
-	INIT_LIST_HEAD(&RCU_nxtlist(cpu));
-	INIT_LIST_HEAD(&RCU_curlist(cpu));
+	RCU_nxttail(cpu) = &RCU_nxtlist(cpu);
 }
 
 static int __devinit rcu_cpu_notify(struct notifier_block *self, 

_

Re: [PATCH] RCU: Reduce size of rcu_head 2 of 2

[Dipankar Sarma]

This is the second part of Rusty's idea - using container_of to
get to the RCU protected structure given the rcu_head in the
callback. It assumes that the rcu_head is embedded in that structure.
This along with the singly-linked rcu_head patch reduces the
size of struct rcu_head by 50%. With rcu_head being used in dentries
and dst_entries, this is useful savings. It does require changes
in call_rcu() API. I have tested the 2 patches with some sanity
tests (dcache/route cache) and RCU works fine.

Thanks
Dipankar



This is a merge of Rusty's patch to save space in rcu_heads
along with the changes required to all the call_rcu() users.
It changes the call_rcu() API and avoids passing an
argument to the callback function. Instead, it is assumed that
the user has embedded the rcu head into a structure that is
useful in the callback and the rcu_head pointer is passed to
the callback. The callback can use container_of() to get the
pointer to its structure and work with it. Together with
the rcu-singly-link patch, it reduces the rcu_head size
by 50%. Considering that we use these in things like
struct dentry and struct dst_entry, this is good savings
in space.


 fs/dcache.c              |    6 +++---
 include/linux/rcupdate.h |   10 ++++------
 include/net/dst.h        |    6 ++++++
 ipc/util.c               |   25 ++++++++++++++++++++-----
 kernel/rcupdate.c        |   25 +++++++++++++++----------
 net/bridge/br_if.c       |    7 ++++---
 net/decnet/dn_route.c    |    4 ++--
 net/ipv4/route.c         |    4 ++--
 8 files changed, 56 insertions(+), 31 deletions(-)

diff -puN include/linux/rcupdate.h~rcu-no-arg include/linux/rcupdate.h
--- linux-2.6.0-test2-rcu/include/linux/rcupdate.h~rcu-no-arg	2003-07-31 16:13:19.000000000 +0530
+++ linux-2.6.0-test2-rcu-dipankar/include/linux/rcupdate.h	2003-07-31 16:13:19.000000000 +0530
@@ -44,18 +44,16 @@
  * struct rcu_head - callback structure for use with RCU
  * @next: next update requests in a list
  * @func: actual update function to call after the grace period.
- * @arg: argument to be passed to the actual update function.
  */
 struct rcu_head {
 	struct rcu_head *next;
-	void (*func)(void *obj);
-	void *arg;
+	void (*func)(struct rcu_head *head);
 };
 
-#define RCU_HEAD_INIT(head) { .next = NULL, .func = NULL, .arg = NULL }
+#define RCU_HEAD_INIT(head) { .next = NULL, .func = NULL }
 #define RCU_HEAD(head) struct rcu_head head = RCU_HEAD_INIT(head)
 #define INIT_RCU_HEAD(ptr) do { \
-       (ptr)->next = NULL; (ptr)->func = NULL; (ptr)->arg = NULL; \
+       (ptr)->next = NULL; (ptr)->func = NULL; \
 } while (0)
 
 
@@ -127,7 +125,7 @@ extern void rcu_check_callbacks(int cpu,
 
 /* Exported interfaces */
 extern void FASTCALL(call_rcu(struct rcu_head *head, 
-                          void (*func)(void *arg), void *arg));
+				void (*func)(struct rcu_head *head)));
 extern void synchronize_kernel(void);
 
 #endif /* __KERNEL__ */
diff -puN kernel/rcupdate.c~rcu-no-arg kernel/rcupdate.c
--- linux-2.6.0-test2-rcu/kernel/rcupdate.c~rcu-no-arg	2003-07-31 16:13:19.000000000 +0530
+++ linux-2.6.0-test2-rcu-dipankar/kernel/rcupdate.c	2003-07-31 16:13:19.000000000 +0530
@@ -59,20 +59,18 @@ static DEFINE_PER_CPU(struct tasklet_str
  * call_rcu - Queue an RCU update request.
  * @head: structure to be used for queueing the RCU updates.
  * @func: actual update function to be invoked after the grace period
- * @arg: argument to be passed to the update function
  *
  * The update function will be invoked as soon as all CPUs have performed 
  * a context switch or been seen in the idle loop or in a user process. 
  * The read-side of critical section that use call_rcu() for updation must 
  * be protected by rcu_read_lock()/rcu_read_unlock().
  */
-void call_rcu(struct rcu_head *head, void (*func)(void *arg), void *arg)
+void call_rcu(struct rcu_head *head, void (*func)(struct rcu_head *rcu))
 {
 	int cpu;
 	unsigned long flags;
 
 	head->func = func;
-	head->arg = arg;
 	head->next = NULL;
 	local_irq_save(flags);
 	cpu = smp_processor_id();
@@ -91,7 +89,7 @@ static void rcu_do_batch(struct rcu_head
 
 	while (list) {
 		next = list->next;
-		list->func(list->arg);
+		list->func(list);
 		list = next;
 	}
 }
@@ -239,11 +237,18 @@ void __init rcu_init(void)
 	register_cpu_notifier(&rcu_nb);
 }
 
+struct rcu_synchronize {
+	struct rcu_head head;
+	struct completion completion;
+};
 
 /* Because of FASTCALL declaration of complete, we use this wrapper */
-static void wakeme_after_rcu(void *completion)
+static void wakeme_after_rcu(struct rcu_head  *head)
 {
-	complete(completion);
+	struct rcu_synchronize *rcu;
+
+	rcu = container_of(head, struct rcu_synchronize, head);
+	complete(&rcu->completion);
 }
 
 /**
@@ -252,14 +257,14 @@ static void wakeme_after_rcu(void *compl
  */
 void synchronize_kernel(void)
 {
-	struct rcu_head rcu;
-	DECLARE_COMPLETION(completion);
+	struct rcu_synchronize rcu;
 
+	init_completion(&rcu.completion);
 	/* Will wake me after RCU finished */
-	call_rcu(&rcu, wakeme_after_rcu, &completion);
+	call_rcu(&rcu.head, wakeme_after_rcu);
 
 	/* Wait for it */
-	wait_for_completion(&completion);
+	wait_for_completion(&rcu.completion);
 }
 
 
diff -puN fs/dcache.c~rcu-no-arg fs/dcache.c
--- linux-2.6.0-test2-rcu/fs/dcache.c~rcu-no-arg	2003-07-31 16:13:19.000000000 +0530
+++ linux-2.6.0-test2-rcu-dipankar/fs/dcache.c	2003-07-31 16:13:19.000000000 +0530
@@ -58,9 +58,9 @@ struct dentry_stat_t dentry_stat = {
 	.age_limit = 45,
 };
 
-static void d_callback(void *arg)
+static void d_callback(struct rcu_head *head)
 {
-	struct dentry * dentry = (struct dentry *)arg;
+	struct dentry * dentry = container_of(head, struct dentry, d_rcu);
 
 	if (dname_external(dentry)) {
 		kfree(dentry->d_qstr);
@@ -76,7 +76,7 @@ static void d_free(struct dentry *dentry
 {
 	if (dentry->d_op && dentry->d_op->d_release)
 		dentry->d_op->d_release(dentry);
- 	call_rcu(&dentry->d_rcu, d_callback, dentry);
+ 	call_rcu(&dentry->d_rcu, d_callback);
 }
 
 /*
diff -puN ipc/util.c~rcu-no-arg ipc/util.c
--- linux-2.6.0-test2-rcu/ipc/util.c~rcu-no-arg	2003-07-31 16:13:19.000000000 +0530
+++ linux-2.6.0-test2-rcu-dipankar/ipc/util.c	2003-07-31 16:13:19.000000000 +0530
@@ -332,25 +332,40 @@ void* ipc_rcu_alloc(int size)
  * Since RCU callback function is called in bh,
  * we need to defer the vfree to schedule_work
  */
-static void ipc_schedule_free(void* arg)
+static void ipc_schedule_free(struct rcu_head *head)
 {
-	struct ipc_rcu_vmalloc *free = arg;
+	struct ipc_rcu_vmalloc *free = 
+		container_of(head, struct ipc_rcu_vmalloc, rcu);
 
 	INIT_WORK(&free->work, vfree, free);
 	schedule_work(&free->work);
 }
 
+/**
+ *	ipc_immediate_free	- free ipc + rcu space
+ *
+ *	Free from the RCU callback context
+ * 
+ */
+static void ipc_immediate_free(struct rcu_head *head)
+{
+	struct ipc_rcu_kmalloc *free = 
+		container_of(head, struct ipc_rcu_kmalloc, rcu);
+	kfree(free);
+}
+
+
+
 void ipc_rcu_free(void* ptr, int size)
 {
 	if (rcu_use_vmalloc(size)) {
 		struct ipc_rcu_vmalloc *free;
 		free = ptr - sizeof(*free);
-		call_rcu(&free->rcu, ipc_schedule_free, free);
+		call_rcu(&free->rcu, ipc_schedule_free);
 	} else {
 		struct ipc_rcu_kmalloc *free;
 		free = ptr - sizeof(*free);
-		/* kfree takes a "const void *" so gcc warns.  So we cast. */
-		call_rcu(&free->rcu, (void (*)(void *))kfree, free);
+		call_rcu(&free->rcu, ipc_immediate_free);
 	}
 
 }
diff -puN net/bridge/br_if.c~rcu-no-arg net/bridge/br_if.c
--- linux-2.6.0-test2-rcu/net/bridge/br_if.c~rcu-no-arg	2003-07-31 16:13:19.000000000 +0530
+++ linux-2.6.0-test2-rcu-dipankar/net/bridge/br_if.c	2003-07-31 16:16:33.000000000 +0530
@@ -38,9 +38,10 @@ static int br_initial_port_cost(struct n
 	return 100;
 }
 
-static void destroy_nbp(void *arg)
+static void destroy_nbp(struct rcu_head *head)
 {
-	struct net_bridge_port *p = arg;
+	struct net_bridge_port *p = 
+			container_of(head, struct net_bridge_port, rcu);
 
 	p->dev->br_port = NULL;
 
@@ -69,7 +70,7 @@ static void del_nbp(struct net_bridge_po
 	del_timer(&p->forward_delay_timer);
 	del_timer(&p->hold_timer);
 	
-	call_rcu(&p->rcu, destroy_nbp, p);
+	call_rcu(&p->rcu, destroy_nbp);
 }
 
 static void del_br(struct net_bridge *br)
diff -puN net/decnet/dn_route.c~rcu-no-arg net/decnet/dn_route.c
--- linux-2.6.0-test2-rcu/net/decnet/dn_route.c~rcu-no-arg	2003-07-31 16:13:19.000000000 +0530
+++ linux-2.6.0-test2-rcu-dipankar/net/decnet/dn_route.c	2003-07-31 16:13:19.000000000 +0530
@@ -145,14 +145,14 @@ static __inline__ unsigned dn_hash(unsig
 
 static inline void dnrt_free(struct dn_route *rt)
 {
-	call_rcu(&rt->u.dst.rcu_head, (void (*)(void *))dst_free, &rt->u.dst);
+	call_rcu(&rt->u.dst.rcu_head, dst_rcu_free);
 }
 
 static inline void dnrt_drop(struct dn_route *rt)
 {
 	if (rt)
 		dst_release(&rt->u.dst);
-	call_rcu(&rt->u.dst.rcu_head, (void (*)(void *))dst_free, &rt->u.dst);
+	call_rcu(&rt->u.dst.rcu_head, dst_rcu_free);
 }
 
 static void dn_dst_check_expire(unsigned long dummy)
diff -puN include/net/dst.h~rcu-no-arg include/net/dst.h
--- linux-2.6.0-test2-rcu/include/net/dst.h~rcu-no-arg	2003-07-31 16:13:19.000000000 +0530
+++ linux-2.6.0-test2-rcu-dipankar/include/net/dst.h	2003-07-31 16:13:19.000000000 +0530
@@ -182,6 +182,12 @@ static inline void dst_free(struct dst_e
 	__dst_free(dst);
 }
 
+static inline void dst_rcu_free(struct rcu_head *head)
+{
+	struct dst_entry *dst = container_of(head, struct dst_entry, rcu_head);
+	dst_free(dst);
+}
+
 static inline void dst_confirm(struct dst_entry *dst)
 {
 	if (dst)
diff -puN net/ipv4/route.c~rcu-no-arg net/ipv4/route.c
--- linux-2.6.0-test2-rcu/net/ipv4/route.c~rcu-no-arg	2003-07-31 16:13:19.000000000 +0530
+++ linux-2.6.0-test2-rcu-dipankar/net/ipv4/route.c	2003-07-31 16:13:19.000000000 +0530
@@ -411,13 +411,13 @@ void __init rt_cache_proc_exit(void)
   
 static __inline__ void rt_free(struct rtable *rt)
 {
-	call_rcu(&rt->u.dst.rcu_head, (void (*)(void *))dst_free, &rt->u.dst);
+	call_rcu(&rt->u.dst.rcu_head, dst_rcu_free);
 }
 
 static __inline__ void rt_drop(struct rtable *rt)
 {
 	ip_rt_put(rt);
-	call_rcu(&rt->u.dst.rcu_head, (void (*)(void *))dst_free, &rt->u.dst);
+	call_rcu(&rt->u.dst.rcu_head, dst_rcu_free);
 }
 
 static __inline__ int rt_fast_clean(struct rtable *rth)

_

Re: [PATCH] RCU: Reduce size of rcu_head 1 of 2

[Andrew Morton]

Dipankar Sarma <dipankar@in.ibm.com> wrote:
>
> I have merged Rusty's original patch for reducing the size of
> rcu_heads by splitting the two main changes into two patches.

There are probably a lot of data structures in which we could save 4 (or 8)
bytes by converting things from doubly linked to singly linked.

And that's good, but given that at this time we are concentrating 100% on
stabilising 2.6 (aren't we?) I'll be letting these patches slide, thanks.

Re: [PATCH] RCU: Reduce size of rcu_head 1 of 2

[Andrew Morton]

Dipankar Sarma <dipankar@in.ibm.com> wrote:
>
> The linked-list change is internal enough for a future backport from
> 2.7. The only concern here is the change in call_rcu() API. What would
> be a good way to manage that ?

Oh I'd be okay with merging a change like this into (say) 2.6.3-pre1,
without it having had a run in 2.7.  We need to be able to do things like
that.

But right now we need to be fully focussed upon important features which
are late (cpumask_t, 64-bit dev_t, 4G+4G, etc) and upon stabilisation of the
current tree.

Re: [PATCH] RCU: Reduce size of rcu_head 1 of 2

[Dipankar Sarma]

On Thu, Jul 31, 2003 at 01:49:54PM -0700, Andrew Morton wrote:
> Dipankar Sarma <dipankar@in.ibm.com> wrote:
> >
> > I have merged Rusty's original patch for reducing the size of
> > rcu_heads by splitting the two main changes into two patches.
> 
> There are probably a lot of data structures in which we could save 4 (or 8)
> bytes by converting things from doubly linked to singly linked.
> 
> And that's good, but given that at this time we are concentrating 100% on
> stabilising 2.6 (aren't we?) I'll be letting these patches slide, thanks.

The linked-list change is internal enough for a future backport from
2.7. The only concern here is the change in call_rcu() API. What would
be a good way to manage that ?

Thanks
Dipankar

Re: [PATCH] RCU: Reduce size of rcu_head 1 of 2

[Rusty Russell]

In message <20030731142545.7bcb50fb.akpm@osdl.org> you write:
> Dipankar Sarma <dipankar@in.ibm.com> wrote:
> >
> > The linked-list change is internal enough for a future backport from
> > 2.7. The only concern here is the change in call_rcu() API. What would
> > be a good way to manage that ?
> 
> Oh I'd be okay with merging a change like this into (say) 2.6.3-pre1,
> without it having had a run in 2.7.  We need to be able to do things like
> that.

No, Andrew, no.

	Gratuitous change to API during stable series BAD BAD BAD.  If
you drop this it stays as is until 2.8.  The extra arg in
unneccessary, but breaking it is worse.

Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] RCU: Reduce size of rcu_head 1 of 2

[Andrew Morton]

Rusty Russell <rusty@rustcorp.com.au> wrote:
>
> 	Gratuitous change to API during stable series BAD BAD BAD.  If
> you drop this it stays as is until 2.8.  The extra arg in
> unneccessary, but breaking it is worse.

There won't be any out-of-tree users by then.

Re: [PATCH] RCU: Reduce size of rcu_head 1 of 2

[Rusty Russell]

In message <20030801160036.029e542b.akpm@osdl.org> you write:
> Rusty Russell <rusty@rustcorp.com.au> wrote:
> >
> > 	Gratuitous change to API during stable series BAD BAD BAD.  If
> > you drop this it stays as is until 2.8.  The extra arg in
> > unneccessary, but breaking it is worse.
> 
> There won't be any out-of-tree users by then.

Not that you will know of, that's the entire point IMHO.

All those people who forward port to 2.6, or who are developing
drivers for the first time, completely outside the "normal" channels.
Just please please please don't break any API in a stable series.

Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] RCU: Reduce size of rcu_head 1 of 2

[Mitchell Blank Jr]

Andrew Morton wrote:
> > 	Gratuitous change to API during stable series BAD BAD BAD.  If
> > you drop this it stays as is until 2.8.  The extra arg in
> > unneccessary, but breaking it is worse.
> 
> There won't be any out-of-tree users by then.

I must be misunderstanding something.  If the point of the patch is to
shrink "struct rcu_head" (which it seems to do) won't that change
offsets in all sorts of things like "struct dentry"?  I know we
officially don't care about binary modules but a change like that
seems pretty gratuitous for such a small gain.

This sounds like the kind of change that should either happen now or
wait for 2.7.1 (and not be backported into 2.6)

-Mitch

Re: [PATCH] RCU: Reduce size of rcu_head 1 of 2

[Greg KH]

On Sat, Aug 02, 2003 at 10:32:24AM +1000, Rusty Russell wrote:
> Just please please please don't break any API in a stable series.

We reserve the right to break any in-kernel api if it is deemed
necessary, this is Linux after all :)

thanks,

greg k-h

Re: [PATCH] RCU: Reduce size of rcu_head 1 of 2

[Matt Mackall]

On Fri, Aug 01, 2003 at 06:16:44PM -0700, Mitchell Blank Jr wrote:
> Andrew Morton wrote:
> > > 	Gratuitous change to API during stable series BAD BAD BAD.  If
> > > you drop this it stays as is until 2.8.  The extra arg in
> > > unneccessary, but breaking it is worse.
> > 
> > There won't be any out-of-tree users by then.
> 
> I must be misunderstanding something.  If the point of the patch is to
> shrink "struct rcu_head" (which it seems to do) won't that change
> offsets in all sorts of things like "struct dentry"?  I know we
> officially don't care about binary modules but a change like that
> seems pretty gratuitous for such a small gain.

Repeat after me: there is no module ABI.

-- 
Matt Mackall : http://www.selenic.com : of or relating to the moon

Re: [PATCH] RCU: Reduce size of rcu_head 1 of 2

[Rusty Russell]

In message <20030802011943.GA1107@kroah.com> you write:
> On Sat, Aug 02, 2003 at 10:32:24AM +1000, Rusty Russell wrote:
> > Just please please please don't break any API in a stable series.
> 
> We reserve the right to break any in-kernel api if it is deemed
> necessary, this is Linux after all :)

Sure.  But it's not neccessary.  The replacement is cleaner and
smaller, sure, but it's not worth changing once 2.6 is out.  In 2.7,
sure.

I'm happy to accept "no" from Andrew, but not happy to accept "we'll
just change the API midway through 2.6".

Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] RCU: Reduce size of rcu_head 1 of 2

[Alan Cox]

On Sad, 2003-08-02 at 02:49, Rusty Russell wrote:
> Sure.  But it's not neccessary.  The replacement is cleaner and
> smaller, sure, but it's not worth changing once 2.6 is out.  In 2.7,
> sure.
> 
> I'm happy to accept "no" from Andrew, but not happy to accept "we'll
> just change the API midway through 2.6".

Please distinguish API from ABI. There is no ABI, there is no need for
an ABI.

Re: [PATCH] RCU: Reduce size of rcu_head 1 of 2

[Rusty Russell]

In message <1059830126.19819.8.camel@dhcp22.swansea.linux.org.uk> you write:
> On Sad, 2003-08-02 at 02:49, Rusty Russell wrote:
> > Sure.  But it's not neccessary.  The replacement is cleaner and
> > smaller, sure, but it's not worth changing once 2.6 is out.  In 2.7,
> > sure.
> > 
> > I'm happy to accept "no" from Andrew, but not happy to accept "we'll
> > just change the API midway through 2.6".
> 
> Please distinguish API from ABI. There is no ABI, there is no need for
> an ABI.

You are confused, but it seems you are not alone.  I don't understand
where this talk of ABI came from.

There are two patches.  Both reduce the size of the "struct rcu_head".
One simply changes the struct rcu_head from a double linked list to a
single linked list.  The other eliminates the "void *data" arg, and
changes the prototype of the call_rcu() function to take a pointer to
the struct rcu_head, rather than a user-defined data ptr.

It is the latter that I am concerned about changing mid-stable-series.

Hope that clarifies,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] RCU: Reduce size of rcu_head 1 of 2

[Andrea Arcangeli]

Hi,

On Fri, Aug 08, 2003 at 12:21:04PM +1000, Rusty Russell wrote:
> There are two patches.  Both reduce the size of the "struct rcu_head".
> One simply changes the struct rcu_head from a double linked list to a
> single linked list.  The other eliminates the "void *data" arg, and
> changes the prototype of the call_rcu() function to take a pointer to
> the struct rcu_head, rather than a user-defined data ptr.
> 
> It is the latter that I am concerned about changing mid-stable-series.

given the number of users (a dozen) I wouldn't be concerned about the
API change either. Much better to change it now that there arne't out of
the tree users yet. IMHO kernel internal API changes aren't a problem if
there are few users all in tree like in this case.

Andrea

Re: [PATCH] RCU: Reduce size of rcu_head 1 of 2

[Rusty Russell]

In message <20030818141606.GU7862@dualathlon.random> you write:
> Hi,
> 
> On Fri, Aug 08, 2003 at 12:21:04PM +1000, Rusty Russell wrote:
> > It is the latter that I am concerned about changing mid-stable-series.
> 
> given the number of users (a dozen) I wouldn't be concerned about the
> API change either.

Hi Andrea,

	We don't know how many there are *outside* the tree during the
stable series, though.  It's really nice if APIs don't change
gratuitously during stable kernels, because there are lots of (free,
source available) patches which are outside the tree, and that's a
*good* thing, IMHO.

Change it now or leave it, IMHO.
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.