On Wed, 2019-04-24 at 16:36 +0200, John Ogness wrote:
> @@ -2830,16 +2829,18 @@ void kmsg_dump(enum kmsg_dump_reason reason)
> if (dumper->max_reason && reason > dumper->max_reason)
> continue;
>
> - /* initialize iterator with data about the stored records */
> - dumper->active = true;
> + /*
> + * use a local copy to avoid modifying the
> + * iterator used by any other cpus/contexts
> + */
> + memcpy(&dumper_local, dumper, sizeof(dumper_local));
>
> - kmsg_dump_rewind(dumper);
> + /* initialize iterator with data about the stored records */
> + dumper_local.active = true;
> + kmsg_dump_rewind(&dumper_local);
>
> /* invoke dumper which will iterate over records */
> - dumper->dump(dumper, reason);
> -
> - /* reset iterator */
> - dumper->active = false;
> + dumper_local.dump(&dumper_local, reason);
> }
When would a dumper (or anything else that checks it) ever see active be
false?
> rcu_read_unlock();
> }
> @@ -2951,9 +2952,7 @@ bool kmsg_dump_get_line(struct kmsg_dumper *dumper,
> bool syslog,
> {
> bool ret;
>
> - mutex_lock(&kmsg_dump_lock);
> ret = kmsg_dump_get_line_nolock(dumper, syslog, line, size, len);
> - mutex_unlock(&kmsg_dump_lock);
>
> return ret;
> }
> @@ -3105,9 +3104,7 @@ void kmsg_dump_rewind_nolock(struct kmsg_dumper
> *dumper)
> */
> void kmsg_dump_rewind(struct kmsg_dumper *dumper)
> {
> - mutex_lock(&kmsg_dump_lock);
> kmsg_dump_rewind_nolock(dumper);
> - mutex_unlock(&kmsg_dump_lock);
> }
> EXPORT_SYMBOL_GPL(kmsg_dump_rewind);
>
Any reason not to get rid of the wrappers now that the lock's gone?
-Scott
