Re: [PATCH] floppy: force media-change checks when clearing events

[Denis Efremov (Oracle)]

Hello,

On 06/05/2026 05:02, Cen Zhang wrote:
> floppy_open() tries to make blocking read/write opens perform a fresh
> media-change check by clearing drive_state[drive].last_checked before
> calling disk_check_media_change().  That timestamp is also updated by
> disk_change() when another FDC operation observes that the disk-change
> line is clear.
> 
> The worker path that calls disk_change() is not serialized by the
> floppy_mutex/open_lock pair held by floppy_open().  A worker can therefore
> store a recent jiffies value after floppy_open() stores zero and before the
> synchronous disk_check_media_change() call reaches floppy_check_events().
> The checkfreq throttle can then treat the media-change state as fresh and
> skip the hardware poll that the open path explicitly tried to force.

Thanks for the report and patch. The race you describe is real. However,
the proposed fix introduces a regression on the close path that I don't
think we can take as-is.

> 
> Use the block layer's clearing mask instead of a racy timestamp sentinel.
> When DISK_EVENT_MEDIA_CHANGE is being synchronously cleared, bypass the
> checkfreq throttle and poll the drive.  Periodic event checks still use
> last_checked to avoid unnecessary polling, and floppy_open() no longer
> needs to write last_checked itself.

bdev_release() unconditionally calls disk_flush_events(disk, DISK_EVENT_MEDIA_CHANGE);
on every final fput() of a block device. disk_flush_events() is not a no-op.
I'm afraid that after this patch every close of /dev/fd0 (or any floppy partition)
forces a synchronous FDC poll.

> 
> Signed-off-by: Cen Zhang <zzzccc427@xxxxxxxxx>
> ---
>  drivers/block/floppy.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index 92e446a643712..7217db0b907b3 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -4051,7 +4051,6 @@ static int floppy_open(struct gendisk *disk, blk_mode_t mode)
>  		fdc_state[FDC(drive)].rawcmd = 2;
>  	if (!(mode & BLK_OPEN_NDELAY)) {
>  		if (mode & (BLK_OPEN_READ | BLK_OPEN_WRITE)) {
> -			drive_state[drive].last_checked = 0;
>  			clear_bit(FD_OPEN_SHOULD_FAIL_BIT,
>  				  &drive_state[drive].flags);
>  			if (disk_check_media_change(disk))
> @@ -4092,7 +4091,9 @@ static unsigned int floppy_check_events(struct gendisk *disk,
>  	    test_bit(FD_VERIFY_BIT, &drive_state[drive].flags))
>  		return DISK_EVENT_MEDIA_CHANGE;
>  
> -	if (time_after(jiffies, drive_state[drive].last_checked + drive_params[drive].checkfreq)) {
> +	if ((clearing & DISK_EVENT_MEDIA_CHANGE) ||
> +	    time_after(jiffies, drive_state[drive].last_checked +
> +			       drive_params[drive].checkfreq)) {
>  		if (lock_fdc(drive))
>  			return 0;
>  		poll_drive(false, 0);

For v2 I think we should avoid using the block-layer clearing mask as a force-poll
signal. We can make floppy_open() do the forced open-time poll directly under
lock_fdc(), then let disk_check_media_change() observe the resulting state.
This code pattern is already lives in FDPOLLDRVSTAT/FDFMTBEG handling branches.

Denis