Re: [PATCH] Allocate module.ref array dynamically

[Rusty Russell <rusty@rustcorp.com.au>]

On Thursday 13 November 2008 06:41:32 Christoph Lameter wrote:
> On Wed, 12 Nov 2008, Rusty Russell wrote:
> > You've introduced a third set of per-cpu primitives, yet the second set
> > still has 0 users.
>
> What second set?

percpu_alloc().

> > Your new basic interface is:
> > CPU_ALLOC/CPU_FREE/CPU_PTR/THIS_CPU/__THIS_CPU
> >
> > I don't think the CAPS adds anything.  I'd like to see standard docbook
> > comments.  It's not clear from your documentation whether this allocates
> > for all possible or only all online CPUs, and the difference between
> > THIS_CPU and __THIS_CPU is not immediately obvious.
>
> The allocation is for all allocated percpu areas which is now per possible
> cpu. The difference between THIS_CPU and __THIS_CPU is the context check
> by smp_processor_id()

Thanks, I figured that out, but it's not documented.  I'll happily create a 
patch, but see below.  BTW, did you end up using __THIS_CPU anywhere?

> > How about re-using alloc_percpu/free_percpu/per_cpu_ptr APIs?  Rename
> > THIS_CPU to __get_cpu_ptr and implement get_cpu_ptr and put_cpu_ptr
> > wrappers (a-la get_cpu_var).
>
> The caps functions are macros not functions. Macros are
> capitalized.

Traditionally, yes, but we seem to be more ambivalent in the kernel.  
list_for_each() et. al spring to mind: code would be uglier if we made them 
caps.

> alloc_percpu must stay until the last remains have been
> evicted.

OK, you anticipate replacement of all alloc_percpu users?

> And the API does work differently.

I'm afraid the difference has escaped me so far.  Please explain why you can't 
use the current API.  Is this subtle difference going to make the transition 
difficult?

Thanks,
Rusty.