All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Peter Oberparleiter <oberpar@linux.ibm.com>
Subject: Re: [PATCH] s390/fault: use wake_up_klogd() in bust_spinlocks()
Date: Thu, 25 Oct 2018 15:48:04 +0900	[thread overview]
Message-ID: <20181025064804.GA20702@jagdpanzerIV> (raw)
In-Reply-To: <20181025062800.GB4037@osiris>

On (10/25/18 08:28), Heiko Carstens wrote:
[..]
> >  		int loglevel_save = console_loglevel;
> > -		console_unblank();
> > -		oops_in_progress = 0;
> > -		/*
> > -		 * OK, the message is on the console.  Now we call printk()
> > -		 * without oops_in_progress set so that printk will give klogd
> > -		 * a poke.  Hold onto your hats...
> > -		 */
> > +
> >  		console_loglevel = 15;
> > -		printk(" ");
> > +		console_unblank();
> >  		console_loglevel = loglevel_save;
> > +		oops_in_progress = 0;
> > +		wake_up_klogd();
> >  	}
> 
> With your patch this looks nearly like the common code variant. I did
> some code archaeology and this function is unchanged since ~17 years.
> When it was introduced it was close to identical to the x86 variant.
> All other architectures use the common code variant in the
> meantime. So if we change this I'd prefer that we switch s390 to the
> common code variant as well.

Right. I couldn't clearly understand what was so special that s390
bust_spinlock() was doing, but assumed that this `console_loglevel'
manipulation probably was somehow important to s390. Though this
console_loglevel adjustment is not 100% guaranteed to make any difference,
because of the way console_unblank() works: if it can't lock console_sem
and it sees oops_in_progress then it does nothing; it doesn't even print
logbuf messages to the consoles. If, however, console_sem is not locked,
then it does print pending logbuf messages, with temporarily verbose
console_loglevel. I concluded that this might be important to you in
one way or another.

> Right now I can't see a reason for not doing that, but I might be
> wrong of course. So, could you please provide a new version which just
> removes this variant and makes s390 use the generic one too.
> 
> We'll see if there is any fallout...

Will do! Sounds good.

	-ss

  reply	other threads:[~2018-10-25  6:48 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-24  4:30 [PATCH] s390/fault: use wake_up_klogd() in bust_spinlocks() Sergey Senozhatsky
2018-10-24  4:34 ` Sergey Senozhatsky
2018-10-25  6:28   ` Heiko Carstens
2018-10-25  6:48     ` Sergey Senozhatsky [this message]
2018-10-25  7:05     ` Sergey Senozhatsky
2018-10-25  8:11       ` Heiko Carstens
2018-10-25  8:28         ` 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=20181025064804.GA20702@jagdpanzerIV \
    --to=sergey.senozhatsky.work@gmail.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=oberpar@linux.ibm.com \
    --cc=schwidefsky@de.ibm.com \
    --cc=sergey.senozhatsky@gmail.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.