Messages in this thread | | | Date | Fri, 11 Feb 2011 17:27:42 -0500 | From | Jason Baron <> | Subject | Re: [PATCH 0/2] jump label: 2.6.38 updates |
| |
On Fri, Feb 11, 2011 at 05:20:25PM -0500, Mathieu Desnoyers wrote: > * Peter Zijlstra (peterz@infradead.org) wrote: > > On Fri, 2011-02-11 at 16:13 -0500, Mathieu Desnoyers wrote: > > > > > > Thoughts ? > > > > #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL) > > + > > +struct jump_label_key { > > + void *ptr; > > +}; > > > > struct jump_label_entry { > > struct hlist_node hlist; > > struct jump_entry *table; > > - int nr_entries; > > /* hang modules off here */ > > struct hlist_head modules; > > unsigned long key; > > + u32 nr_entries; > > + int refcount; > > }; > > > > #else > > > > +struct jump_label_key { > > + int state; > > +}; > > > > #endif > > > > So why can't we make that jump_label_entry::refcount and > > jump_label_key::state an atomic_t and be done with it? > > > > Then the enabled case uses if (atomic_inc_return(&key->ptr->refcount) == > > 1), and the disabled atomic_inc(&key->state). > > > > OK, by "enabled" you mean #if defined(CC_HAVE_ASM_GOTO) && > defined(CONFIG_JUMP_LABEL), and "disabled", the #else. > > I guess the only downside is the extra volatile for the atomic_read for > the fallback case, which is not really much of problem realistically > speaking: anyway, the volatile is a good thing to have in the fallback > case to force the compiler to re-read the variable. Let's go with your > idea. > > Thanks, > > Mathieu >
ok, I'll try and re-spin the interface based around atomic_t, if we are all agreed...there was also a circular dependency issue with atomic.h including kernel.h which included jump_label.h, and that was why we had a separate, jump_label_ref.h header file, but hopefully I can be resolve that in a clean way.
thanks,
-Jason
| |