From 67f1e027c27054e641584655020a417eaac9cb3a Mon Sep 17 00:00:00 2001 From: Lukas Prediger Date: Tue, 14 Sep 2021 00:09:42 +0100 Subject: drivers/cdrom: improved ioctl for media change detection The current implementation of the CDROM_MEDIA_CHANGED ioctl relies on global state, meaning that only one process can detect a disc change while the ioctl call will return 0 for other calling processes afterwards (see bug 213267). This introduces a new cdrom ioctl, CDROM_TIMED_MEDIA_CHANGE, that works by maintaining a timestamp of the last detected disc change instead of a boolean flag: Processes calling this ioctl command can provide a timestamp of the last disc change known to them and receive an indication whether the disc was changed since then and the updated timestamp. I considered fixing the buggy behavior in the original CDROM_MEDIA_CHANGED ioctl but that would require maintaining state for each calling process in the kernel, which seems like a worse solution than introducing this new ioctl. Signed-off-by: Lukas Prediger Link: https://lore.kernel.org/all/20210912191207.74449-1-lumip@lumip.de Signed-off-by: Phillip Potter Link: https://lore.kernel.org/r/20210913230942.1188-1-phil@philpotter.co.uk Signed-off-by: Jens Axboe --- drivers/cdrom/cdrom.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 4 deletions(-) (limited to 'drivers/cdrom') diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c index bd2e5b1560f5..89a68457820a 100644 --- a/drivers/cdrom/cdrom.c +++ b/drivers/cdrom/cdrom.c @@ -344,6 +344,12 @@ static void cdrom_sysctl_register(void); static LIST_HEAD(cdrom_list); +static void signal_media_change(struct cdrom_device_info *cdi) +{ + cdi->mc_flags = 0x3; /* set media changed bits, on both queues */ + cdi->last_media_change_ms = ktime_to_ms(ktime_get()); +} + int cdrom_dummy_generic_packet(struct cdrom_device_info *cdi, struct packet_command *cgc) { @@ -616,6 +622,7 @@ int register_cdrom(struct gendisk *disk, struct cdrom_device_info *cdi) ENSURE(cdo, generic_packet, CDC_GENERIC_PACKET); cdi->mc_flags = 0; cdi->options = CDO_USE_FFLAGS; + cdi->last_media_change_ms = ktime_to_ms(ktime_get()); if (autoclose == 1 && CDROM_CAN(CDC_CLOSE_TRAY)) cdi->options |= (int) CDO_AUTO_CLOSE; @@ -1421,8 +1428,7 @@ static int cdrom_select_disc(struct cdrom_device_info *cdi, int slot) cdi->ops->check_events(cdi, 0, slot); if (slot == CDSL_NONE) { - /* set media changed bits, on both queues */ - cdi->mc_flags = 0x3; + signal_media_change(cdi); return cdrom_load_unload(cdi, -1); } @@ -1455,7 +1461,7 @@ static int cdrom_select_disc(struct cdrom_device_info *cdi, int slot) slot = curslot; /* set media changed bits on both queues */ - cdi->mc_flags = 0x3; + signal_media_change(cdi); if ((ret = cdrom_load_unload(cdi, slot))) return ret; @@ -1521,7 +1527,7 @@ int media_changed(struct cdrom_device_info *cdi, int queue) cdi->ioctl_events = 0; if (changed) { - cdi->mc_flags = 0x3; /* set bit on both queues */ + signal_media_change(cdi); ret |= 1; cdi->media_written = 0; } @@ -2336,6 +2342,49 @@ static int cdrom_ioctl_media_changed(struct cdrom_device_info *cdi, return ret; } +/* + * Media change detection with timing information. + * + * arg is a pointer to a cdrom_timed_media_change_info struct. + * arg->last_media_change may be set by calling code to signal + * the timestamp (in ms) of the last known media change (by the caller). + * Upon successful return, ioctl call will set arg->last_media_change + * to the latest media change timestamp known by the kernel/driver + * and set arg->has_changed to 1 if that timestamp is more recent + * than the timestamp set by the caller. + */ +static int cdrom_ioctl_timed_media_change(struct cdrom_device_info *cdi, + unsigned long arg) +{ + int ret; + struct cdrom_timed_media_change_info __user *info; + struct cdrom_timed_media_change_info tmp_info; + + if (!CDROM_CAN(CDC_MEDIA_CHANGED)) + return -ENOSYS; + + info = (struct cdrom_timed_media_change_info __user *)arg; + cd_dbg(CD_DO_IOCTL, "entering CDROM_TIMED_MEDIA_CHANGE\n"); + + ret = cdrom_ioctl_media_changed(cdi, CDSL_CURRENT); + if (ret < 0) + return ret; + + if (copy_from_user(&tmp_info, info, sizeof(tmp_info)) != 0) + return -EFAULT; + + tmp_info.media_flags = 0; + if (tmp_info.last_media_change - cdi->last_media_change_ms < 0) + tmp_info.media_flags |= MEDIA_CHANGED_FLAG; + + tmp_info.last_media_change = cdi->last_media_change_ms; + + if (copy_to_user(info, &tmp_info, sizeof(*info)) != 0) + return -EFAULT; + + return 0; +} + static int cdrom_ioctl_set_options(struct cdrom_device_info *cdi, unsigned long arg) { @@ -3313,6 +3362,8 @@ int cdrom_ioctl(struct cdrom_device_info *cdi, struct block_device *bdev, return cdrom_ioctl_eject_sw(cdi, arg); case CDROM_MEDIA_CHANGED: return cdrom_ioctl_media_changed(cdi, arg); + case CDROM_TIMED_MEDIA_CHANGE: + return cdrom_ioctl_timed_media_change(cdi, arg); case CDROM_SET_OPTIONS: return cdrom_ioctl_set_options(cdi, arg); case CDROM_CLEAR_OPTIONS: -- cgit v1.2.3 From bbc3925cf696422492ebdaba386e61450fa2294c Mon Sep 17 00:00:00 2001 From: luo penghao Date: Thu, 21 Oct 2021 08:46:21 +0100 Subject: cdrom: Remove redundant variable and its assignment Variable is not used in functions, and its assignment is redundant too. So it should be deleted. Also the inner-most set of parentheses is no longer needed. The clang_analyzer complains as follows: drivers/cdrom/cdrom.c:877: warning: Although the value stored to 'ret' is used in the enclosing expression, the value is never actually read from 'ret'. Reported-by: Zeal Robot Signed-off-by: luo penghao Link: https://lore.kernel.org/all/20211020024229.1036219-1-luo.penghao@zte.com.cn Signed-off-by: Phillip Potter Link: https://lore.kernel.org/r/20211021074621.901-1-phil@philpotter.co.uk Signed-off-by: Jens Axboe --- drivers/cdrom/cdrom.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/cdrom') diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c index 89a68457820a..9877e413fce3 100644 --- a/drivers/cdrom/cdrom.c +++ b/drivers/cdrom/cdrom.c @@ -871,7 +871,7 @@ static void cdrom_mmc3_profile(struct cdrom_device_info *cdi) { struct packet_command cgc; char buffer[32]; - int ret, mmc3_profile; + int mmc3_profile; init_cdrom_command(&cgc, buffer, sizeof(buffer), CGC_DATA_READ); @@ -881,7 +881,7 @@ static void cdrom_mmc3_profile(struct cdrom_device_info *cdi) cgc.cmd[8] = sizeof(buffer); /* Allocation Length */ cgc.quiet = 1; - if ((ret = cdi->ops->generic_packet(cdi, &cgc))) + if (cdi->ops->generic_packet(cdi, &cgc)) mmc3_profile = 0xffff; else mmc3_profile = (buffer[6] << 8) | buffer[7]; -- cgit v1.2.3