All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luis Chamberlain <mcgrof@kernel.org>
To: Bart Van Assche <bvanassche@acm.org>
Cc: Christoph Hellwig <hch@infradead.org>,
	axboe@kernel.dk, viro@zeniv.linux.org.uk,
	gregkh@linuxfoundation.org, rostedt@goodmis.org,
	mingo@redhat.com, jack@suse.cz, ming.lei@redhat.com,
	nstange@suse.de, akpm@linux-foundation.org, mhocko@suse.com,
	yukuai3@huawei.com, linux-block@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, Omar Sandoval <osandov@fb.com>,
	Hannes Reinecke <hare@suse.com>, Michal Hocko <mhocko@kernel.org>
Subject: Re: [PATCH 3/5] blktrace: refcount the request_queue during ioctl
Date: Thu, 16 Apr 2020 05:29:09 +0000	[thread overview]
Message-ID: <20200416052909.GI11244@42.do-not-panic.com> (raw)
In-Reply-To: <a71d9c9b-72c8-8905-aeba-08e5382f5a81@acm.org>

On Wed, Apr 15, 2020 at 08:43:32PM -0700, Bart Van Assche wrote:
> On 2020-04-15 18:12, Luis Chamberlain wrote:
> > On Wed, Apr 15, 2020 at 07:18:22AM -0700, Bart Van Assche wrote:
> >> blk_get_queue() prevents concurrent freeing of struct request_queue but
> >> does not prevent concurrent blk_cleanup_queue() calls.
> > 
> > Wouldn't concurrent blk_cleanup_queue() calls be a bug? If so should
> > I make it clear that it would be or simply prevent it?
> 
> I think calling blk_cleanup_queue() while the queue refcount > 0 is well
> established behavior. At least the SCSI core triggers that behavior
> since a very long time. I prefer not to change that behavior.

I see. An alternative is to simply check if we already are cleaning up
and if so abort early on the blk_cleanup_queue(). That would allow
re-entrant calls, and just be a no-op to the additional calls. Or is
the re-entrant, two attemps to really do all the work
blk_cleanup_queue() expected functionality already?

> Regarding patch 3/5: how about dropping that patch? If the queue
> refcount can drop to zero while blk_trace_ioctl() is in progress I think
> that should be fixed in the block_device_operations.open callback
> instead of in blk_trace_ioctl().

I'll take a look, thanks!

  Luis

  reply	other threads:[~2020-04-16  5:29 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-14  4:18 [PATCH 0/5] blktrace: fix use after free Luis Chamberlain
2020-04-14  4:18 ` [PATCH 1/5] block: move main block debugfs initialization to its own file Luis Chamberlain
2020-04-14  7:35   ` Greg KH
2020-04-15  2:44   ` Bart Van Assche
2020-04-14  4:18 ` [PATCH 2/5] blktrace: fix debugfs use after free Luis Chamberlain
2020-04-14  7:37   ` Greg KH
2020-04-14 15:38   ` Christoph Hellwig
2020-04-15  2:46   ` Bart Van Assche
2020-04-15 17:38   ` Eric Sandeen
2020-04-15 21:48     ` Bart Van Assche
2020-04-16  0:56     ` Luis Chamberlain
2020-04-16  1:02       ` Eric Sandeen
2020-04-16  1:20         ` Luis Chamberlain
2020-04-16  2:10   ` Ming Lei
2020-04-16  5:25     ` Luis Chamberlain
2020-04-16  5:47       ` Ming Lei
2020-04-16  6:09         ` Ming Lei
2020-04-16  6:22           ` Luis Chamberlain
2020-04-16  6:20         ` Luis Chamberlain
2020-04-16  6:28           ` Ming Lei
2020-04-17  4:09             ` Luis Chamberlain
2020-04-17  4:09               ` Luis Chamberlain
2020-04-14  4:19 ` [PATCH 3/5] blktrace: refcount the request_queue during ioctl Luis Chamberlain
2020-04-14 15:40   ` Christoph Hellwig
2020-04-15  6:16     ` Luis Chamberlain
2020-04-15  7:14       ` Christoph Hellwig
2020-04-15 12:34         ` Luis Chamberlain
2020-04-15 12:39           ` Christoph Hellwig
2020-04-15 13:25             ` Luis Chamberlain
2020-04-15 14:18           ` Bart Van Assche
2020-04-16  1:12             ` Luis Chamberlain
2020-04-16  3:43               ` Bart Van Assche
2020-04-16  5:29                 ` Luis Chamberlain [this message]
2020-04-15 14:45       ` Bart Van Assche
2020-04-16  1:17         ` Luis Chamberlain
2020-04-16  2:31   ` Ming Lei
2020-04-16  5:36     ` Luis Chamberlain
2020-04-14  4:19 ` [PATCH 4/5] mm/swapfile: refcount block and queue before using blkcg_schedule_throttle() Luis Chamberlain
2020-04-14 15:44   ` Christoph Hellwig
2020-04-15  5:42     ` Luis Chamberlain
2020-04-15  7:27       ` Christoph Hellwig
2020-04-15  7:34         ` Christoph Hellwig
2020-04-15 13:19           ` Luis Chamberlain
2020-04-16  6:10             ` Christoph Hellwig
2020-04-16  6:22   ` Ming Lei
2020-04-16  6:25     ` Luis Chamberlain
2020-04-16  6:34       ` Ming Lei
2020-04-14  4:19 ` [PATCH 5/5] block: revert back to synchronous request_queue removal Luis Chamberlain
2020-04-14 15:47   ` Christoph Hellwig
2020-04-14 20:58     ` Luis Chamberlain
2020-04-15  6:46       ` Christoph Hellwig
2020-04-15 13:20         ` Luis Chamberlain
2020-04-16  2:36   ` Ming Lei
2020-04-14  7:38 ` [PATCH 0/5] blktrace: fix use after free Greg KH

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=20200416052909.GI11244@42.do-not-panic.com \
    --to=mcgrof@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=bvanassche@acm.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hare@suse.com \
    --cc=hch@infradead.org \
    --cc=jack@suse.cz \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=mhocko@suse.com \
    --cc=ming.lei@redhat.com \
    --cc=mingo@redhat.com \
    --cc=nstange@suse.de \
    --cc=osandov@fb.com \
    --cc=rostedt@goodmis.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=yukuai3@huawei.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.