Linux-Serial Archive on lore.kernel.org
 help / color / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	linux-kernel@vger.kernel.org,
	Steven Rostedt <rostedt@goodmis.org>,
	Daniel Wang <wonderfly@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Alan Cox <gnomes@lxorguk.ukuu.org.uk>,
	Jiri Slaby <jslaby@suse.com>, Peter Feiner <pfeiner@google.com>,
	linux-serial@vger.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	john.ogness@linutronix.de
Subject: Re: [RFC][PATCHv2 2/4] printk: move printk_safe macros to printk header
Date: Wed, 17 Oct 2018 16:00:44 +0200
Message-ID: <20181017140044.GK3121@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20181017105015.udzegzfh7cxgatso@pathway.suse.cz>

On Wed, Oct 17, 2018 at 12:50:15PM +0200, Petr Mladek wrote:

> > If you have a lockless buffer and a trailing printk thread, that's
> > deferred.
> > 
> > And earlycon _should_ be lockless (yes, I know, some suck)
> > 
> > But if you do this deferred nonsense that's currently in printk, then
> > you risk never seeing the message -- the same problem I have with the
> > whole printk_safe thing too.
> 
> What do you mean with printing the message, please? Storing it
> into a buffer or showing on console?
> 
> I guess that it is storing because you suggest handling
> the console by a printk kthread.

I'm not sure from the context you quoted.

> Now, please, what is your vision of the lock less buffer?
> Would it be similar with the one used by ftrace?

No, exactly not like ftrace in fact.

> The current implementation of the lockless buffer is very tricky
> and supports only one reader. It will be even bigger maze
> of code when we add such a support.

The ftrace thing is way complicated and unsuitable. It does what it
does, but I would've never build it that way around.

> IMHO, the current printk buffer is much more easy from
> this point of view. It is one global buffer plus
> two per-cpu buffers that are mostly empty.

I'm arguing for a single (global) lockless buffer with per-cpu line
buffers.

> I still think that the buffer is the minor problem these
> days. The big problem are consoles and I do not see
> how any lockless buffer could help with it.

Yes, the consoles are horrible unfixable crap (some far worse than
others).

But the logbuf is also horrible, and that we can in fact fix.

> Also note that by deferred printk I mean deferring the console
> handling! IMHO, there are _no more problems_ with storing
> the messages into the buffer if we accept that the current
> very limited use of printk_safe per-cpu buffers is easier
> than any complicated generic lockless buffer.

They hide messages. The whole two radically different printk paths is
also quite horrible.

And lockless buffers aren't all _that_ complicated, esp. not when
performance isn't the top priority.

And earlycon is mostly usable, esp. the serial ones. Those, when
configured, should synchronously print along. The current design
also makes that difficult.

A wee little like so; typed in a hurry, never been near a compiler.

struct ll_buffer {
	void *buffer;
	unsigned int size;

	unsigned int tail, reserve, head;

	unsigned int cpu;
	unsigned int ctx;
};

struct ll_entry {
	unsigned int size;
	char data[0];
};

struct ll_handle {
	unsigned int cpu;
	struct ll_entry *lhe;
};

static struct ll_buffer logbuf = {
	.buffer = /* static storage */,
	.cpu = -1;
};

static void __ll_get(struct ll_buffer *llb, struct ll_handle *llh)
{
	unsigned int cpu, old;

	cpu = smp_processor_id();
	for (;;) {
		old = READ_ONCE(llb->cpu);
		if (old == -1) {
			if (try_cmpxchg_acquire(&llb->cpu, &old, cpu))
				break;
		}
		if (old == cpu)
			break;

		cpu_relax();
	}

	llh->cpu = old;
}

static void __ll_put(struct ll_buffer *llb, struct ll_handle *llh)
{
	smp_store_release(llb->cpu, llh->cpu);
}

static char *__ll_reserve(struct ll_buffer *llb, struct ll_hande *llh, unsigned int size)
{
	unsigned int tail, res;
	struct ll_entry *lle;

	__ll_get(llb, llh);

	llb->ctx++;

	size += sizeof(struct ll_entry);
	size += 3;
	size &= ~3;

	do {
		for (;;) {
			tail = READ_ONCE(llb->tail);
			res = READ_ONCE(llb->resserve);

			if (CIRC_SPACE(tail, res, llb->size) > size)
				break;

			lle = llb->buffer + (tail % llb->size);
			cmpxchg(&llb->tail, tail, tail + lhe->size);
		}

	} while (cmpxchg(&llb->reserve, res, res + size) != res);

	llh->lhe = llb->buffer + (res % llb->size);
	llh->lhe->size = size;
}

static void __ll_commit(struct ll_buffer *llb, struct ll_handle *llh)
{
	for (;;) {
		unsigned int ctx, res;

		res = READ_ONCE(llb->res);
		barrier();
		ctx = --llb->ctx;
		if (ctx)
			break;

		smp_store_release(llb->head, res);

		if (likely(READ_ONCE(llb->res) == res))
			break;

		llb->ctx++;
	}

	__ll_put(llb, llh);
}

#define MAX_LINE	1020
#define MAX_CTX		4

struct line_buffers {
	int ctx;
	char buffers[MAX_CTX][MAX_LINE];
};

static DEFINE_PER_CPU(struct line_buffers, linbufs);

int vprintk(const char *fmt, va_list args)
{
	struct ll_handle handle;
	struct line_buffers *lb;
	struct ll_entry *lhe;
	char linbuf;
	int sz;

	preempt_disable();
	lb = this_cpu_ptr(&linbufs);
	linbuf = lb->buffers[lb->ctx++];

	sz = scnprintf(linbuf, MAX_LINE, fmt, args);

	__ll_reserve(&logbuf, &handle, sz);
	memcpy(llh->lhe->data, linbuf, sz); // XXX doesn't wrap on llb->size
	__ll_commit(&logbuf, &handle);

	ln->ctx--;
	preempt_enable();
}


struct ll_reader {
	unsigned int tail;
	char line[MAX_LINE];
};

static bool __ll_read(struct ll_buffer *llb, struct ll_reader *llr)
{
	unsigned int tail, reader, head, sz;
	struct ll_entry *lhe;
	bool lost = false;

	for (;;) {
		head = READ_ONCE(llb->head);
		smp_rmb();
		tail = READ_ONCE(llb->tail);
		reader = READ_ONCE(llr->tail);
		if (unlikely((signed)(tail - reader) < 0)) {
			lost = true;
			reader = tail;
		}
		smp_rmb();
		lhe = llb->buffer + reader;
		sz = lhe->size;
		llr->tail = reader + sz;
		sz -= sizeof(struct ll_entry);

		if ((signed)(head - reader) > 0)
			memcpy(llr->line, lhe->data, sz); // XXX wrap

		smp_rmb();
		tail = READ_ONCE(llb->tail);
		if ((signed)(tail - reader) >= 0)
			break;
	}

	return lost;
}

  reply index

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-16  5:04 [RFC][PATCHv2 0/4] less deadlock prone serial consoles Sergey Senozhatsky
2018-10-16  5:04 ` [RFC][PATCHv2 1/4] panic: avoid deadlocks in re-entrant console drivers Sergey Senozhatsky
2018-10-17  4:48   ` Sergey Senozhatsky
2018-10-23 11:07   ` Petr Mladek
2018-10-23 11:54     ` Sergey Senozhatsky
2018-10-23 12:04       ` Sergey Senozhatsky
2018-10-23 12:12         ` Sergey Senozhatsky
2018-10-25  9:06           ` Petr Mladek
2018-10-25  9:31             ` Sergey Senozhatsky
2018-10-25  8:29       ` Petr Mladek
2018-10-25  9:05         ` Sergey Senozhatsky
2018-10-25 10:10   ` [PATCHv3] " Sergey Senozhatsky
2018-10-25 10:51     ` kbuild test robot
2018-10-25 11:56       ` Sergey Senozhatsky
2018-10-31 12:27     ` Petr Mladek
2018-11-01  1:48       ` Sergey Senozhatsky
2018-11-01  8:08         ` Petr Mladek
2018-11-22 13:12           ` Petr Mladek
2018-12-12  0:53             ` Daniel Wang
2018-12-12  5:23               ` Sergey Senozhatsky
2018-12-12  5:59                 ` Daniel Wang
2018-12-12  6:06                   ` Sergey Senozhatsky
2018-12-12  6:09                     ` Daniel Wang
2018-10-16  5:04 ` [RFC][PATCHv2 2/4] printk: move printk_safe macros to printk header Sergey Senozhatsky
2018-10-16  7:27   ` Peter Zijlstra
2018-10-16 11:40     ` Petr Mladek
2018-10-16 12:17       ` Peter Zijlstra
2018-10-17 10:50         ` Petr Mladek
2018-10-17 14:00           ` Peter Zijlstra [this message]
2018-10-22 14:30             ` Petr Mladek
2018-10-16 12:27     ` Sergey Senozhatsky
2018-10-16 12:38       ` Peter Zijlstra
2018-10-16 12:54       ` Peter Zijlstra
2018-10-16 14:21         ` Peter Zijlstra
2018-10-17  4:32         ` Sergey Senozhatsky
2018-10-17  7:57           ` Peter Zijlstra
2018-10-17 13:36             ` Sergey Senozhatsky
2018-10-23  6:25         ` Sergey Senozhatsky
2018-10-16  5:04 ` [RFC][PATCHv2 3/4] serial: introduce uart_port locking helpers Sergey Senozhatsky
2018-12-08  3:12   ` Sergey Senozhatsky
2018-12-12 11:08     ` Greg Kroah-Hartman
2018-10-16  5:04 ` [RFC][PATCHv2 4/4] tty: 8250: switch to " Sergey Senozhatsky
2018-10-16  7:23 ` [RFC][PATCHv2 0/4] less deadlock prone serial consoles Peter Zijlstra
2018-10-16  8:12   ` Sergey Senozhatsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181017140044.GK3121@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=gnomes@lxorguk.ukuu.org.uk \
    --cc=gregkh@linuxfoundation.org \
    --cc=john.ogness@linutronix.de \
    --cc=jslaby@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=pfeiner@google.com \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=torvalds@linux-foundation.org \
    --cc=wonderfly@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Serial Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-serial/0 linux-serial/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-serial linux-serial/ https://lore.kernel.org/linux-serial \
		linux-serial@vger.kernel.org
	public-inbox-index linux-serial

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-serial


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git