Hi Jens,
this series replaced the not very nice check_disk_change() function with
a new bdev_media_changed that avoids having the ->revalidate_disk call
at its end. As a result ->revalidate_disk can be removed from a lot of
drivers.
Diffstat:
block/genhd.c | 29 ++++++++++++++++++++++++++-
drivers/block/amiflop.c | 2 -
drivers/block/ataflop.c | 7 +++---
drivers/block/floppy.c | 8 ++++---
drivers/block/paride/pcd.c | 2 -
drivers/block/swim.c | 22 +-------------------
drivers/block/swim3.c | 4 +--
drivers/block/xsysace.c | 26 +++++++++---------------
drivers/cdrom/gdrom.c | 2 -
drivers/ide/ide-cd.c | 16 ++++-----------
drivers/ide/ide-disk.c | 5 ----
drivers/ide/ide-floppy.c | 2 -
drivers/ide/ide-gd.c | 48 +++++----------------------------------------
drivers/md/md.c | 2 -
drivers/scsi/sd.c | 7 +++---
drivers/scsi/sr.c | 36 +++++++++++++--------------------
fs/block_dev.c | 31 -----------------------------
include/linux/genhd.h | 3 --
include/linux/ide.h | 2 -
19 files changed, 86 insertions(+), 168 deletions(-)
Pass a struct ace_device to ace_revalidate_disk, move the media changed
check into the one caller that needs it, and give the routine a better
name.
Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/block/xsysace.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c
index eefe542f2d9fff..8d581c7536fb51 100644
--- a/drivers/block/xsysace.c
+++ b/drivers/block/xsysace.c
@@ -888,26 +888,20 @@ static unsigned int ace_check_events(struct gendisk *gd, unsigned int clearing)
return ace->media_change ? DISK_EVENT_MEDIA_CHANGE : 0;
}
-static int ace_revalidate_disk(struct gendisk *gd)
+static void ace_media_changed(struct ace_device *ace)
{
- struct ace_device *ace = gd->private_data;
unsigned long flags;
- dev_dbg(ace->dev, "ace_revalidate_disk()\n");
-
- if (ace->media_change) {
- dev_dbg(ace->dev, "requesting cf id and scheduling tasklet\n");
+ dev_dbg(ace->dev, "requesting cf id and scheduling tasklet\n");
- spin_lock_irqsave(&ace->lock, flags);
- ace->id_req_count++;
- spin_unlock_irqrestore(&ace->lock, flags);
+ spin_lock_irqsave(&ace->lock, flags);
+ ace->id_req_count++;
+ spin_unlock_irqrestore(&ace->lock, flags);
- tasklet_schedule(&ace->fsm_tasklet);
- wait_for_completion(&ace->id_completion);
- }
+ tasklet_schedule(&ace->fsm_tasklet);
+ wait_for_completion(&ace->id_completion);
dev_dbg(ace->dev, "revalidate complete\n");
- return ace->id_result;
}
static int ace_open(struct block_device *bdev, fmode_t mode)
@@ -922,8 +916,8 @@ static int ace_open(struct block_device *bdev, fmode_t mode)
ace->users++;
spin_unlock_irqrestore(&ace->lock, flags);
- if (bdev_check_media_change(bdev))
- ace_revalidate_disk(bdev->bd_disk);
+ if (bdev_check_media_change(bdev) && ace->media_change)
+ ace_media_changed(ace);
mutex_unlock(&xsysace_mutex);
return 0;
@@ -1080,7 +1074,7 @@ static int ace_setup(struct ace_device *ace)
(unsigned long long) ace->physaddr, ace->baseaddr, ace->irq);
ace->media_change = 1;
- ace_revalidate_disk(ace->gd);
+ ace_media_changed(ace);
/* Make the sysace device 'live' */
add_disk(ace->gd);
--
2.28.0
Switch to use bdev_check_media_change instead of check_disk_change and
call floppy_revalidate manually. Given that floppy_revalidate only
deals with media change events, the extra call into ->revalidate_disk
from bdev_disk_changed is not required either, so stop wiring up the
method.
Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/block/ataflop.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index a50e13af030526..3e881fdb06e0ad 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -1732,7 +1732,8 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode,
/* invalidate the buffer track to force a reread */
BufferDrive = -1;
set_bit(drive, &fake_change);
- check_disk_change(bdev);
+ if (bdev_check_media_change(bdev))
+ floppy_revalidate(bdev->bd_disk);
return 0;
default:
return -EINVAL;
@@ -1909,7 +1910,8 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
return 0;
if (mode & (FMODE_READ|FMODE_WRITE)) {
- check_disk_change(bdev);
+ if (bdev_check_media_change(bdev))
+ floppy_revalidate(bdev->bd_disk);
if (mode & FMODE_WRITE) {
if (p->wpstat) {
if (p->ref < 0)
@@ -1953,7 +1955,6 @@ static const struct block_device_operations floppy_fops = {
.release = floppy_release,
.ioctl = fd_ioctl,
.check_events = floppy_check_events,
- .revalidate_disk= floppy_revalidate,
};
static const struct blk_mq_ops ataflop_mq_ops = {
--
2.28.0
Switch to use bdev_check_media_change instead of check_disk_change and
call ace_revalidate_disk manually. Given that ace_revalidate_disk only
deals with media change events, the extra call into ->revalidate_disk
from bdev_disk_changed is not required either, so stop wiring up the
method.
Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/block/xsysace.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c
index 5d8e0ab3f054f5..eefe542f2d9fff 100644
--- a/drivers/block/xsysace.c
+++ b/drivers/block/xsysace.c
@@ -922,7 +922,8 @@ static int ace_open(struct block_device *bdev, fmode_t mode)
ace->users++;
spin_unlock_irqrestore(&ace->lock, flags);
- check_disk_change(bdev);
+ if (bdev_check_media_change(bdev))
+ ace_revalidate_disk(bdev->bd_disk);
mutex_unlock(&xsysace_mutex);
return 0;
@@ -966,7 +967,6 @@ static const struct block_device_operations ace_fops = {
.open = ace_open,
.release = ace_release,
.check_events = ace_check_events,
- .revalidate_disk = ace_revalidate_disk,
.getgeo = ace_getgeo,
};
--
2.28.0
Switch to use bdev_check_media_changed instead of check_disk_change and
call idecd_revalidate_disk manually. Given that idecd_revalidate_disk
only re-reads the TOC, and we already do the same at probe time, the
extra call into ->revalidate_disk from bdev_disk_changed is not required
either, so stop wiring up the method.
Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/ide/ide-cd.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 212bb2d8bf346a..6a38cbc80aea0d 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -56,6 +56,7 @@ static DEFINE_MUTEX(ide_cd_mutex);
static DEFINE_MUTEX(idecd_ref_mutex);
static void ide_cd_release(struct device *);
+static int idecd_revalidate_disk(struct gendisk *disk);
static struct cdrom_info *ide_cd_get(struct gendisk *disk)
{
@@ -1611,7 +1612,8 @@ static int idecd_open(struct block_device *bdev, fmode_t mode)
struct cdrom_info *info;
int rc = -ENXIO;
- check_disk_change(bdev);
+ if (bdev_check_media_change(bdev))
+ idecd_revalidate_disk(bdev->bd_disk);
mutex_lock(&ide_cd_mutex);
info = ide_cd_get(bdev->bd_disk);
@@ -1770,7 +1772,6 @@ static const struct block_device_operations idecd_ops = {
.compat_ioctl = IS_ENABLED(CONFIG_COMPAT) ?
idecd_compat_ioctl : NULL,
.check_events = idecd_check_events,
- .revalidate_disk = idecd_revalidate_disk
};
/* module options */
--
2.28.0
Like check_disk_changed, except that it does not call ->revalidate_disk
but leaves that to the caller.
Signed-off-by: Christoph Hellwig <[email protected]>
---
block/genhd.c | 29 ++++++++++++++++++++++++++++-
fs/block_dev.c | 17 +++--------------
include/linux/genhd.h | 2 +-
3 files changed, 32 insertions(+), 16 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 081f1039d9367f..9d060e79eb31d8 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -2052,7 +2052,7 @@ void disk_flush_events(struct gendisk *disk, unsigned int mask)
* CONTEXT:
* Might sleep.
*/
-unsigned int disk_clear_events(struct gendisk *disk, unsigned int mask)
+static unsigned int disk_clear_events(struct gendisk *disk, unsigned int mask)
{
struct disk_events *ev = disk->ev;
unsigned int pending;
@@ -2090,6 +2090,33 @@ unsigned int disk_clear_events(struct gendisk *disk, unsigned int mask)
return pending;
}
+/**
+ * bdev_check_media_change - check if a removable media has been changed
+ * @bdev: block device to check
+ *
+ * Check whether a removable media has been changed, and attempt to free all
+ * dentries and inodes and invalidates all block device page cache entries in
+ * that case.
+ *
+ * Returns %true if the block device changed, or %false if not.
+ */
+bool bdev_check_media_change(struct block_device *bdev)
+{
+ unsigned int events;
+
+ events = disk_clear_events(bdev->bd_disk, DISK_EVENT_MEDIA_CHANGE |
+ DISK_EVENT_EJECT_REQUEST);
+ if (!(events & DISK_EVENT_MEDIA_CHANGE))
+ return false;
+
+ if (__invalidate_device(bdev, true))
+ pr_warn("VFS: busy inodes on changed media %s\n",
+ bdev->bd_disk->disk_name);
+ set_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
+ return true;
+}
+EXPORT_SYMBOL(bdev_check_media_change);
+
/*
* Separate this part out so that a different pointer for clearing_ptr can be
* passed in for disk_clear_events.
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 9cb205405f9d99..37cb809b217926 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1350,21 +1350,10 @@ EXPORT_SYMBOL(revalidate_disk_size);
*/
int check_disk_change(struct block_device *bdev)
{
- struct gendisk *disk = bdev->bd_disk;
- const struct block_device_operations *bdops = disk->fops;
- unsigned int events;
-
- events = disk_clear_events(disk, DISK_EVENT_MEDIA_CHANGE |
- DISK_EVENT_EJECT_REQUEST);
- if (!(events & DISK_EVENT_MEDIA_CHANGE))
+ if (!bdev_check_media_change(bdev))
return 0;
-
- if (__invalidate_device(bdev, true))
- pr_warn("VFS: busy inodes on changed media %s\n",
- disk->disk_name);
- set_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
- if (bdops->revalidate_disk)
- bdops->revalidate_disk(bdev->bd_disk);
+ if (bdev->bd_disk->fops->revalidate_disk)
+ bdev->bd_disk->fops->revalidate_disk(bdev->bd_disk);
return 1;
}
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index c618b27292fcc8..322d48a207728a 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -315,7 +315,6 @@ extern void disk_unblock_events(struct gendisk *disk);
extern void disk_flush_events(struct gendisk *disk, unsigned int mask);
void set_capacity_revalidate_and_notify(struct gendisk *disk, sector_t size,
bool update_bdev);
-extern unsigned int disk_clear_events(struct gendisk *disk, unsigned int mask);
/* drivers/char/random.c */
extern void add_disk_randomness(struct gendisk *disk) __latent_entropy;
@@ -372,6 +371,7 @@ void unregister_blkdev(unsigned int major, const char *name);
void revalidate_disk_size(struct gendisk *disk, bool verbose);
int check_disk_change(struct block_device *bdev);
+bool bdev_check_media_change(struct block_device *bdev);
int __invalidate_device(struct block_device *bdev, bool kill_dirty);
void bd_set_nr_sectors(struct block_device *bdev, sector_t sectors);
--
2.28.0
floppy_revalidate mostly duplicates work already done in floppy_open
despite only beeing called from floppy_open. Remove the function and
just clear the ->ejected flag directly under the right condition.
Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/block/swim.c | 24 ++----------------------
1 file changed, 2 insertions(+), 22 deletions(-)
diff --git a/drivers/block/swim.c b/drivers/block/swim.c
index d4565c555b289f..52dd1efa00f9c5 100644
--- a/drivers/block/swim.c
+++ b/drivers/block/swim.c
@@ -217,8 +217,6 @@ extern int swim_read_sector_header(struct swim __iomem *base,
extern int swim_read_sector_data(struct swim __iomem *base,
unsigned char *data);
-static int floppy_revalidate(struct gendisk *disk);
-
static DEFINE_MUTEX(swim_mutex);
static inline void set_swim_mode(struct swim __iomem *base, int enable)
{
@@ -640,8 +638,8 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
return 0;
if (mode & (FMODE_READ|FMODE_WRITE)) {
- if (bdev_check_media_change(bdev))
- floppy_revalidate(bdev->bd_disk);
+ if (bdev_check_media_change(bdev) && fs->disk_in)
+ fs->ejected = 0;
if ((mode & FMODE_WRITE) && fs->write_protected) {
err = -EROFS;
goto out;
@@ -738,24 +736,6 @@ static unsigned int floppy_check_events(struct gendisk *disk,
return fs->ejected ? DISK_EVENT_MEDIA_CHANGE : 0;
}
-static int floppy_revalidate(struct gendisk *disk)
-{
- struct floppy_state *fs = disk->private_data;
- struct swim __iomem *base = fs->swd->base;
-
- swim_drive(base, fs->location);
-
- if (fs->ejected)
- setup_medium(fs);
-
- if (!fs->disk_in)
- swim_motor(base, OFF);
- else
- fs->ejected = 0;
-
- return !fs->disk_in;
-}
-
static const struct block_device_operations floppy_fops = {
.owner = THIS_MODULE,
.open = floppy_unlocked_open,
--
2.28.0
Looks good,
Reviewed-by: Johannes Thumshirn <[email protected]>
Looks good,
Reviewed-by: Johannes Thumshirn <[email protected]>
And down by one,
Reviewed-by: Johannes Thumshirn <[email protected]>
On 2020-09-02 10:11 a.m., Christoph Hellwig wrote:
> Hi Jens,
>
> this series replaced the not very nice check_disk_change() function with
> a new bdev_media_changed that avoids having the ->revalidate_disk call
> at its end. As a result ->revalidate_disk can be removed from a lot of
> drivers.
>
For over 20 years the sg driver has been carrying this snippet that hangs
off the completion callback:
if (driver_stat & DRIVER_SENSE) {
struct scsi_sense_hdr ssh;
if (scsi_normalize_sense(sbp, sense_len, &ssh)) {
if (!scsi_sense_is_deferred(&ssh)) {
if (ssh.sense_key == UNIT_ATTENTION) {
if (sdp->device->removable)
sdp->device->changed = 1;
}
}
}
}
Is it needed? The unit attention (UA) may not be associated with the
device changing. Shouldn't the SCSI mid-level monitor UAs if they
impact the state of a scsi_device object?
Doug Gilbert
Looks good,
Reviewed-by: Johannes Thumshirn <[email protected]>
Looks good,
Reviewed-by: Johannes Thumshirn <[email protected]>
On 9/2/20 5:38 PM, Douglas Gilbert wrote:
> On 2020-09-02 10:11 a.m., Christoph Hellwig wrote:
>> Hi Jens,
>>
>> this series replaced the not very nice check_disk_change() function with
>> a new bdev_media_changed that avoids having the ->revalidate_disk call
>> at its end. As a result ->revalidate_disk can be removed from a lot of
>> drivers.
>>
>
> For over 20 years the sg driver has been carrying this snippet that hangs
> off the completion callback:
>
> if (driver_stat & DRIVER_SENSE) {
> struct scsi_sense_hdr ssh;
>
> if (scsi_normalize_sense(sbp, sense_len, &ssh)) {
> if (!scsi_sense_is_deferred(&ssh)) {
> if (ssh.sense_key == UNIT_ATTENTION) {
> if (sdp->device->removable)
> sdp->device->changed = 1;
> }
> }
> }
> }
>
> Is it needed? The unit attention (UA) may not be associated with the
> device changing. Shouldn't the SCSI mid-level monitor UAs if they
> impact the state of a scsi_device object?
>
We do; check scsi_io_completion_action() in drivers/scsi/scsi_lib.c
So I don't think you'd need to keep it in sg.c.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
[email protected] +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer