On 2021-01-27, Michael Kelley <mikel...@microsoft.com> wrote:
>> In preparation for removing printk's @logbuf_lock, dumpers that have
>> assumed to be protected against parallel calls must provide their own
>> synchronization. Add a locally static spinlock to synchronize the
>> kmsg_dump call.
>>
>> Signed-off-by: John Ogness <john.ogn...@linutronix.de>
>> ---
>> drivers/hv/vmbus_drv.c | 12 ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
>> index fbeddef90941..08db95e1a139 100644
>> --- a/drivers/hv/vmbus_drv.c
>> +++ b/drivers/hv/vmbus_drv.c
>> @@ -1362,23 +1362,27 @@ static void hv_kmsg_dump(struct kmsg_dumper *dumper,
>> enum kmsg_dump_reason reason,
>> struct kmsg_dumper_iter *iter)
>> {
>> + static DEFINE_SPINLOCK(lock);
>> size_t bytes_written;
>> phys_addr_t panic_pa;
>> + unsigned long flags;
>>
>> /* We are only interested in panics. */
>> if ((reason != KMSG_DUMP_PANIC) || (!sysctl_record_panic_msg))
>> return;
>>
>> + if (!spin_trylock_irqsave(&lock, flags))
>> + return;
>> +
>> panic_pa = virt_to_phys(hv_panic_page);
>>
>> - /*
>> - * Write dump contents to the page. No need to synchronize; panic should
>> - * be single-threaded.
>> - */
>> + /* Write dump contents to the page. */
>> kmsg_dump_get_buffer(iter, false, hv_panic_page, HV_HYP_PAGE_SIZE,
>> &bytes_written);
>> if (bytes_written)
>> hyperv_report_panic_msg(panic_pa, bytes_written);
>> +
>> + spin_unlock_irqrestore(&lock, flags);
>> }
>>
>> static struct kmsg_dumper hv_kmsg_dumper = {
>> --
>> 2.20.1
>
> I don't think this patch is needed. Per some of the unmodified code
> above, and the comment, we proceed to the call to kmsg_dump_get_buffer()
> only in the panic path. Code in the panic path prior to invoking kmsg_dump()
> ensures that we are single-threaded on a single CPU. So I think everything
> is good here without the new spin lock. The subsequent call to
> hyperv_report_panic_msg() also assumes that we are single-threaded.
You are correct. I will withdraw this patch from the series. Thanks for
the feedback.
John Ogness
