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 <lumip@lumip.de>
Link: https://lore.kernel.org/all/20210912191207.74449-1-lumip@lumip.de
Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
Link: https://lore.kernel.org/r/20210913230942.1188-1-phil@philpotter.co.uk
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/Documentation/cdrom/cdrom-standard.rst b/Documentation/cdrom/cdrom-standard.rst
index 5845960ca382185e89603015f1d10429636d0467..52ea7b6b2fe8eb668c6c7fbd4389124de19af0a4 100644 (file)
--- a/Documentation/cdrom/cdrom-standard.rst
+++ b/Documentation/cdrom/cdrom-standard.rst
@@ -907,6 +907,17 @@ commands can be identified by the underscores in their names.
        specifies the slot for which the information is given. The special
        value *CDSL_CURRENT* requests that information about the currently
        selected slot be returned.
+`CDROM_TIMED_MEDIA_CHANGE`
+       Checks whether the disc has been changed since a user supplied time
+       and returns the time of the last disc change.
+
+       *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 of the last known media change (by the caller).
+       Upon successful return, this ioctl call will set
+       *arg->last_media_change* to the latest media change timestamp (in ms)
+       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.
 `CDROM_DRIVE_STATUS`
        Returns the status of the drive by a call to
        *drive_status()*. Return values are defined in cdrom_drive_status_.

diff --git a/Documentation/userspace-api/ioctl/cdrom.rst b/Documentation/userspace-api/ioctl/cdrom.rst
index 3b4c0506de466a362bc4c9d686cdc2e4462b422e..bac5bbf93ca0b81a3b4e8b3605cadb92fd936283 100644 (file)
--- a/Documentation/userspace-api/ioctl/cdrom.rst
+++ b/Documentation/userspace-api/ioctl/cdrom.rst
@@ -54,6 +54,9 @@ are as follows:
        CDROM_SELECT_SPEED      Set the CD-ROM speed
        CDROM_SELECT_DISC       Select disc (for juke-boxes)
        CDROM_MEDIA_CHANGED     Check is media changed
+       CDROM_TIMED_MEDIA_CHANGE        Check if media changed
+                                       since given time
+                                       (struct cdrom_timed_media_change_info)
        CDROM_DRIVE_STATUS      Get tray position, etc.
        CDROM_DISC_STATUS       Get disc type, etc.
        CDROM_CHANGER_NSLOTS    Get number of slots

diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index bd2e5b1560f525040327c6517dad3456f95cf601..89a68457820ab8b8a093664937dff11348317686 100644 (file)
--- 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:

diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
index c4fef00abdf38f92eb547bd2d6cba700a1ca9706..0a89f111e00e1c730c283785a5155f4ceaf02d2d 100644 (file)
--- a/include/linux/cdrom.h
+++ b/include/linux/cdrom.h
@@ -64,6 +64,7 @@ struct cdrom_device_info {
        int for_data;
        int (*exit)(struct cdrom_device_info *);
        int mrw_mode_page;
+       __s64 last_media_change_ms;
 };
 
 struct cdrom_device_ops {

diff --git a/include/uapi/linux/cdrom.h b/include/uapi/linux/cdrom.h
index 6c34f6e2f1f71e8de5d0f71b2f988ad70428a51b..804ff8d98f710c873c4876fcf3d6878cec4728f5 100644 (file)
--- a/include/uapi/linux/cdrom.h
+++ b/include/uapi/linux/cdrom.h
@@ -147,6 +147,8 @@
 #define CDROM_NEXT_WRITABLE    0x5394  /* get next writable block */
 #define CDROM_LAST_WRITTEN     0x5395  /* get last block written on disc */
 
+#define CDROM_TIMED_MEDIA_CHANGE   0x5396  /* get the timestamp of the last media change */
+
 /*******************************************************
  * CDROM IOCTL structures
  *******************************************************/
@@ -295,6 +297,23 @@ struct cdrom_generic_command
        };
 };
 
+/* This struct is used by CDROM_TIMED_MEDIA_CHANGE */
+struct cdrom_timed_media_change_info {
+       __s64   last_media_change;      /* Timestamp of the last detected media
+                                        * change in ms. May be set by caller,
+                                        * updated upon successful return of
+                                        * ioctl.
+                                        */
+       __u64   media_flags;            /* Flags returned by ioctl to indicate
+                                        * media status.
+                                        */
+};
+#define MEDIA_CHANGED_FLAG     0x1     /* Last detected media change was more
+                                        * recent than last_media_change set by
+                                        * caller.
+                                        */
+/* other bits of media_flags available for future use */
+
 /*
  * A CD-ROM physical sector size is 2048, 2052, 2056, 2324, 2332, 2336, 
  * 2340, or 2352 bytes long.