All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guoqing Jiang <guoqing.jiang@linux.dev>
To: Christoph Hellwig <hch@lst.de>, Song Liu <song@kernel.org>
Cc: Luis Chamberlain <mcgrof@kernel.org>,
	linux-raid@vger.kernel.org,
	syzbot+fadc0aaf497e6a493b9f@syzkaller.appspotmail.com,
	Neil Brown <neilb@suse.de>
Subject: Re: [PATCH 1/5] md: fix a lock order reversal in md_alloc
Date: Fri, 3 Sep 2021 14:08:29 +0800	[thread overview]
Message-ID: <2861229a-c9ff-3811-85eb-4362e46d2fe2@linux.dev> (raw)
In-Reply-To: <20210901113833.1334886-2-hch@lst.de>



On 9/1/21 7:38 PM, Christoph Hellwig wrote:
> Commit b0140891a8cea3 ("md: Fix race when creating a new md device.")
> not only moved assigning mddev->gendisk before calling add_disk, which
> fixes the races described in the commit log, but also added a
> mddev->open_mutex critical section over add_disk and creation of the
> md kobj.  Adding a kobject after add_disk is racy vs deleting the gendisk
> right after adding it, but md already prevents against that by holding
> a mddev->active reference.

Assuming you mean md_open calls mddev_find -> mddev_get
  -> atomic_inc(&mddev->active), but the path had already existed
before b0140891a8c,  and md_alloc also called mddev_find at
that time, not sure how it prevents the race though I probably missed
something. Cc Neil.

> On the other hand taking this lock added a lock order reversal with what
> is not disk->open_mutex (used to be bdev->bd_mutex when the commit was
> added) for partition devices, which need that lock for the internal open
> for the partition scan, and a recent commit also takes it for
> non-partitioned devices, leading to further lockdep splatter.
>
> Fixes: b0140891a8ce ("md: Fix race when creating a new md device.")
> Fixes: d62633873590 ("block: support delayed holder registration")

IIUC, the issue appeared after d6263387359 (which was for dm issue),
perhaps stable maintainer should not apply this to any stable kernel
if it only includes b0140891a8ce.

Thanks,
Guoqing

  reply	other threads:[~2021-09-03  6:08 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-01 11:38 fix a lock order reversal in md_alloc Christoph Hellwig
2021-09-01 11:38 ` [PATCH 1/5] md: " Christoph Hellwig
2021-09-03  6:08   ` Guoqing Jiang [this message]
2021-09-03  7:48     ` NeilBrown
2021-09-01 11:38 ` [PATCH 2/5] md: add error handling support for add_disk() Christoph Hellwig
2021-09-01 11:38 ` [PATCH 3/5] md: add the bitmap group to the default groups for the md kobject Christoph Hellwig
2021-09-01 11:38 ` [PATCH 4/5] md: extend disks_mutex coverage Christoph Hellwig
2021-09-01 11:38 ` [PATCH 5/5] md: properly unwind when failing to add the kobject in md_alloc Christoph Hellwig
2021-09-02  5:06 ` fix a lock order reversal " Song Liu
2021-09-04  1:48 ` Luis Chamberlain
2021-09-09  6:14 ` Song Liu

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=2861229a-c9ff-3811-85eb-4362e46d2fe2@linux.dev \
    --to=guoqing.jiang@linux.dev \
    --cc=hch@lst.de \
    --cc=linux-raid@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=neilb@suse.de \
    --cc=song@kernel.org \
    --cc=syzbot+fadc0aaf497e6a493b9f@syzkaller.appspotmail.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.