All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ming Lei <ming.lei@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Jens Axboe <axboe@kernel.dk>, linux-block@vger.kernel.org
Subject: Re: [PATCH 2/2] block: loop: delete partitions after clearing & changing fd
Date: Wed, 8 Jul 2020 17:13:18 +0800	[thread overview]
Message-ID: <20200708091318.GA3321276@T590> (raw)
In-Reply-To: <20200707175312.GB3730@lst.de>

On Tue, Jul 07, 2020 at 07:53:12PM +0200, Christoph Hellwig wrote:
> On Tue, Jul 07, 2020 at 04:45:52PM +0800, Ming Lei wrote:
> > After clearing fd or changing fd, we have to delete old partitions,
> > otherwise they may become ghost partitions.
> > 
> > Fix this issue by clearing GENHD_FL_NO_PART_SCAN during calling
> > bdev_disk_changed() which won't drop old partitions if GENHD_FL_NO_PART_SCAN
> > isn't set.
> 
> I don't think messing with GENHD_FL_NO_PART_SCAN is a good idea, as
> that will also cause an actual partition scan.  But except for historic
> reasons I can't think of a good idea to even check for
> GENHD_FL_NO_PART_SCAN in blk_drop_partitions.

I think it is safe to not check it in blk_drop_partitions(), how about
the following patch?

From a20209464c367c338beee5555f2cb5c8e8ad9f78 Mon Sep 17 00:00:00 2001
From: Ming Lei <ming.lei@redhat.com>
Date: Wed, 8 Jul 2020 16:07:19 +0800
Subject: [PATCH] block: always remove partitions in blk_drop_partitions()

So far blk_drop_partitions() only removes partitions when
disk_part_scan_enabled() return true. This way can make ghost partition on
loop device after changing/clearing FD in case that PARTSCAN is disabled.

Fix this issue by always removing partitions in blk_drop_partitions(), and
this way is correct because:

1) only loop, mmc and GENHD_FL_HIDDEN disks(nvme multipath) may set
GENHD_FL_NO_PART_SCAN

2) GENHD_FL_HIDDEN disks doesn't expose disk to block device fs, and
bdev_disk_changed()/blk_drop_partitions() won't be called for this kind of
disk

3) for mmc, if GENHD_FL_NO_PART_SCAN is set, no any partitions can be added
for this kind of disk, so blk_drop_partitions() basically does nothing no
matter if GENHD_FL_NO_PART_SCAN is set or not because disk_max_parts(disk) <= 1

4) for loop, bdev_disk_changed() is called in two cases: one is set fd and set
status, when there shouldn't be any partitions; another is clearing/changing fd,
we need to remove old partitions and re-read new partitions if there are and
PART_SCAN is enabled.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/partitions/core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/block/partitions/core.c b/block/partitions/core.c
index 78951e33b2d7..e62a98a8eeb7 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -619,8 +619,6 @@ int blk_drop_partitions(struct block_device *bdev)
 	struct disk_part_iter piter;
 	struct hd_struct *part;
 
-	if (!disk_part_scan_enabled(bdev->bd_disk))
-		return 0;
 	if (bdev->bd_part_count)
 		return -EBUSY;
 
-- 
2.25.2




thanks,
Ming


  reply	other threads:[~2020-07-08  9:13 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-07  8:45 [PATCH 0/2] block: loop: delete partitions after clearing & changing fd Ming Lei
2020-07-07  8:45 ` [PATCH 1/2] block: loop: share code of reread partitions Ming Lei
2020-07-07 17:49   ` Christoph Hellwig
2020-07-07  8:45 ` [PATCH 2/2] block: loop: delete partitions after clearing & changing fd Ming Lei
2020-07-07 17:53   ` Christoph Hellwig
2020-07-08  9:13     ` Ming Lei [this message]
2020-07-08  9:52       ` Ming Lei
  -- strict thread matches above, loose matches on Subject: below --
2025-03-08 16:14 [PATCH 0/5] loop: improve loop aio perf by IOCB_NOWAIT Ming Lei
2025-03-08 16:14 ` [PATCH 2/2] block: loop: delete partitions after clearing & changing fd Ming Lei

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=20200708091318.GA3321276@T590 \
    --to=ming.lei@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=hch@lst.de \
    --cc=linux-block@vger.kernel.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.