linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Waiman Long <longman@redhat.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>, Will Deacon <will.deacon@arm.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com,
	linux-mm@kvack.org, iommu@lists.linux-foundation.org,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Tejun Heo <tj@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v2 07/17] debugobjects: Move printk out of db lock critical sections
Date: Fri, 23 Nov 2018 12:36:48 +0100	[thread overview]
Message-ID: <20181123113648.6fieltetklnoex6v@pathway.suse.cz> (raw)
In-Reply-To: <fd006ace-f339-34c2-d87f-51f145ac8364@redhat.com>

On Thu 2018-11-22 15:29:35, Waiman Long wrote:
> On 11/22/2018 11:02 AM, Petr Mladek wrote:
> > Anyway, I wonder what was the primary motivation for this patch.
> > Was it the system hang? Or was it lockdep report about nesting
> > two terminal locks: db->lock, pool_lock with logbuf_lock?
> 
> The primary motivation was to make sure that printk() won't be called
> while holding either db->lock or pool_lock in the debugobjects code. In
> the determination of which locks can be made terminal, I focused on
> local spinlocks that won't cause boundary to an unrelated subsystem as
> it will greatly complicate the analysis.

Then please mention this as the real reason in the commit message.

The reason that printk might take too long in IRQ context does
not explain why we need the patch. printk() is called in IRQ
context in many other locations. I do not see why this place
should be special. The delay is the price for the chance to
see the problem.


> I didn't realize that it fixed a hang problem that I was seeing until I
> did bisection to find out that it was caused by the patch that cause the
> debugobjects splat in the first place a few days ago. I was comparing
> the performance status of the pre and post patched kernel. The pre-patch
> kernel failed to boot in the one of my test systems, but the
> post-patched kernel could. I narrowed it down to this patch as the fix
> for the hang problem.

The hang is a mystery. My guess is that there is a race somewhere.
The patch might have reduced the race window but it probably did
not fix the race itself.

Best Regards,
Petr

  reply	other threads:[~2018-11-23 11:36 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-19 18:55 [PATCH v2 00/17] locking/lockdep: Add a new class of terminal locks Waiman Long
2018-11-19 18:55 ` [PATCH v2 01/17] locking/lockdep: Remove version from lock_class structure Waiman Long
2018-12-11 15:22   ` [tip:locking/core] locking/lockdep: Remove ::version " tip-bot for Waiman Long
2018-11-19 18:55 ` [PATCH v2 02/17] locking/lockdep: Rework lockdep_set_novalidate_class() Waiman Long
2018-11-19 18:55 ` [PATCH v2 03/17] locking/lockdep: Add a new terminal lock type Waiman Long
2018-12-07  9:19   ` Peter Zijlstra
2018-11-19 18:55 ` [PATCH v2 04/17] locking/lockdep: Add DEFINE_TERMINAL_SPINLOCK() and related macros Waiman Long
2018-11-19 18:55 ` [PATCH v2 05/17] printk: Mark logbuf_lock & console_owner_lock as terminal locks Waiman Long
2018-11-19 18:55 ` [PATCH v2 06/17] debugobjects: Mark pool_lock as a terminal lock Waiman Long
2018-11-19 18:55 ` [PATCH v2 07/17] debugobjects: Move printk out of db lock critical sections Waiman Long
2018-11-21 16:49   ` Waiman Long
2018-11-22  2:04     ` Sergey Senozhatsky
2018-11-22 10:16       ` Peter Zijlstra
2018-11-23  2:40         ` Sergey Senozhatsky
2018-11-23 11:48           ` Petr Mladek
2018-11-26  4:57             ` Sergey Senozhatsky
2018-11-29 13:09               ` Petr Mladek
2018-11-22 16:02       ` Petr Mladek
2018-11-22 20:29         ` Waiman Long
2018-11-23 11:36           ` Petr Mladek [this message]
2018-11-22 19:57       ` Waiman Long
2018-11-23  2:35         ` Sergey Senozhatsky
2018-11-23 11:20         ` Petr Mladek
2018-12-07  9:21   ` Peter Zijlstra
2018-11-19 18:55 ` [PATCH v2 08/17] locking/lockdep: Add support for nestable terminal locks Waiman Long
2018-12-07  9:22   ` Peter Zijlstra
2018-12-07  9:52     ` Peter Zijlstra
2018-11-19 18:55 ` [PATCH v2 09/17] debugobjects: Make object hash locks " Waiman Long
2018-11-22 15:33   ` Petr Mladek
2018-11-22 20:17     ` Waiman Long
2018-11-23  9:29       ` Petr Mladek
2018-12-07  9:47   ` Peter Zijlstra
2018-11-19 18:55 ` [PATCH v2 10/17] lib/stackdepot: Make depot_lock a terminal spinlock Waiman Long
2018-11-19 18:55 ` [PATCH v2 11/17] locking/rwsem: Mark rwsem.wait_lock as a terminal lock Waiman Long
2018-11-19 18:55 ` [PATCH v2 12/17] cgroup: Mark the rstat percpu lock as terminal Waiman Long
2018-11-19 18:55 ` [PATCH v2 13/17] mm/kasan: Make quarantine_lock a terminal lock Waiman Long
2018-11-19 18:55 ` [PATCH v2 14/17] dma-debug: Mark free_entries_lock as terminal Waiman Long
2018-11-19 18:55 ` [PATCH v2 15/17] kernfs: Mark kernfs_open_node_lock as terminal lock Waiman Long
2018-11-19 18:55 ` [PATCH v2 16/17] delay_acct: Mark task's delays->lock as terminal spinlock Waiman Long
2018-11-19 18:55 ` [PATCH v2 17/17] locking/lockdep: Check raw/non-raw locking conflicts Waiman Long

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=20181123113648.6fieltetklnoex6v@pathway.suse.cz \
    --to=pmladek@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=aryabinin@virtuozzo.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=will.deacon@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).