Messages in this thread | | | From | John Ogness <> | Subject | Re: [PATCH next v2 2/3] printk: change @clear_seq to atomic64_t | Date | Sun, 06 Dec 2020 21:29:59 +0106 |
| |
On 2020-12-04, Petr Mladek <pmladek@suse.com> wrote: > On Tue 2020-12-01 21:59:40, John Ogness wrote: >> Currently @clear_seq access is protected by @logbuf_lock. Once >> @logbuf_lock is removed some other form of synchronization will be >> required. Change the type of @clear_seq to atomic64_t to provide the >> synchronization. >> >> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c >> index fc5e3a7d6d89..e9018c4e1b66 100644 >> --- a/kernel/printk/printk.c >> +++ b/kernel/printk/printk.c >> @@ -3412,7 +3418,7 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_buffer); >> */ >> void kmsg_dump_rewind_nolock(struct kmsg_dumper *dumper) >> { >> - dumper->cur_seq = clear_seq; >> + dumper->cur_seq = atomic64_read(&clear_seq); > > Sigh, atomic64_read() uses a spin lock in the generic implementation > that is used on some architectures. > > Hmm, this seems to be the only location where the lock must not be > used.
Yes, and it is read-only access. Perhaps atomic64_t is the wrong thing to use here. We could use a seqcount_latch and a shadow variable so that if a writer has been preempted, we can use the previous value. (Only kmsg_dump would need to use the lockless variant to read the value.)
void clear_seq_set(u64 val) { spin_lock_irq(&clear_lock); raw_write_seqcount_latch(&clear_latch); clear_seq[0] = val; raw_write_seqcount_latch(&clear_latch); clear_seq[1] = val; spin_unlock_irq(&clear_lock); }
u64 clear_seq_get_nolock(void) { unsigned int seq, idx; u64 val;
do { seq = raw_read_seqcount_latch(&clear_latch); idx = seq & 0x1; val = clear_seq[idx]; } while (read_seqcount_latch_retry(&clear_latch, seq));
return val; }
u64 clear_seq_get(void) { u64 val; spin_lock_irq(&clear_lock); val = clear_seq[0]; spin_unlock_irq(&clear_lock); return val; }
> Alternative solution would to always fallback to the first_seq on > these architectures. Few people would complain when they see more > messages. We could always improve it when it causes problems.
I am also OK with this solution.
John Ogness
| |