All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@redhat.com>
To: Stefan Bader <stefan.bader@canonical.com>
Cc: linux-kernel@vger.kernel.org, dm-devel@redhat.com,
	linux-block@vger.kernel.org, Alasdair Kergon <agk@redhat.com>,
	Jens Axboe <axboe@kernel.dk>,
	Tyler Hicks <tyler.hicks@canonical.com>
Subject: Re: [PATCH 1/1] blk/core: Gracefully handle unset make_request_fn
Date: Thu, 23 Jan 2020 05:35:41 -0500	[thread overview]
Message-ID: <20200123103541.GA28102@redhat.com> (raw)
In-Reply-To: <20200123091713.12623-2-stefan.bader@canonical.com>

On Thu, Jan 23 2020 at  4:17am -0500,
Stefan Bader <stefan.bader@canonical.com> wrote:

> When device-mapper adapted for multi-queue functionality, they
> also re-organized the way the make-request function was set.
> Before, this happened when the device-mapper logical device was
> created. Now it is done once the mapping table gets loaded the
> first time (this also decides whether the block device is request
> or bio based).
> 
> However in generic_make_request(), the request function gets used
> without further checks and this happens if one tries to mount such
> a partially set up device.
> 
> This can easily be reproduced with the following steps:
>  - dmsetup create -n test
>  - mount /dev/dm-<#> /mnt
> 
> This maybe is something which also should be fixed up in device-
> mapper.

I'll look closer at other options.

> But given there is already a check for an unset queue
> pointer and potentially there could be other drivers which do or
> might do the same, it sounds like a good move to add another check
> to generic_make_request_checks() and to bail out if the request
> function has not been set, yet.
> 
> BugLink: https://bugs.launchpad.net/bugs/1860231

From that bug;
"The currently proposed fix introduces no chance of stability
regressions. There is a chance of a very small performance regression
since an additional pointer comparison is performed on each block layer
request but this is unlikely to be noticeable."

This captures my immediate concern: slowing down everyone for this DM
edge-case isn't desirable.

Mike

  parent reply	other threads:[~2020-01-23 10:35 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-23  9:17 [PATCH 0/1] Handle NULL make_request_fn in generic_make_request() Stefan Bader
2020-01-23  9:17 ` [PATCH 1/1] blk/core: Gracefully handle unset make_request_fn Stefan Bader
2020-01-23 10:23   ` Tyler Hicks
2020-01-23 10:35   ` Mike Snitzer [this message]
2020-01-23 17:28     ` Mike Snitzer
2020-01-23 18:52       ` Jens Axboe
2020-01-24  6:04         ` Stefan Bader
2020-01-27 19:32         ` Mike Snitzer
2020-01-27 19:39           ` Jens Axboe
2020-01-28 14:32           ` Stefan Bader
2020-01-28 16:26             ` Mike Snitzer

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=20200123103541.GA28102@redhat.com \
    --to=snitzer@redhat.com \
    --cc=agk@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=dm-devel@redhat.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stefan.bader@canonical.com \
    --cc=tyler.hicks@canonical.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.