Mailing List Archive

This is a multi-part message in MIME format.
--------------E21141A3ED9088585281D8D9
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Hello Everybody!
The following simple patch is a correction of a design error
in the slab implementation.
Several YEARS ago it got introduced into the general linux kernel.
It is providing a mechanism of constructors and destructors for the
objects allocaed and deallocated. However still after YEARS I found
actually only one place in the kernel where it was used!
READ TWICE: Only one.
And there only the constructor was present and even the usage of it
was bogous at least. (Please see the corresponding comment in the
patch.)
READ TWICE: bogous.
There where good reaons why nobody hessitated to use this crap.
1. In linux all the major objects of special purpose nature are using
thin wrapper founctions for allocation and deallocation around the
generic allocator functions. This is accomplishing the very benefits
promised by the generic stuff without actually penalizing the general
case with care about checks for the existence of
constructors/destructors
atomic non atomic out of order calls any many many other checks.
2. The comment I have added to the place where the constructor mechanism
was previously used explains why it was bogous there. And it's quite
easy to introduce errors of the same kind by the usage of them in other
places. (If anybody had dared to actually use them!)
So my conclusion is there was once again somone blindly following
bad designs (however prominent doesn't matter for me.). And yes the
explanation above is a strong evidence that the Bonwick paper is
containing many many other lies too. (In esp. the whole colouring thing
seems to be a good joke resulting in optimizations only visibly in the
laboratory.)
So Linus please apply this straight forward patch before the next
release.
You told you where loving removing dead code :-). And this
patch doesn't change anything at all. It's just deleting and giving
at least me 16443 bytes of precious memmory back (OK __init not taked
into account).
It's stable the diff was produced under a kernel compiled with it.
And this mail will be send by it too. In the meantime the CD-ROM
drive of the very same mashine is plaing the "Sunday" album from
Faithless oh and X11 is running really:-).
BTW> The whole slab.c seems to be an enormous mess after second look....
The style it was done seems to come from the Lord of Coding himself.
Sombody was trying to accompilsh the Ultimate Truth with it. Of course
without thinking about esthetics, size, and actual usfullness of the
many
dead features it is providing. In whole it gives me the impression
that the epoche of Baroq is still not quite that long ago past ;-)
Marcin
PS. Yes I preffer a Techno allocator over one from Mozart.
--------------E21141A3ED9088585281D8D9
Content-Type: text/plain; charset=us-ascii;
name="diff-pre6"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="diff-pre6"
diff -urN linux-orig/fs/buffer.c linux/fs/buffer.c
--- linux-orig/fs/buffer.c Mon Jan 11 00:52:30 1999
+++ linux/fs/buffer.c Tue Jan 12 20:58:31 1999
@@ -1553,8 +1553,7 @@

bh_cachep = kmem_cache_create("buffer_head",
sizeof(struct buffer_head),
- 0,
- SLAB_HWCACHE_ALIGN, NULL, NULL);
+ 0, SLAB_HWCACHE_ALIGN);
if(!bh_cachep)
panic("Cannot create buffer head SLAB cache\n");
/*
diff -urN linux-orig/fs/dcache.c linux/fs/dcache.c
--- linux-orig/fs/dcache.c Mon Jan 11 00:52:30 1999
+++ linux/fs/dcache.c Tue Jan 12 21:00:02 1999
@@ -911,19 +911,14 @@
int i;
struct list_head *d = dentry_hashtable;

- /*
- * A constructor could be added for stable state like the lists,
- * but it is probably not worth it because of the cache nature
- * of the dcache.
+ /*
* If fragmentation is too bad then the SLAB_HWCACHE_ALIGN
* flag could be removed here, to hint to the allocator that
- * it should not try to get multiple page regions.
+ * it should not try to get multiple page regions.
*/
- dentry_cache = kmem_cache_create("dentry_cache",
+ dentry_cache = kmem_cache_create("dentry",
sizeof(struct dentry),
- 0,
- SLAB_HWCACHE_ALIGN,
- NULL, NULL);
+ 0, SLAB_HWCACHE_ALIGN);
if (!dentry_cache)
panic("Cannot create dentry cache");

diff -urN linux-orig/fs/dquot.c linux/fs/dquot.c
--- linux-orig/fs/dquot.c Tue Dec 29 20:07:37 1998
+++ linux/fs/dquot.c Tue Jan 12 23:32:26 1999
@@ -1163,17 +1163,20 @@

void __init dquot_init_hash(void)
{
- printk(KERN_NOTICE "VFS: Diskquotas version %s initialized\n", __DQUOT_VERSION__);
-
+ /* This is the very second only place where we don't align upon
+ * cache line boundary. The +4 looks very suspicious to me...
+ */
dquot_cachep = kmem_cache_create("dquot", sizeof(struct dquot),
sizeof(unsigned long) * 4,
- SLAB_HWCACHE_ALIGN, NULL, NULL);
+ SLAB_HWCACHE_ALIGN);

if (!dquot_cachep)
panic("Cannot create dquot SLAB cache\n");

memset(dquot_hash, 0, sizeof(dquot_hash));
memset((caddr_t)&dqstats, 0, sizeof(dqstats));
+
+ printk(KERN_NOTICE "VFS: Diskquotas version %s initialized\n", __DQUOT_VERSION__);
}

/*
diff -urN linux-orig/fs/file_table.c linux/fs/file_table.c
--- linux-orig/fs/file_table.c Tue Dec 29 20:10:48 1998
+++ linux/fs/file_table.c Tue Jan 12 20:57:11 1999
@@ -52,8 +52,7 @@
void __init file_table_init(void)
{
filp_cache = kmem_cache_create("filp", sizeof(struct file),
- 0,
- SLAB_HWCACHE_ALIGN, NULL, NULL);
+ 0, SLAB_HWCACHE_ALIGN);
if(!filp_cache)
panic("VFS: Cannot alloc filp SLAB cache.");
/*
diff -urN linux-orig/include/linux/slab.h linux/include/linux/slab.h
--- linux-orig/include/linux/slab.h Mon Jan 11 01:04:59 1999
+++ linux/include/linux/slab.h Tue Jan 12 21:07:36 1999
@@ -39,18 +39,11 @@
#define SLAB_HIGH_PACK 0x00004000UL /* XXX */
#endif

-/* flags passed to a constructor func */
-#define SLAB_CTOR_CONSTRUCTOR 0x001UL /* if not set, then deconstructor */
-#define SLAB_CTOR_ATOMIC 0x002UL /* tell constructor it can't sleep */
-#define SLAB_CTOR_VERIFY 0x004UL /* tell constructor it's a verify call */
-
/* prototypes */
extern long kmem_cache_init(long, long);
extern void kmem_cache_sizes_init(void);
extern kmem_cache_t *kmem_find_general_cachep(size_t);
-extern kmem_cache_t *kmem_cache_create(const char *, size_t, size_t, unsigned long,
- void (*)(void *, kmem_cache_t *, unsigned long),
- void (*)(void *, kmem_cache_t *, unsigned long));
+extern kmem_cache_t *kmem_cache_create(const char *, size_t, size_t, unsigned long);
extern int kmem_cache_shrink(kmem_cache_t *);
extern void *kmem_cache_alloc(kmem_cache_t *, int);
extern void kmem_cache_free(kmem_cache_t *, void *);
@@ -60,6 +53,8 @@
extern void kfree_s(const void *, size_t);

extern void kmem_cache_reap(int);
+
+/* interface to the /proc fs */
extern int get_slabinfo(char *);

/* System wide caches */
diff -urN linux-orig/kernel/fork.c linux/kernel/fork.c
--- linux-orig/kernel/fork.c Mon Jan 11 00:52:33 1999
+++ linux/kernel/fork.c Tue Jan 12 20:41:21 1999
@@ -124,8 +124,7 @@
int i;

uid_cachep = kmem_cache_create("uid_cache", sizeof(struct user_struct),
- 0,
- SLAB_HWCACHE_ALIGN, NULL, NULL);
+ 0, SLAB_HWCACHE_ALIGN);
if(!uid_cachep)
panic("Cannot create uid taskcount SLAB cache\n");

@@ -632,11 +631,9 @@

void __init filescache_init(void)
{
- files_cachep = kmem_cache_create("files_cache",
+ files_cachep = kmem_cache_create("files",
sizeof(struct files_struct),
- 0,
- SLAB_HWCACHE_ALIGN,
- NULL, NULL);
+ 0, SLAB_HWCACHE_ALIGN);
if (!files_cachep)
panic("Cannot create files cache");
}
diff -urN linux-orig/kernel/signal.c linux/kernel/signal.c
--- linux-orig/kernel/signal.c Tue Dec 29 20:10:31 1998
+++ linux/kernel/signal.c Tue Jan 12 23:35:42 1999
@@ -20,12 +20,6 @@

#define DEBUG_SIG 0

-#if DEBUG_SIG
-#define SIG_SLAB_DEBUG (SLAB_DEBUG_FREE | SLAB_RED_ZONE /* | SLAB_POISON */)
-#else
-#define SIG_SLAB_DEBUG 0
-#endif
-
static kmem_cache_t *signal_queue_cachep;

int nr_queued_signals;
@@ -33,11 +27,25 @@

void __init signals_init(void)
{
+
+ /*
+ * 12/01/1999 Marcin Dalecki <dalecki@cs.net.pl>:
+ *
+ * The alignment parameters passed to kmem_cache_create passed here
+ * look *very* suspicious to me. Anybody else is just using
+ * (...,...,0,SLAB_HWCACHE_ALIGN). Maybe if we could use it here to,
+ * we would have a small penalty here, but the simplifications possible
+ * in the whole slab mechanism which this would just benefit everything
+ * else much more significantly.
+ *
+ * Richard would it be OK?
+ */
+
signal_queue_cachep =
kmem_cache_create("signal_queue",
sizeof(struct signal_queue),
__alignof__(struct signal_queue),
- SIG_SLAB_DEBUG, NULL, NULL);
+ 0);
}


diff -urN linux-orig/mm/mmap.c linux/mm/mmap.c
--- linux-orig/mm/mmap.c Tue Dec 29 20:09:49 1998
+++ linux/mm/mmap.c Tue Jan 12 20:43:29 1999
@@ -708,15 +708,13 @@
{
vm_area_cachep = kmem_cache_create("vm_area_struct",
sizeof(struct vm_area_struct),
- 0, SLAB_HWCACHE_ALIGN,
- NULL, NULL);
+ 0, SLAB_HWCACHE_ALIGN);
if(!vm_area_cachep)
panic("vma_init: Cannot alloc vm_area_struct cache.");

mm_cachep = kmem_cache_create("mm_struct",
sizeof(struct mm_struct),
- 0, SLAB_HWCACHE_ALIGN,
- NULL, NULL);
+ 0, SLAB_HWCACHE_ALIGN);
if(!mm_cachep)
panic("vma_init: Cannot alloc mm_struct cache.");
}
diff -urN linux-orig/mm/slab.c linux/mm/slab.c
--- linux-orig/mm/slab.c Tue Dec 29 20:09:49 1998
+++ linux/mm/slab.c Tue Jan 12 23:03:08 1999
@@ -116,7 +116,7 @@
* 0 if you wish to reduce memory usage.
*
* SLAB_DEBUG_SUPPORT - 1 for kmem_cache_create() to honour; SLAB_DEBUG_FREE,
- * SLAB_DEBUG_INITIAL, SLAB_RED_ZONE & SLAB_POISON.
+ * SLAB_RED_ZONE & SLAB_POISON.
* 0 for faster, smaller, code (especially in the critical paths).
*
* SLAB_STATS - 1 to collect stats for /proc/slabinfo.
@@ -135,11 +135,11 @@
/* Legal flag mask for kmem_cache_create(). */
#if SLAB_DEBUG_SUPPORT
#if 0
-#define SLAB_C_MASK (SLAB_DEBUG_FREE|SLAB_DEBUG_INITIAL|SLAB_RED_ZONE| \
+#define SLAB_C_MASK (SLAB_DEBUG_FREE|SLAB_RED_ZONE| \
SLAB_POISON|SLAB_HWCACHE_ALIGN|SLAB_NO_REAP| \
SLAB_HIGH_PACK)
#endif
-#define SLAB_C_MASK (SLAB_DEBUG_FREE|SLAB_DEBUG_INITIAL|SLAB_RED_ZONE| \
+#define SLAB_C_MASK (SLAB_DEBUG_FREE|SLAB_RED_ZONE| \
SLAB_POISON|SLAB_HWCACHE_ALIGN|SLAB_NO_REAP)
#else
#if 0
@@ -233,8 +233,6 @@
unsigned long c_dflags; /* dynamic flags */
size_t c_org_size;
unsigned long c_gfporder; /* order of pgs per slab (2^n) */
- void (*c_ctor)(void *, kmem_cache_t *, unsigned long); /* constructor func */
- void (*c_dtor)(void *, kmem_cache_t *, unsigned long); /* de-constructor func */
unsigned long c_align; /* alignment of objs */
size_t c_colour; /* cache colouring range */
size_t c_colour_next;/* cache colouring */
@@ -377,7 +375,7 @@
/* growing */ 0,
/* dflags */ 0,
/* org_size, gfp */ 0, 0,
-/* ctor, dtor, align */ NULL, NULL, L1_CACHE_BYTES,
+/* align */ L1_CACHE_BYTES,
/* colour, colour_next */ 0, 0,
/* failures */ 0,
/* name */ "kmem_cache",
@@ -459,7 +457,7 @@
unsigned int found = 0;

cache_slabp = kmem_cache_create("slab_cache", sizeof(kmem_slab_t),
- 0, SLAB_HWCACHE_ALIGN, NULL, NULL);
+ 0, SLAB_HWCACHE_ALIGN);
if (cache_slabp) {
char **names = cache_sizes_name;
cache_sizes_t *sizes = cache_sizes;
@@ -471,7 +469,7 @@
* allow tighter packing of the smaller caches. */
if (!(sizes->cs_cachep =
kmem_cache_create(*names++, sizes->cs_size,
- 0, SLAB_HWCACHE_ALIGN, NULL, NULL)))
+ 0, SLAB_HWCACHE_ALIGN)))
goto panic_time;
if (!found) {
/* Inc off-slab bufctl limit until the ceiling is hit. */
@@ -603,46 +601,6 @@
static void
kmem_slab_destroy(kmem_cache_t *cachep, kmem_slab_t *slabp)
{
- if (cachep->c_dtor
-#if SLAB_DEBUG_SUPPORT
- || cachep->c_flags & (SLAB_POISON | SLAB_RED_ZONE)
-#endif /*SLAB_DEBUG_SUPPORT*/
- ) {
- /* Doesn't use the bufctl ptrs to find objs. */
- unsigned long num = cachep->c_num;
- void *objp = slabp->s_mem;
- do {
-#if SLAB_DEBUG_SUPPORT
- if (cachep->c_flags & SLAB_RED_ZONE) {
- if (*((unsigned long*)(objp)) != SLAB_RED_MAGIC1)
- printk(KERN_ERR "kmem_slab_destroy: "
- "Bad front redzone - %s\n",
- cachep->c_name);
- objp += BYTES_PER_WORD;
- if (*((unsigned long*)(objp+cachep->c_org_size)) !=
- SLAB_RED_MAGIC1)
- printk(KERN_ERR "kmem_slab_destroy: "
- "Bad rear redzone - %s\n",
- cachep->c_name);
- }
- if (cachep->c_dtor)
-#endif /*SLAB_DEBUG_SUPPORT*/
- (cachep->c_dtor)(objp, cachep, 0);
-#if SLAB_DEBUG_SUPPORT
- else if (cachep->c_flags & SLAB_POISON) {
- if (kmem_check_poison_obj(cachep, objp))
- printk(KERN_ERR "kmem_slab_destroy: "
- "Bad poison - %s\n", cachep->c_name);
- }
- if (cachep->c_flags & SLAB_RED_ZONE)
- objp -= BYTES_PER_WORD;
-#endif /* SLAB_DEBUG_SUPPORT */
- objp += cachep->c_offset;
- if (!slabp->s_index)
- objp += sizeof(kmem_bufctl_t);
- } while (--num);
- }
-
slabp->s_magic = SLAB_MAGIC_DESTROYED;
if (slabp->s_index)
kmem_cache_free(cachep->c_index_cachep, slabp->s_index);
@@ -676,9 +634,8 @@
* NOTE: The 'name' is assumed to be memory that is _not_ going to disappear.
*/
kmem_cache_t *
-kmem_cache_create(const char *name, size_t size, size_t offset,
- unsigned long flags, void (*ctor)(void*, kmem_cache_t *, unsigned long),
- void (*dtor)(void*, kmem_cache_t *, unsigned long))
+kmem_cache_create(const char *name, size_t size,
+ size_t offset, unsigned long flags)
{
const char *func_nm= KERN_ERR "kmem_create: ";
kmem_cache_t *searchp;
@@ -708,41 +665,11 @@
goto opps;
}

- if (dtor && !ctor) {
- /* Decon, but no con - doesn't make sense */
- printk("%sDecon but no con - %s\n", func_nm, name);
- goto opps;
- }
-
if (offset < 0 || offset > size) {
printk("%sOffset weird %d - %s\n", func_nm, (int) offset, name);
offset = 0;
}

-#if SLAB_DEBUG_SUPPORT
- if ((flags & SLAB_DEBUG_INITIAL) && !ctor) {
- /* No constructor, but inital state check requested */
- printk("%sNo con, but init state check requested - %s\n", func_nm, name);
- flags &= ~SLAB_DEBUG_INITIAL;
- }
-
- if ((flags & SLAB_POISON) && ctor) {
- /* request for poisoning, but we can't do that with a constructor */
- printk("%sPoisoning requested, but con given - %s\n", func_nm, name);
- flags &= ~SLAB_POISON;
- }
-#if 0
- if ((flags & SLAB_HIGH_PACK) && ctor) {
- printk("%sHigh pack requested, but con given - %s\n", func_nm, name);
- flags &= ~SLAB_HIGH_PACK;
- }
- if ((flags & SLAB_HIGH_PACK) && (flags & (SLAB_POISON|SLAB_RED_ZONE))) {
- printk("%sHigh pack requested, but with poisoning/red-zoning - %s\n",
- func_nm, name);
- flags &= ~SLAB_HIGH_PACK;
- }
-#endif
-#endif /* SLAB_DEBUG_SUPPORT */
#endif /* SLAB_MGMT_CHECKS */

/* Always checks flags, a caller might be expecting debug
@@ -951,8 +878,6 @@
cachep->c_firstp = kmem_slab_end(cachep);
cachep->c_lastp = kmem_slab_end(cachep);
cachep->c_flags = flags;
- cachep->c_ctor = ctor;
- cachep->c_dtor = dtor;
cachep->c_magic = SLAB_C_MAGIC;
cachep->c_name = name; /* Simply point to the name. */
spin_lock_init(&cachep->c_spinlock);
@@ -1072,8 +997,7 @@
}

static inline void
-kmem_cache_init_objs(kmem_cache_t * cachep, kmem_slab_t * slabp, void *objp,
- unsigned long ctor_flags)
+kmem_cache_init_objs(kmem_cache_t * cachep, kmem_slab_t * slabp, void *objp)
{
kmem_bufctl_t **bufpp = &slabp->s_freep;
unsigned long num = cachep->c_num-1;
@@ -1087,12 +1011,6 @@
}
#endif /* SLAB_DEBUG_SUPPORT */

- /* Constructors are not allowed to allocate memory from the same cache
- * which they are a constructor for. Otherwise, deadlock.
- * They must also be threaded.
- */
- if (cachep->c_ctor)
- cachep->c_ctor(objp, cachep, ctor_flags);
#if SLAB_DEBUG_SUPPORT
else if (cachep->c_flags & SLAB_POISON) {
/* need to poison the objs */
@@ -1139,7 +1057,6 @@
void *objp;
size_t offset;
unsigned int dma, local_flags;
- unsigned long ctor_flags;
unsigned long save_flags;

/* Be lazy and only check for valid flags here,
@@ -1165,14 +1082,7 @@
flags &= ~SLAB_LEVEL_MASK;
flags |= SLAB_ATOMIC;
}
- ctor_flags = SLAB_CTOR_CONSTRUCTOR;
local_flags = (flags & SLAB_LEVEL_MASK);
- if (local_flags == SLAB_ATOMIC) {
- /* Not allowed to sleep. Need to tell a constructor about
- * this - it might need to know...
- */
- ctor_flags |= SLAB_CTOR_ATOMIC;
- }

/* About to mess with non-constant members - lock. */
spin_lock_irqsave(&cachep->c_spinlock, save_flags);
@@ -1228,7 +1138,7 @@
* an obj and its related bufctl. For off-slab bufctls, c_offset is
* the distance between objs in the slab.
*/
- kmem_cache_init_objs(cachep, slabp, objp, ctor_flags);
+ kmem_cache_init_objs(cachep, slabp, objp);

spin_lock_irq(&cachep->c_spinlock);

@@ -1539,10 +1449,6 @@
return;
#if SLAB_DEBUG_SUPPORT
init_state_check:
- /* Need to call the slab's constructor so the
- * caller can perform a verify of its state (debugging).
- */
- cachep->c_ctor(objp, cachep, SLAB_CTOR_CONSTRUCTOR|SLAB_CTOR_VERIFY);
goto finished_initial;
extra_checks:
if (!kmem_extra_free_checks(cachep, slabp->s_freep, bufp, objp)) {
@@ -1842,7 +1748,7 @@
kmem_cache_t *test_cachep;

printk(KERN_INFO "kmem_test() - start\n");
- test_cachep = kmem_cache_create("test-cachep", 16, 0, SLAB_RED_ZONE|SLAB_POISON, NULL, NULL);
+ test_cachep = kmem_cache_create("test-cachep", 16, 0, SLAB_RED_ZONE|SLAB_POISON);
if (test_cachep) {
char *objp = kmem_cache_alloc(test_cachep, SLAB_KERNEL);
if (objp) {
diff -urN linux-orig/net/core/skbuff.c linux/net/core/skbuff.c
--- linux-orig/net/core/skbuff.c Tue Sep 15 07:52:10 1998
+++ linux/net/core/skbuff.c Tue Jan 12 21:03:54 1999
@@ -107,13 +107,35 @@
#ifdef CONFIG_INET
printk("IP fragment buffer size : %u\n",
atomic_read(&ip_frag_mem));
-#endif
+#endif
+}
+
+/*
+ * Slab constructor for a skb head.
+ */
+static inline void skb_headerinit(struct sk_buff *skb)
+{
+ skb->destructor = NULL;
+ skb->pkt_type = PACKET_HOST; /* Default type */
+ skb->pkt_bridged = 0; /* Not bridged */
+ skb->prev = skb->next = NULL;
+ skb->list = NULL;
+ skb->sk = NULL;
+ skb->stamp.tv_sec=0; /* No idea about time */
+ skb->ip_summed = 0;
+ skb->security = 0; /* By default packets are insecure */
+ skb->dst = NULL;
+#ifdef CONFIG_IP_FIREWALL
+ skb->fwmark = 0;
+#endif
+ memset(skb->cb, 0, sizeof(skb->cb));
+ skb->priority = 0;
}

-/* Allocate a new skbuff. We do this ourselves so we can fill in a few
+/* Allocate a new skbuff. We do this ourselves so we can fill in a few
* 'private' fields and also do memory statistics to find all the
* [BEEP] leaks.
- *
+ *
*/

struct sk_buff *alloc_skb(unsigned int size,int gfp_mask)
@@ -134,6 +156,7 @@
skb = kmem_cache_alloc(skbuff_head_cache, gfp_mask);
if (skb == NULL)
goto nohead;
+ skb_headerinit(skb);

/* Get the DATA. Size must match skb_add_mtu(). */
size = ((size + 15) & ~15);
@@ -174,30 +197,6 @@
}


-/*
- * Slab constructor for a skb head.
- */
-static inline void skb_headerinit(void *p, kmem_cache_t *cache,
- unsigned long flags)
-{
- struct sk_buff *skb = p;
-
- skb->destructor = NULL;
- skb->pkt_type = PACKET_HOST; /* Default type */
- skb->pkt_bridged = 0; /* Not bridged */
- skb->prev = skb->next = NULL;
- skb->list = NULL;
- skb->sk = NULL;
- skb->stamp.tv_sec=0; /* No idea about time */
- skb->ip_summed = 0;
- skb->security = 0; /* By default packets are insecure */
- skb->dst = NULL;
-#ifdef CONFIG_IP_FIREWALL
- skb->fwmark = 0;
-#endif
- memset(skb->cb, 0, sizeof(skb->cb));
- skb->priority = 0;
-}

/*
* Free an skbuff by memory without cleaning the state.
@@ -224,7 +223,7 @@
dst_release(skb->dst);
if(skb->destructor)
skb->destructor(skb);
- skb_headerinit(skb, NULL, 0); /* clean state */
+ skb_headerinit(skb); /* clean state */
kfree_skbmem(skb);
}

@@ -235,15 +234,35 @@
struct sk_buff *skb_clone(struct sk_buff *skb, int gfp_mask)
{
struct sk_buff *n;
-
+
+ /*
+ * 12/01/1999 Marcin Dalecki <dalecki@cs.net.pl>
+ *
+ * The following code actually reveals, why nobody was using the
+ * constructor destructor mechanism previously provided by the slab
+ * allocator. Here the kmem_cache_alloc was calling the skb_headerinit
+ * function indirectly to just reset the results of it by copying over
+ * the values from the cloned skb.
+ *
+ * In Linux we are generally using thinny wrappers around the special
+ * purpose allocation functions, we do the initialization/cleaning on
+ * demand for all the different kinds of objects present across the
+ * kernel. This is providing the same level of efficiency without the
+ * penalties of additional complexity and obfuscation, provided by the
+ * previously present general constructor/destructor mechanis of the
+ * slab.
+ *
+ * It was a DESIGN ERROR in respect of the rest of the kernel.
+ */
+
n = kmem_cache_alloc(skbuff_head_cache, gfp_mask);
if (!n)
return NULL;
-
memcpy(n, skb, sizeof(*n));
+
atomic_inc(skb_datarefp(skb));
skb->cloned = 1;
-
+
atomic_inc(&net_allocs);
atomic_inc(&net_skbcount);
dst_clone(n->dst);
@@ -369,11 +388,9 @@

void __init skb_init(void)
{
- skbuff_head_cache = kmem_cache_create("skbuff_head_cache",
+ skbuff_head_cache = kmem_cache_create("skbuff_head",
sizeof(struct sk_buff),
- 0,
- SLAB_HWCACHE_ALIGN,
- skb_headerinit, NULL);
+ 0, SLAB_HWCACHE_ALIGN);
if (!skbuff_head_cache)
panic("cannot create skbuff cache");
}
diff -urN linux-orig/net/core/sock.c linux/net/core/sock.c
--- linux-orig/net/core/sock.c Tue Dec 29 20:09:04 1998
+++ linux/net/core/sock.c Tue Jan 12 21:03:11 1999
@@ -287,8 +287,8 @@
case SO_PASSCRED:
sock->passcred = valbool;
break;
-
-
+
+
#ifdef CONFIG_NETDEVICES
case SO_BINDTODEVICE:
{
@@ -509,8 +509,8 @@

void __init sk_init(void)
{
- sk_cachep = kmem_cache_create("sock", sizeof(struct sock), 0,
- SLAB_HWCACHE_ALIGN, 0, 0);
+ sk_cachep = kmem_cache_create("sock", sizeof(struct sock),
+ 0, SLAB_HWCACHE_ALIGN);

}

diff -urN linux-orig/net/ipv4/tcp.c linux/net/ipv4/tcp.c
--- linux-orig/net/ipv4/tcp.c Mon Jan 11 00:52:33 1999
+++ linux/net/ipv4/tcp.c Tue Jan 12 21:05:28 1999
@@ -1754,22 +1754,19 @@

tcp_openreq_cachep = kmem_cache_create("tcp_open_request",
sizeof(struct open_request),
- 0, SLAB_HWCACHE_ALIGN,
- NULL, NULL);
+ 0, SLAB_HWCACHE_ALIGN);
if(!tcp_openreq_cachep)
panic("tcp_init: Cannot alloc open_request cache.");

tcp_bucket_cachep = kmem_cache_create("tcp_bind_bucket",
sizeof(struct tcp_bind_bucket),
- 0, SLAB_HWCACHE_ALIGN,
- NULL, NULL);
+ 0, SLAB_HWCACHE_ALIGN);
if(!tcp_bucket_cachep)
panic("tcp_init: Cannot alloc tcp_bind_bucket cache.");

tcp_timewait_cachep = kmem_cache_create("tcp_tw_bucket",
sizeof(struct tcp_tw_bucket),
- 0, SLAB_HWCACHE_ALIGN,
- NULL, NULL);
+ 0, SLAB_HWCACHE_ALIGN);
if(!tcp_timewait_cachep)
panic("tcp_init: Cannot alloc tcp_tw_bucket cache.");
}
--------------E21141A3ED9088585281D8D9--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/