All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Aleksey Makarov <aleksey.makarov@linaro.org>
Cc: Petr Mladek <pmladek@suse.com>,
	linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
	Sudeep Holla <sudeep.holla@arm.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Peter Hurley <peter@hurleysoftware.com>,
	Jiri Slaby <jslaby@suse.com>, Robin Murphy <robin.murphy@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	"Nair, Jayachandran" <Jayachandran.Nair@cavium.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Subject: Re: [PATCH v8 3/3] printk: fix double printing with earlycon
Date: Tue, 28 Mar 2017 11:04:04 +0900	[thread overview]
Message-ID: <20170328020404.GA10573@jagdpanzerIV.localdomain> (raw)
In-Reply-To: <4b561f81-67af-f6a3-76c9-d0d8499c52bd@linaro.org>

On (03/27/17 19:28), Aleksey Makarov wrote:
[..]
> > > +			/*
> > > +			 * Maintain an invariant that will help to find if
> > > +			 * the matching console is preferred, see
> > > +			 * register_console():
> > > +			 *
> > > +			 * The last non-braille console is always
> > > +			 * the preferred one.
> > > +			 */
> > > +			for (last = MAX_CMDLINECONSOLES - 1;
> > > +			     last >= 0 && !console_cmdline[last].name[0];
> > > +			     last--)
> > > +				;
> > 
> > This is a rather non-trivial code to find the last element.
> > I might make sense to count it in a global variable.
> > Then we might remove the check for console_cmdline[i].name[0]
> > also in the other for cycles and make them better readable.
> 
> Having an additional variable console_cmdline_last pointing to the last element
> would require maintaining consistency between this variable and
> contents of console_cmdline.  For the code we have it is not hard, but when code
> is changed we need to check this.  Also there exists preferred_console that
> has almost the same meaning but it points not to the last element, but to the
> last non-braille element.  Also we need to have a special value (-1) for this
> variable for empty array. So I personally would instead try to rewrite this:
> 
> 	for (last = MAX_CMDLINECONSOLES - 1; last >= 0; last--)
> 		if (console_cmdline[last].name[0])
> 			break;
> 
> Is it better?  If not, I will send a version with console_cmdline_last.

personally I'm fine with the nested loop. the latest version
	"for (last = MAX_CMDLINECONSOLES - 1; last >= 0;..."

is even easier to read.


so we do not just iterate console_cmdline anymore, but also modify it.
this, probably, has impact on the following scenario

CPU0						CPU1

add_preferred_console()			add_preferred_console()
  __add_preferred_console()		  __add_preferred_console()
      swap(i1, last)			    swap(i2, last)

	temp1 = i1
	i1 = last				temp2 = i2
	last = temp1				i2 = last
						last = temp2

so both i1 and i2 will point to 'last' now, IOW, we will have two
identical entries in console_cmdline, while i1 or i2 will be lost.


neither add_preferred_console() nor __add_preferred_console() have any
serialization. and I assume that we can call add_preferred_console()
concurrently, can't we?

	-ss

  reply	other threads:[~2017-03-28  2:04 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-15 10:28 [PATCH v5 0/3] printk: fix double printing with earlycon Aleksey Makarov
2017-03-15 10:28 ` [PATCH v5 1/3] printk: fix name/type/scope of preferred_console var Aleksey Makarov
2017-03-15 10:28 ` [PATCH v5 2/3] printk: rename selected_console -> preferred_console Aleksey Makarov
2017-03-15 10:28 ` [PATCH v5 3/3] printk: fix double printing with earlycon Aleksey Makarov
2017-03-15 16:58   ` Petr Mladek
2017-03-16  7:30     ` Sergey Senozhatsky
2017-03-16 10:36     ` Aleksey Makarov
2017-03-16 13:54       ` Petr Mladek
2017-03-17 10:32         ` Aleksey Makarov
2017-03-17 11:43 ` [PATCH v6 " Aleksey Makarov
2017-03-17 13:34   ` Aleksey Makarov
2017-03-17 13:43 ` [PATCH v7 " Aleksey Makarov
2017-03-20  6:16   ` Sergey Senozhatsky
2017-03-20 10:03 ` [PATCH v8 " Aleksey Makarov
2017-03-27 14:14   ` Petr Mladek
2017-03-27 16:28     ` Aleksey Makarov
2017-03-28  2:04       ` Sergey Senozhatsky [this message]
2017-03-28 12:56         ` Petr Mladek
2017-03-30  5:55           ` Sergey Senozhatsky
2017-04-04 11:12             ` Petr Mladek
2017-04-05 18:26               ` Aleksey Makarov
2017-04-05 20:20 ` [PATCH v9 " Aleksey Makarov
2017-04-05 21:57   ` Andy Shevchenko
2017-04-06  4:44     ` Aleksey Makarov
2017-04-10 14:22   ` Petr Mladek
2017-04-10 18:00     ` Aleksey Makarov
2017-04-11  1:54       ` Sergey Senozhatsky
2017-04-11  7:43       ` Petr Mladek
2017-04-12  6:24         ` Aleksey Makarov
2017-05-09  8:29   ` Sabrina Dubroca
2017-05-11  8:24     ` Sergey Senozhatsky
2017-05-11  8:41       ` Sergey Senozhatsky
2017-05-11 11:32         ` Sergey Senozhatsky
2017-05-11 21:17           ` Aleksey Makarov
2017-05-12  1:11             ` Sergey Senozhatsky
2017-05-11 21:13         ` Aleksey Makarov
2017-05-12 12:57         ` Petr Mladek
2017-05-12 13:46           ` Petr Mladek
2017-05-14 21:01             ` Aleksey Makarov
2017-05-13 11:48           ` Sergey Senozhatsky
2017-05-14 20:37           ` Aleksey Makarov
2017-05-18 15:49             ` Petr Mladek
2017-05-26  9:37               ` Aleksey Makarov
2017-06-01 12:03                 ` Petr Mladek
2017-06-06 14:31                   ` Petr Mladek
2017-06-06 16:03                     ` Petr Mladek
2017-06-07  9:13                       ` 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=20170328020404.GA10573@jagdpanzerIV.localdomain \
    --to=sergey.senozhatsky.work@gmail.com \
    --cc=Jayachandran.Nair@cavium.com \
    --cc=aleksey.makarov@linaro.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jslaby@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=peter@hurleysoftware.com \
    --cc=pmladek@suse.com \
    --cc=robin.murphy@arm.com \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=sudeep.holla@arm.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.