Re: synchronization model: was: Re: [PATCH printk-rework 09/14] printk: introduce a kmsg_dump iterator

From: John Ogness
Date: Wed Feb 24 2021 - 10:50:29 EST


On 2021-02-24, John Ogness <john.ogness@xxxxxxxxxxxxx> wrote:
> The @active flag is useless. It should be removed.

I would like to clarify my statement, because the @active flag _did_
protect the arch/um dumper until now. (Although it didn't actually
matter because arch/um does not have SMP or preemption support.)

In mainline we have 6 dumpers. They can be classified as follows:

1. Dumpers that provide their own synchronization to protect against
parallel or nested dump() calls.

- arch/powerpc/kernel/nvram_64.c
- fs/pstore/platform.c
- arch/um/kernel/kmsg_dump.c (after this series)

2. Dumpers that are safe because they only dump on KMSG_DUMP_PANIC,
which (currently) can never happen in parallel or nested.

- arch/powerpc/platforms/powernv/opal-kmsg.c
- drivers/hv/vmbus_drv.c

3. Dumpers that are unsafe and even @active did not provide the needed
synchronization.

- drivers/mtd/mtdoops.c

In all 6 dumpers, @action does not provide any help. That is why it can
be removed.

But I am concerned about drivers/mtd/mtdoops.c that does not have any
synchronization. Since my series is adding sychronization to
arch/um/kernel/kmsg_dump.c, I suppose it should also add it to
drivers/mtd/mtdoops.c also.

And rather than moving the useless @active from kmsg_dumper to
kmsg_dump_iter, I should just drop it.

Unless there are any objections, I will make these changes for my v3.

John Ogness