Skip to contentSkip to conversation

: Migrate to new naming scheme, using macro to declare cache

PATCH In review Gerrit Renker 2 participants 6 messages Jun 11, 2007 – Jun 17, 2007 Networking

lore
Share & tools
lore
#0 lore raw

PATCH 3/6 : Migrate to new naming scheme, using macro to declare cache

[TFRC]: Migrate to new naming scheme, using macro to declare cache

This patch
* begins migration to consistent naming scheme of this changeset -> all functions of the TFRC module begin with `tfrc_';
* simplifies cache allocation using a macro (details below);
* simplifies allocation from cache by using gfp_any();
* renames routines into cache_init()/cache_cleanup() to show what they do;
* is justified by having almost twice as many deletions as insertions.

Details:
The TFRC module exports a cache-allocation service which can be used to set up transmitter histories. The present solution to declare this uses a wrapper around struct kmem_cache. Confusingly, the wrapper was called `_hist', but it is in reality a cache. Replaced by a macro to serve the same purpose.

---
 net/dccp/ccids/ccid3.c              |   15 +++++--------
 net/dccp/ccids/lib/packet_history.c |   41 ++++++++++++------------------------
 net/dccp/ccids/lib/packet_history.h |    6 +++--
 3 files changed, 24 insertions(+), 38 deletions(-)
net/dccp/ccids/lib/packet_history.h +4 -2
@@ -52,8 +52,10 @@
 /*
  * 	Transmitter History data structures and declarations
  */
-extern struct dccp_tx_hist *dccp_tx_hist_new(const char *name);
-extern void		    dccp_tx_hist_delete(struct dccp_tx_hist *hist);
+#define DECLARE_TFRC_TX_CACHE(name)	static struct kmem_cache *(name);
+
+extern int  tfrc_tx_cache_init(struct kmem_cache **cache, const char *name);
+extern void tfrc_tx_cache_cleanup(struct kmem_cache *cache);
 
 /**
  *  tfrc_tx_hist  -  Simple singly-linked TX history list
net/dccp/ccids/lib/packet_history.c +14 -27

63 lines not on this page · raw message

net/dccp/ccids/ccid3.c +6 -9
@@ -47,7 +47,7 @@ static int ccid3_debug;
 #define ccid3_pr_debug(format, a...)
 #endif
 
-static struct dccp_tx_hist *ccid3_tx_hist;
+DECLARE_TFRC_TX_CACHE(ccid3_tx_hist);
 static struct dccp_rx_hist *ccid3_rx_hist;
 static struct dccp_li_hist *ccid3_li_hist;
 
@@ -1202,8 +1202,7 @@ static __init int ccid3_module_init(void
 	if (ccid3_rx_hist = NULL)
 		goto out;
 
-	ccid3_tx_hist = dccp_tx_hist_new("ccid3");
-	if (ccid3_tx_hist = NULL)
+	if (tfrc_tx_cache_init(&ccid3_tx_hist, "ccid3"))
 		goto out_free_rx;
 
 	ccid3_li_hist = dccp_li_hist_new("ccid3");
@@ -1220,8 +1219,7 @@ out_free_loss_interval_history:
 	dccp_li_hist_delete(ccid3_li_hist);
 	ccid3_li_hist = NULL;
 out_free_tx:
-	dccp_tx_hist_delete(ccid3_tx_hist);
-	ccid3_tx_hist = NULL;
+	tfrc_tx_cache_cleanup(ccid3_tx_hist);
 out_free_rx:
 	dccp_rx_hist_delete(ccid3_rx_hist);
 	ccid3_rx_hist = NULL;
@@ -1233,10 +1231,9 @@ static __exit void ccid3_module_exit(voi
 {
 	ccid_unregister(&ccid3);
 
-	if (ccid3_tx_hist != NULL) {
-		dccp_tx_hist_delete(ccid3_tx_hist);
-		ccid3_tx_hist = NULL;
-	}
+	if (ccid3_tx_hist != NULL)
+		tfrc_tx_cache_cleanup(ccid3_tx_hist);
+
 	if (ccid3_rx_hist != NULL) {
 		dccp_rx_hist_delete(ccid3_rx_hist);
 		ccid3_rx_hist = NULL;
#1 lore raw
Quoted 2 lines · Gerrit Renker

[TFRC]: Migrate to new naming scheme, using macro to declare cache

I agree totally with the intent. I reworked li_hist as well but Arnaldo decided to do in a better way and I think he'll want the same for this also.

See his patch (which is in Dave M's tree now) at:
http://www.mail-archive.com/[email protected]/msg01879.html

I also suspect his patch will stop yours applying.

Ian

Signature · 3 lines
Web: http://wand.net.nz/~iam4/
Blog: http://iansblog.jandi.co.nz
WAND Network Research Group
#2 lore raw
In reply to Gerrit Renker

Ian McDonald wrote:

|  > [TFRC]: Migrate to new naming scheme, using macro to declare cache
|  >
|  I agree totally with the intent. I reworked li_hist as well but
|  Arnaldo decided to do in a better way and I think he'll want the same
|  for this also.

Is this an Ack?

|  See his patch (which is in Dave M's tree now) at:
|  http://www.mail-archive.com/[email protected]/msg01879.html
|  
|  I also suspect his patch will stop yours applying.
|  

Yes that's right - I worked out the intersection of recent changes to the patches submitted so far, and above patch was part of it. Will send that later.

#3 lore raw
In reply to Gerrit Renker
Quoted 7 lines · Gerrit Renker

Ian McDonald wrote:

|  > [TFRC]: Migrate to new naming scheme, using macro to declare cache
|  >
|  I agree totally with the intent. I reworked li_hist as well but
|  Arnaldo decided to do in a better way and I think he'll want the same
|  for this also.

Is this an Ack?

Not yet - it's saying look at his patch and see if you want to match his style...

Quoted 4 lines
|  See his patch (which is in Dave M's tree now) at:
|  http://www.mail-archive.com/[email protected]/msg01879.html
|
Signature · 3 lines
Web: http://wand.net.nz/~iam4/
Blog: http://iansblog.jandi.co.nz
WAND Network Research Group
#4 lore raw
In reply to Gerrit Renker

Quoting Ian McDonald:

|  On 6/17/07, Gerrit Renker <[email protected]> wrote:
|  > Ian McDonald wrote:
|  > |  > [TFRC]: Migrate to new naming scheme, using macro to declare cache
|  > |  >
|  > |  I agree totally with the intent. I reworked li_hist as well but
|  > |  Arnaldo decided to do in a better way and I think he'll want the same
|  > |  for this also.
|  > Is this an Ack?
|  
|  Not yet - it's saying look at his patch and see if you want to match
|  his style...

Difficult to see what is meant. I think that my set mirrors in a way what Arnaldo does in the spirit, but not in detail. Since in the end the approach is different. But the principles are similar:
* also use explicit allocation/deallocation routines
* also put the slab allocation into the module, as Arnaldos patch
* effectively the same functions are removed in my changeset
* as discussed earlier, modularity of calc_first_li is achieved by using a function pointer.

=> wonder whether I should add `__read_mostly' to the slab declaration, but that is
a smaller issue. I will think about this.

The main difference is that in my changeset further functionality is implemented which intersects with the work Arnaldo has been doing.

|  >
|  > |  See his patch (which is in Dave M's tree now) at:
|  > |  http://www.mail-archive.com/[email protected]/msg01879.html
|  > |
#5 lore raw
In reply to Gerrit Renker
Quoted 2 lines · Gerrit Renker

The main difference is that in my changeset further functionality is implemented which intersects with the work Arnaldo has been doing.

OK.

Signature · 3 lines
Web: http://wand.net.nz/~iam4/
Blog: http://iansblog.jandi.co.nz
WAND Network Research Group