All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Y. Ts'o" <tytso@mit.edu>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Linux Filesystem Development List <linux-fsdevel@vger.kernel.org>,
	linux-mm@kvack.org
Subject: Re: [PATCH -v2] memcg: fix a crash in wb_workfn when a device disappears
Date: Tue, 7 Jan 2020 21:12:38 -0500	[thread overview]
Message-ID: <20200108021238.GA218104@mit.edu> (raw)
In-Reply-To: <20200107153348.388a20e85e045d209c459e52@linux-foundation.org>

On Tue, Jan 07, 2020 at 03:33:48PM -0800, Andrew Morton wrote:
> On Fri, 27 Dec 2019 19:52:11 -0500 "Theodore Ts'o" <tytso@mit.edu> wrote:
> 
> > Unfortunately, del_gendisk() in block/gen_hd.c never got the memo
> > about the Brave New memcg World, and calls bdi_unregister directly.
> > It does this without informing the file system, or the memcg code, or
> > anything else.  This causes the root wb associated with the bdi to be
> > unregistered, but none of the memcg-specific wb's are shutdown.  So when
> > one of these wb's are woken up to do delayed work, they try to
> > dereference their wb->bdi->dev to fetch the device name, but
> > unfortunately bdi->dev is now NULL, thanks to the bdi_unregister()
> > called by del_gendisk().   As a result, *boom*.
> > 
> > Fortunately, it looks like the rest of the writeback path is perfectly
> > happy with bdi->dev and bdi->owner being NULL, so the simplest fix is
> > to create a bdi_dev_name() function which can handle bdi->dev being
> > NULL.  This also allows us to bulletproof the writeback tracepoints to
> > prevent them from dereferencing a NULL pointer and crashing the kernel
> > if one is tracing with memcg's enabled, and an iSCSI device dies or a
> > USB storage stick is pulled.
> 
> Is hotremoval of a device while tracing writeback the only known way of
> triggering this?

The most common way of triggering this will be hotremoval of a device
while writeback with memcg enabled is going on.  It was triggering
several times a day in a heavily loaded production environment.

> Is it worth a cc:stable?

Yes, I think so.

						- Ted

      reply	other threads:[~2020-01-08  2:12 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-27 19:48 [PATCH] memcg: fix a crash in wb_workfn when a device disappears Theodore Ts'o
2019-12-27 20:31 ` Theodore Ts'o
2019-12-27 21:16 ` kbuild test robot
2019-12-27 21:16   ` kbuild test robot
2019-12-27 21:16   ` kbuild test robot
2019-12-27 21:19   ` Theodore Y. Ts'o
2019-12-27 21:19     ` Theodore Y. Ts'o
2019-12-27 21:19     ` Theodore Y. Ts'o
2019-12-27 22:32 ` kbuild test robot
2019-12-27 22:32   ` kbuild test robot
2019-12-27 22:32   ` kbuild test robot
2019-12-28  0:52 ` [PATCH -v2] " Theodore Ts'o
2020-01-03 17:15   ` Theodore Y. Ts'o
2020-01-03 17:46     ` Chris Mason
2020-01-07 23:33   ` Andrew Morton
2020-01-08  2:12     ` Theodore Y. Ts'o [this message]

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=20200108021238.GA218104@mit.edu \
    --to=tytso@mit.edu \
    --cc=akpm@linux-foundation.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    /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.