Messages in this thread Patch in this message | | | Date | Fri, 19 Sep 2008 10:31:37 +0800 | From | Lai Jiangshan <> | Subject | Re: [RFC PATCH] rcu: introduce kfree_rcu() |
| |
Andrew Morton wrote:
>> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h >> index e8b4039..04c654f 100644 >> --- a/include/linux/rcupdate.h >> +++ b/include/linux/rcupdate.h >> @@ -253,4 +253,25 @@ extern void rcu_barrier_sched(void); >> extern void rcu_init(void); >> extern int rcu_needs_cpu(int cpu); >> >> +#define __KFREE_RCU_MAX_OFFSET 4095 >> +#define KFREE_RCU_MAX_OFFSET (sizeof(void *) * __KFREE_RCU_MAX_OFFSET) >> + >> +#define __rcu_reclaim(head) \ >> +do { \ >> + unsigned long __offset = (unsigned long)head->func; \ >> + if (__offset <= __KFREE_RCU_MAX_OFFSET) \ >> + kfree((void *)head - sizeof(void *) * __offset); \ >> + else \ >> + head->func(head); \ >> +} while(0) > > All the above could do with some comments explaining what it does. > > Please use checkpatch. > > Surely it cannot be resirable to have a "function" which can call kfree > either synchronously or asynchronously depending upon the size of the > object which was passed to it? If the caller wants rcu treatment of > the freeing then that is what the caller must be given. I mean, if the > caller can tolerate a synchrnous call to kfree() then the caller should > have directly called kfree? > > I think (and pray) that the above could have been implemented as an > out-of-line C function? >
__rcu_reclaim(head) is called when @head 's grace period had completed.
__rcu_reclaim(head) uses "if (__offset <= __KFREE_RCU_MAX_OFFSET)" to check whether @head is queued by kfree_rcu() or normal call_rcu().
if @head is queued by kfree_rcu(), ((void *)head - sizeof(void *) * __offset) is the pointer that the memory chunk need to be freed.
otherwise call head->func(head) as original code.
__rcu_reclaim has __prefix, will not be used by user, it is a helper for rcu-machine. it is always called asynchronously. sorry for not comments for it.
>> + >> +/** >> + * kfree_rcu - free previously allocated memory after a grace period. >> + * @ptr: pointer returned by kmalloc. >> + * @head: structure to be used for queueing the RCU updates. This structure >> + * is a part of previously allocated memory @ptr. >> + */ >> +extern void kfree_rcu(const void *ptr, struct rcu_head *head); > > rcu kerneldoc is implemented in the .c file, not in the .h file.
Good, Thanks.
> >> #endif /* __LINUX_RCUPDATE_H */ >> diff --git a/kernel/rcuclassic.c b/kernel/rcuclassic.c >> index aad93cd..5a14190 100644 >> --- a/kernel/rcuclassic.c >> +++ b/kernel/rcuclassic.c >> @@ -232,7 +232,7 @@ static void rcu_do_batch(struct rcu_data *rdp) >> while (list) { >> next = list->next; >> prefetch(next); >> - list->func(list); >> + __rcu_reclaim(list); >> list = next; >> if (++count >= rdp->blimit) >> break; >> diff --git a/kernel/rcupdate.c b/kernel/rcupdate.c >> index 467d594..aa9b56a 100644 >> --- a/kernel/rcupdate.c >> +++ b/kernel/rcupdate.c >> @@ -162,6 +162,18 @@ void rcu_barrier_sched(void) >> } >> EXPORT_SYMBOL_GPL(rcu_barrier_sched); >> >> +void kfree_rcu(const void *ptr, struct rcu_head *head) >> +{ >> + unsigned long offset; >> + typedef void (*rcu_callback)(struct rcu_head *); >> + >> + offset = (void *)head - (void *)ptr; >> + BUG_ON(offset > KFREE_RCU_MAX_OFFSET); >> + >> + call_rcu(head, (rcu_callback)(offset / sizeof(void *))); > > How can this work? We take the difference between two pointers, divide > that by 4 or 8, then treat the resulting number as the address of an > RCU callback function. > > I think I'm missing something here.
We will use __rcu_reclaim(head) sort it out when it's grace period has completed.
> >> +} >> +EXPORT_SYMBOL_GPL(kfree_rcu); >> + >> void __init rcu_init(void) >> { >> __rcu_init(); >> diff --git a/kernel/rcupreempt.c b/kernel/rcupreempt.c >> index 2782793..62a9e54 100644 >> --- a/kernel/rcupreempt.c >> +++ b/kernel/rcupreempt.c >> @@ -1108,7 +1108,7 @@ static void rcu_process_callbacks(struct softirq_action *unused) >> spin_unlock_irqrestore(&rdp->lock, flags); >> while (list) { >> next = list->next; >> - list->func(list); >> + __rcu_reclaim(list); >> list = next; >> RCU_TRACE_ME(rcupreempt_trace_invoke); >> } > > >
| |