Messages in this thread | | | Date | Fri, 16 Oct 2009 12:10:45 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH v10 4/5] core: Add kernel message dumper to call on oopses and panics |
| |
* Simon Kagstrom <simon.kagstrom@netinsight.net> wrote:
> +int kmsg_dump_register(struct kmsg_dumper *dumper) > +{ > + unsigned long flags; > + > + /* The dump callback needs to be set */ > + if (!dumper->dump) > + return -EINVAL; > + > + spin_lock_irqsave(&dump_list_lock, flags); > + > + /* Don't allow registering multiple times */ > + if (dumper->registered) { > + spin_unlock_irqrestore(&dump_list_lock, flags); > + > + return -EBUSY; > + } > + > + dumper->registered = 1; > + list_add(&dumper->list, &dump_list); > + spin_unlock_irqrestore(&dump_list_lock, flags); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(kmsg_dump_register);
i dont want to bikeshed paint this but this sequence caught my eyes. We generally do flatter and clearer locking sequences:
int kmsg_dump_register(struct kmsg_dumper *dumper) { unsigned long flags; int err = -EBUSY;
/* The dump callback needs to be set */ if (!dumper->dump) return -EINVAL;
spin_lock_irqsave(&dump_list_lock, flags);
/* Don't allow registering multiple times */ if (!dumper->registered) { dumper->registered = 1; list_add_tail(&dumper->list, &dump_list); err = 0; }
spin_unlock_irqrestore(&dump_list_lock, flags);
return err; } EXPORT_SYMBOL_GPL(kmsg_dump_register);
(warning: untested, written in mail editor)
Same goes for kmsg_dump_unregister().
Ingo
| |