updated ipc lock patch
include/linux/msg.h
Quoted 3 lines
diff -urpN --exclude TAGS -X
/home/rusty/devel/kernel/kernel-patches/current-dontdiff--minimalworking-2.5.44-mm4-ipc-rcu/include/linux/msg.hworking-2.5.44-mm4-ipc-rcu-fix/include/linux/msg.hinclude/linux/msg.h +0
-0
Oops. That patch had some fluff in msg.h and sem.h. Delete those, or just use this instead (still against Mingming's mm4 "ignore kmalloc failure" patch):
ipc/util.c +51 -35
@@ -213,21 +213,49 @@ struct kern_ipc_perm* ipc_rmid(struct ipreturn p; } +struct ipc_rcu_kmalloc +{ +struct rcu_head rcu; +/* "void *" makes sure alignment of following data is sane. */ +void *data[0 ]; +}; + +struct ipc_rcu_vmalloc +{ +struct rcu_head rcu; +struct work_struct work; +/* "void *" makes sure alignment of following data is sane. */ +void *data[0 ]; +}; + +static inline int use_vmalloc (int size) +{ +/* Too big for a single page? */ +if (sizeof (struct ipc_rcu_kmalloc ) + size >PAGE_SIZE ) +return 1 ; +return 0 ; +} +/** * ipc_alloc - allocate ipc space * @size: size desired * * Allocate memory from the appropriate pools and return a pointer to it. -* NULL is returned if the allocation fails +* NULL is returned if the allocation fails. This can be freed with +* ipc_free (to free immediately) or ipc_rcu_free (to free once safe). */ -void *ipc_alloc (int size) {void * out; -if (size >PAGE_SIZE ) - out =vmalloc (size); -else - out =kmalloc (size,GFP_KERNEL ); +/* We prepend the allocation with the rcu struct, and +workqueue if necessary (for vmalloc). */ +if (use_vmalloc (size)) { + out =vmalloc (sizeof (struct ipc_rcu_vmalloc ) + size); +if (out) out +=sizeof (struct ipc_rcu_vmalloc ); + }else { + out =kmalloc (sizeof (struct ipc_rcu_kmalloc )+size,GFP_KERNEL ); +if (out) out +=sizeof (struct ipc_rcu_kmalloc ); + }return out; }
@@ -242,48 +270,36 @@ void* ipc_alloc(int size)void ipc_free (void * ptr,int size) { -if (size >PAGE_SIZE ) -vfree (ptr); +if (use_vmalloc (size)) +vfree (ptr -sizeof (struct ipc_rcu_vmalloc ));else -kfree (ptr); +kfree (ptr -sizeof (struct ipc_rcu_kmalloc )); }/* * Since RCU callback function is called in bh, * we need to defer the vfree to schedule_work */ -static void ipc_free_scheduled (void * arg) +static void ipc_schedule_free (void *arg) { -struct rcu_ipc_free *a = (struct rcu_ipc_free *)arg; -vfree (a->ptr); -kfree (a); -} +struct ipc_rcu_vmalloc *free = arg; -static void ipc_free_callback (void * arg) -{ -struct rcu_ipc_free *a = (struct rcu_ipc_free *)arg; -/* -* if data is vmalloced, then we need to delay the free -*/ -if (a->size >PAGE_SIZE ) { -INIT_WORK (&a->work, ipc_free_scheduled, arg); -schedule_work (&a->work); - }else { -kfree (a->ptr); -kfree (a); - } +INIT_WORK (&free->work, vfree, free); +schedule_work (&free->work); }void ipc_rcu_free (void * ptr,int size) { -struct rcu_ipc_free * arg; - - arg = (struct rcu_ipc_free *)kmalloc (sizeof (*arg),GFP_KERNEL ); -if (arg ==NULL ) -return ; - arg->ptr = ptr; - arg->size = size; -call_rcu (&arg->rcu_head, ipc_free_callback, arg); +if (use_vmalloc (size)) { +struct ipc_rcu_vmalloc *free; + free = ptr -sizeof (*free); +call_rcu (&free->rcu, ipc_schedule_free, 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); + } }/**
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
ipc/util.c struct kern_ipc_perm* ipc_rmid(struct ip · line 221
Quoted 6 lines · Rusty Russell
ipc/util.c +6
-0+struct ipc_rcu_kmalloc +{ +struct rcu_head rcu; +/* "void *" makes sure alignment of following data is sane. */ +void *data[0 ]; +};
Rusty, why not using gcc "aligned" keywords instead of black magic :
void *data[0];
- Davide
In message <[email protected]> y ou write:
Quoted 11 lines · Rusty Russell
ipc/util.c struct kern_ipc_perm* ipc_rmid(struct ip · line 221
Quoted 7 lines
ipc/util.c +6
-0+struct ipc_rcu_kmalloc +{ +struct rcu_head rcu; +/* "void *" makes sure alignment of following data is sane. */ +void *data[0 ]; +};Rusty, why not using gcc "aligned" keywords instead of black magic :
Quoted 1 line
void *data[0];
I think it's clearer *why* it's being done than:
struct ipc_rcu_kmalloc
{
struct rcu_head rcu;
} __attribute__((aligned(__alignof__(void *))));And simpler than:
struct ipc_rcu_kmalloc
{
struct rcu_head rcu;
/* "void *" makes sure alignment of following data is sane. */
char data[0] __attribute__((aligned(__alignof__(void *))));
};Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
Quoted 13 lines · Rusty Russell
In message <[email protected]> y ou write:
Quoted 11 lines · Rusty Russell
ipc/util.c struct kern_ipc_perm* ipc_rmid(struct ip · line 221
Quoted 7 lines
ipc/util.c +6
-0+struct ipc_rcu_kmalloc +{ +struct rcu_head rcu; +/* "void *" makes sure alignment of following data is sane. */ +void *data[0 ]; +};Rusty, why not using gcc "aligned" keywords instead of black magic :
Quoted 8 lines
Quoted 1 line
void *data[0];
I think it's clearer *why* it's being done than:
struct ipc_rcu_kmalloc { struct rcu_head rcu; } __attribute__((aligned(__alignof__(void *))));
Well, not really Rusty. The above syntax uses documented gcc features already used inside the kernel, while the fact that void *data[0]; enforces alignment it is not ( to my knowledge ) documented anywhere. You can also avoid the comment using the aligned syntax ...
- Davide
In message <[email protected]> yo u write:
Quoted 9 lines · Rusty Russell
Quoted 7 lines
I think it's clearer *why* it's being done than:
struct ipc_rcu_kmalloc { struct rcu_head rcu; } __attribute__((aligned(__alignof__(void *))));
Quoted 4 lines
Well, not really Rusty. The above syntax uses documented gcc features already used inside the kernel, while the fact that void *data[0]; enforces alignment it is not ( to my knowledge ) documented anywhere. You can also avoid the comment using the aligned syntax ...
A comment is definitely required: you must say *why* you are aligning the structure (a clearer comment would be better, of course):
/* We return a pointer after the structure to the unwitting caller: ensure safe alignment. */
The alignment of a structure member of type X is the alignment of type
X: this seems obvious to me. And "type X data[0];" is the standard way of representing a variable struct.
Have we picked all the nits yet? 8)
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
Quoted 19 lines · Rusty Russell
In message <[email protected]> yo u write:
Quoted 9 lines · Rusty Russell
Quoted 7 lines
I think it's clearer *why* it's being done than:
struct ipc_rcu_kmalloc { struct rcu_head rcu; } __attribute__((aligned(__alignof__(void *))));Quoted 4 lines
Well, not really Rusty. The above syntax uses documented gcc features already used inside the kernel, while the fact that void *data[0]; enforces alignment it is not ( to my knowledge ) documented anywhere. You can also avoid the comment using the aligned syntax ...
A comment is definitely required: you must say *why* you are aligning the structure (a clearer comment would be better, of course):
Quoted 8 lines
/* We return a pointer after the structure to the unwitting caller: ensure safe alignment. */The alignment of a structure member of type X is the alignment of type
X: this seems obvious to me. And "type X data[0];" is the standard way of representing a variable struct.Have we picked all the nits yet? 8)
Rusty, I give you two reason why you should be using the gcc documented "aligned" features :)
1) using "void *data[0];" and expecting an alignement, being undocumented, will force not obvious constraints for the gcc folks
2) I was looking at my latest copy of the ISO standard, section :
6.7.5.2 "Array declarators" - Point 1)
where it is explicitly declared that the constant expression that declared the size of the array must be greater then zero
Looking at how gcc folks is driving towards standards I will expect them screaming at you in a near future :)
- Davide