Re: [PATCH 08/31] block: share code between disk_check_media_change and disk_force_media_change

Hannes Reinecke
2023-06-07T12:19:29Z

On 6/6/23 09:39, Christoph Hellwig wrote:
> Factor the common logic between disk_check_media_change and
> disk_force_media_change into a helper.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> block/disk-events.c | 37 ++++++++++++++++---------------------
> 1 file changed, 16 insertions(+), 21 deletions(-)
>
> diff --git a/block/disk-events.c b/block/disk-events.c
> index 8b1b63225738f8..06f325662b3494 100644
> --- a/block/disk-events.c
> +++ b/block/disk-events.c
> @@ -262,6 +262,18 @@ static unsigned int disk_clear_events(struct gendisk *disk, unsigned int mask)
> return pending;
> }
>
> +static bool __disk_check_media_change(struct gendisk *disk, unsigned int events)
> +{
> + if (!(events & DISK_EVENT_MEDIA_CHANGE))
> + return false;
> +
> + if (__invalidate_device(disk->part0, true))
> + pr_warn("VFS: busy inodes on changed media %s\n",
> + disk->disk_name);
> + set_bit(GD_NEED_PART_SCAN, &disk->state);
> + return true;
> +}
> +
> /**
> * disk_check_media_change - check if a removable media has been changed
> * @disk: gendisk to check
> @@ -274,18 +286,9 @@ static unsigned int disk_clear_events(struct gendisk *disk, unsigned int mask)
> */
> bool disk_check_media_change(struct gendisk *disk)
> {
> - unsigned int events;
> -
> - events = disk_clear_events(disk, DISK_EVENT_MEDIA_CHANGE |
> - DISK_EVENT_EJECT_REQUEST);
> - if (!(events & DISK_EVENT_MEDIA_CHANGE))
> - return false;
> -
> - if (__invalidate_device(disk->part0, true))
> - pr_warn("VFS: busy inodes on changed media %s\n",
> - disk->disk_name);
> - set_bit(GD_NEED_PART_SCAN, &disk->state);
> - return true;
> + return __disk_check_media_change(disk,
> + disk_clear_events(disk, DISK_EVENT_MEDIA_CHANGE |
> + DISK_EVENT_EJECT_REQUEST));

Can you move the call to disk_clear_events() out of the call to
__disk_check_media_change()?
I find this pattern hard to read.

Cheers,

Hannes