linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] drivers/cdrom: improved ioctl for media change detection
@ 2021-09-13 23:09 Phillip Potter
  2021-09-15  2:05 ` Jens Axboe
  0 siblings, 1 reply; 12+ messages in thread
From: Phillip Potter @ 2021-09-13 23:09 UTC (permalink / raw)
  To: axboe; +Cc: linux-block

From: Lukas Prediger <lumip@lumip.de>

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>
---
 Documentation/cdrom/cdrom-standard.rst      | 11 ++++
 Documentation/userspace-api/ioctl/cdrom.rst |  3 ++
 drivers/cdrom/cdrom.c                       | 59 +++++++++++++++++++--
 include/linux/cdrom.h                       |  1 +
 include/uapi/linux/cdrom.h                  | 19 +++++++
 5 files changed, 89 insertions(+), 4 deletions(-)

diff --git a/Documentation/cdrom/cdrom-standard.rst b/Documentation/cdrom/cdrom-standard.rst
index 5845960ca382..52ea7b6b2fe8 100644
--- 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 3b4c0506de46..bac5bbf93ca0 100644
--- 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 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:
diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
index c4fef00abdf3..0a89f111e00e 100644
--- 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 6c34f6e2f1f7..804ff8d98f71 100644
--- 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.  
-- 
2.31.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] drivers/cdrom: improved ioctl for media change detection
  2021-09-13 23:09 [PATCH v3] drivers/cdrom: improved ioctl for media change detection Phillip Potter
@ 2021-09-15  2:05 ` Jens Axboe
  2021-09-16 23:07   ` Phillip Potter
  0 siblings, 1 reply; 12+ messages in thread
From: Jens Axboe @ 2021-09-15  2:05 UTC (permalink / raw)
  To: Phillip Potter; +Cc: linux-block

On 9/13/21 5:09 PM, Phillip Potter wrote:
> From: Lukas Prediger <lumip@lumip.de>
> 
> 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.

Applied for 5.16, thanks.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] drivers/cdrom: improved ioctl for media change detection
  2021-09-15  2:05 ` Jens Axboe
@ 2021-09-16 23:07   ` Phillip Potter
  0 siblings, 0 replies; 12+ messages in thread
From: Phillip Potter @ 2021-09-16 23:07 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block

On Tue, Sep 14, 2021 at 08:05:41PM -0600, Jens Axboe wrote:
> On 9/13/21 5:09 PM, Phillip Potter wrote:
> > From: Lukas Prediger <lumip@lumip.de>
> > 
> > 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.
> 
> Applied for 5.16, thanks.
> 
> -- 
> Jens Axboe
> 

Many thanks Jens.

Regards,
Phil

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] drivers/cdrom: improved ioctl for media change detection
  2021-10-06 20:52           ` Randy Dunlap
@ 2021-10-07 23:04             ` Phillip Potter
  0 siblings, 0 replies; 12+ messages in thread
From: Phillip Potter @ 2021-10-07 23:04 UTC (permalink / raw)
  To: Randy Dunlap; +Cc: axboe, hch, linux-block, linux-kernel, Lukas Prediger

On Wed, Oct 06, 2021 at 01:52:44PM -0700, Randy Dunlap wrote:
> On 9/12/21 12:12 PM, Lukas Prediger wrote:
> > diff --git a/Documentation/userspace-api/ioctl/cdrom.rst b/Documentation/userspace-api/ioctl/cdrom.rst
> > index 3b4c0506de46..bac5bbf93ca0 100644
> > --- 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
> 
> Hi Lukas, Phil,
> 
> This doc change causes a documentation build warning:
> 
> Documentation/userspace-api/ioctl/cdrom.rst:57: WARNING: Malformed table.
> Text in column margin in table line 42.
> 
> The "=====" lines describe the table columns and they cannot be
> exceeded without a warning. The table needs to be reformatted.
> 
> Lukas, will you handle that?
> thanks.
> 
> 
> ======================  ===============================================
> CDROMPAUSE              Pause Audio Operation
> CDROMRESUME             Resume paused Audio Operation
> CDROMPLAYMSF            Play Audio MSF (struct cdrom_msf)
> CDROMPLAYTRKIND         Play Audio Track/index (struct cdrom_ti)
> CDROMREADTOCHDR         Read TOC header (struct cdrom_tochdr)
> CDROMREADTOCENTRY       Read TOC entry (struct cdrom_tocentry)
> CDROMSTOP               Stop the cdrom drive
> CDROMSTART              Start the cdrom drive
> CDROMEJECT              Ejects the cdrom media
> CDROMVOLCTRL            Control output volume (struct cdrom_volctrl)
> CDROMSUBCHNL            Read subchannel data (struct cdrom_subchnl)
> CDROMREADMODE2          Read CDROM mode 2 data (2336 Bytes)
>                         (struct cdrom_read)
> CDROMREADMODE1          Read CDROM mode 1 data (2048 Bytes)
>                         (struct cdrom_read)
> CDROMREADAUDIO          (struct cdrom_read_audio)
> CDROMEJECT_SW           enable(1)/disable(0) auto-ejecting
> CDROMMULTISESSION       Obtain the start-of-last-session
>                         address of multi session disks
>                         (struct cdrom_multisession)
> CDROM_GET_MCN           Obtain the "Universal Product Code"
>                         if available (struct cdrom_mcn)
> CDROM_GET_UPC           Deprecated, use CDROM_GET_MCN instead.
> CDROMRESET              hard-reset the drive
> CDROMVOLREAD            Get the drive's volume setting
>                         (struct cdrom_volctrl)
> CDROMREADRAW            read data in raw mode (2352 Bytes)
>                         (struct cdrom_read)
> CDROMREADCOOKED         read data in cooked mode
> CDROMSEEK               seek msf address
> CDROMPLAYBLK            scsi-cd only, (struct cdrom_blk)
> CDROMREADALL            read all 2646 bytes
> CDROMGETSPINDOWN        return 4-bit spindown value
> CDROMSETSPINDOWN        set 4-bit spindown value
> CDROMCLOSETRAY          pendant of CDROMEJECT
> CDROM_SET_OPTIONS       Set behavior options
> CDROM_CLEAR_OPTIONS     Clear behavior options
> 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
> CDROM_LOCKDOOR          lock or unlock door
> CDROM_DEBUG             Turn debug messages on/off
> CDROM_GET_CAPABILITY    get capabilities
> CDROMAUDIOBUFSIZ        set the audio buffer size
> DVD_READ_STRUCT         Read structure
> DVD_WRITE_STRUCT        Write structure
> DVD_AUTH                Authentication
> CDROM_SEND_PACKET       send a packet to the drive
> CDROM_NEXT_WRITABLE     get next writable block
> CDROM_LAST_WRITTEN      get last block written on disc
> ======================  ===============================================
> 
> 
> -- 
> ~Randy

Hi Randy,

Thanks for heads up. I've prepared a patch to reformat the table which
I'll send shortly.

Regards,
Phil

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] drivers/cdrom: improved ioctl for media change detection
  2021-09-12 19:12         ` Lukas Prediger
  2021-09-13  7:50           ` Phillip Potter
@ 2021-10-06 20:52           ` Randy Dunlap
  2021-10-07 23:04             ` Phillip Potter
  1 sibling, 1 reply; 12+ messages in thread
From: Randy Dunlap @ 2021-10-06 20:52 UTC (permalink / raw)
  To: Lukas Prediger, phil; +Cc: axboe, hch, linux-block, linux-kernel

On 9/12/21 12:12 PM, Lukas Prediger wrote:
> diff --git a/Documentation/userspace-api/ioctl/cdrom.rst b/Documentation/userspace-api/ioctl/cdrom.rst
> index 3b4c0506de46..bac5bbf93ca0 100644
> --- 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

Hi Lukas, Phil,

This doc change causes a documentation build warning:

Documentation/userspace-api/ioctl/cdrom.rst:57: WARNING: Malformed table.
Text in column margin in table line 42.

The "=====" lines describe the table columns and they cannot be
exceeded without a warning. The table needs to be reformatted.

Lukas, will you handle that?
thanks.


======================  ===============================================
CDROMPAUSE              Pause Audio Operation
CDROMRESUME             Resume paused Audio Operation
CDROMPLAYMSF            Play Audio MSF (struct cdrom_msf)
CDROMPLAYTRKIND         Play Audio Track/index (struct cdrom_ti)
CDROMREADTOCHDR         Read TOC header (struct cdrom_tochdr)
CDROMREADTOCENTRY       Read TOC entry (struct cdrom_tocentry)
CDROMSTOP               Stop the cdrom drive
CDROMSTART              Start the cdrom drive
CDROMEJECT              Ejects the cdrom media
CDROMVOLCTRL            Control output volume (struct cdrom_volctrl)
CDROMSUBCHNL            Read subchannel data (struct cdrom_subchnl)
CDROMREADMODE2          Read CDROM mode 2 data (2336 Bytes)
                         (struct cdrom_read)
CDROMREADMODE1          Read CDROM mode 1 data (2048 Bytes)
                         (struct cdrom_read)
CDROMREADAUDIO          (struct cdrom_read_audio)
CDROMEJECT_SW           enable(1)/disable(0) auto-ejecting
CDROMMULTISESSION       Obtain the start-of-last-session
                         address of multi session disks
                         (struct cdrom_multisession)
CDROM_GET_MCN           Obtain the "Universal Product Code"
                         if available (struct cdrom_mcn)
CDROM_GET_UPC           Deprecated, use CDROM_GET_MCN instead.
CDROMRESET              hard-reset the drive
CDROMVOLREAD            Get the drive's volume setting
                         (struct cdrom_volctrl)
CDROMREADRAW            read data in raw mode (2352 Bytes)
                         (struct cdrom_read)
CDROMREADCOOKED         read data in cooked mode
CDROMSEEK               seek msf address
CDROMPLAYBLK            scsi-cd only, (struct cdrom_blk)
CDROMREADALL            read all 2646 bytes
CDROMGETSPINDOWN        return 4-bit spindown value
CDROMSETSPINDOWN        set 4-bit spindown value
CDROMCLOSETRAY          pendant of CDROMEJECT
CDROM_SET_OPTIONS       Set behavior options
CDROM_CLEAR_OPTIONS     Clear behavior options
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
CDROM_LOCKDOOR          lock or unlock door
CDROM_DEBUG             Turn debug messages on/off
CDROM_GET_CAPABILITY    get capabilities
CDROMAUDIOBUFSIZ        set the audio buffer size
DVD_READ_STRUCT         Read structure
DVD_WRITE_STRUCT        Write structure
DVD_AUTH                Authentication
CDROM_SEND_PACKET       send a packet to the drive
CDROM_NEXT_WRITABLE     get next writable block
CDROM_LAST_WRITTEN      get last block written on disc
======================  ===============================================


-- 
~Randy

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] drivers/cdrom: improved ioctl for media change detection
  2021-09-12 19:12         ` Lukas Prediger
@ 2021-09-13  7:50           ` Phillip Potter
  2021-10-06 20:52           ` Randy Dunlap
  1 sibling, 0 replies; 12+ messages in thread
From: Phillip Potter @ 2021-09-13  7:50 UTC (permalink / raw)
  To: Lukas Prediger; +Cc: axboe, hch, linux-block, linux-kernel, rdunlap

On Sun, Sep 12, 2021 at 10:12:08PM +0300, Lukas Prediger wrote:
> > Dear Lukas,
> > 
> > This v3 patch does not apply to my tree, or the mainline one for that
> > matter. A few problems I've noticed that are the cause of this:
> >
> > [..] 
> >
> > Please advise, many thanks. As mentioned before, v2 applied perfectly
> > fine for me.
> >
> > Regards,
> > Phil
> 
> Hey Phil,
> it seems I had accidentally formated the patch off my v5.4 testing branch.
> Here's the proper version based on current master, that should apply cleanly.
> 
> I've also removed the continuation backslashes that Randy pointed out now.
> 
> Sorry for the troubles,
> Lukas
> 
Dear Lukas,

No apology necessary - I will check this over after work this evening
and then send onto Jens. Many thanks.

Regards,
Phil

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3] drivers/cdrom: improved ioctl for media change detection
  2021-09-12 18:04       ` Phillip Potter
@ 2021-09-12 19:12         ` Lukas Prediger
  2021-09-13  7:50           ` Phillip Potter
  2021-10-06 20:52           ` Randy Dunlap
  0 siblings, 2 replies; 12+ messages in thread
From: Lukas Prediger @ 2021-09-12 19:12 UTC (permalink / raw)
  To: phil; +Cc: axboe, hch, linux-block, linux-kernel, rdunlap, Lukas Prediger

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>
---

> Dear Lukas,
> 
> This v3 patch does not apply to my tree, or the mainline one for that
> matter. A few problems I've noticed that are the cause of this:
>
> [..] 
>
> Please advise, many thanks. As mentioned before, v2 applied perfectly
> fine for me.
>
> Regards,
> Phil

Hey Phil,
it seems I had accidentally formated the patch off my v5.4 testing branch.
Here's the proper version based on current master, that should apply cleanly.

I've also removed the continuation backslashes that Randy pointed out now.

Sorry for the troubles,
Lukas

---
 Documentation/cdrom/cdrom-standard.rst      | 11 ++++
 Documentation/userspace-api/ioctl/cdrom.rst |  3 ++
 drivers/cdrom/cdrom.c                       | 59 +++++++++++++++++++--
 include/linux/cdrom.h                       |  1 +
 include/uapi/linux/cdrom.h                  | 19 +++++++
 5 files changed, 89 insertions(+), 4 deletions(-)

diff --git a/Documentation/cdrom/cdrom-standard.rst b/Documentation/cdrom/cdrom-standard.rst
index 5845960ca382..52ea7b6b2fe8 100644
--- 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 3b4c0506de46..bac5bbf93ca0 100644
--- 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 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:
diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
index c4fef00abdf3..0a89f111e00e 100644
--- 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 6c34f6e2f1f7..6d528a7a1a59 100644
--- 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.  
-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] drivers/cdrom: improved ioctl for media change detection
  2021-09-11 17:48     ` Lukas Prediger
@ 2021-09-12 18:04       ` Phillip Potter
  2021-09-12 19:12         ` Lukas Prediger
  0 siblings, 1 reply; 12+ messages in thread
From: Phillip Potter @ 2021-09-12 18:04 UTC (permalink / raw)
  To: Lukas Prediger; +Cc: axboe, hch, linux-block, linux-kernel, rdunlap

On Sat, Sep 11, 2021 at 08:48:17PM +0300, Lukas Prediger wrote:
> Hi Randy, Hi Phil,
> 
> >>
> >> Hi Lukas,
> >>
> >> Just a minor nit:
> >>
> >> On 9/10/21 2:16 PM, Lukas Prediger wrote:
> >> > +#define MEDIA_CHANGED_FLAG   0x1     /* Last detected media change was more \
> >> > +                                      * recent than last_media_change set by\
> >> > +                                      * caller.                             \
> >> > +                                      */
> >>
> >> Drop the "continuation" backslashes.
> >> They are not needed.
> >>
> >> thanks.
> >> --
> >> ~Randy
> >>
> 
> Hm, my IDE was complaining about these but I just tested building without and
> that worked fine. No idea what that was about then..
> 
> >
> > Dear Lukas,
> >
> > Happy to take these out for you and save you resubmitting.
> > I'm very happy with patch anyway. Once I hear back from
> > you I'll send onto Jens with my approval after one final test :-)
> >
> 
> That would be very nice of you!
> 
> >
> > Thanks again for the code.
> >
> 
> My pleasure, and thanks for the helpful feedback!
> 
> >
> > Regards,
> > Phil
> 
> Best regards,
> Lukas

Dear Lukas,

This v3 patch does not apply to my tree, or the mainline one for that
matter. A few problems I've noticed that are the cause of this:

Misnamed file:
Documentation/ioctl/cdrom.rst - this file should actually be:
Documentation/userspace-api/ioctl/cdrom.rst (and is in v2)

Failed hunks:
v3 fails on applying the changes to drivers/cdrom/cdrom.c as well, due
to two hunks failing (because of different lines preceding your
changes).

It also fails to apply the second hunk to include/uapi/linux/cdrom.h,
again due to differences in pre-existing content. Did you base this v3
patch against a different/older tree/branch?

Please advise, many thanks. As mentioned before, v2 applied perfectly
fine for me.

Regards,
Phil

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] drivers/cdrom: improved ioctl for media change detection
  2021-09-11 11:00   ` Phillip Potter
@ 2021-09-11 17:48     ` Lukas Prediger
  2021-09-12 18:04       ` Phillip Potter
  0 siblings, 1 reply; 12+ messages in thread
From: Lukas Prediger @ 2021-09-11 17:48 UTC (permalink / raw)
  To: phil; +Cc: axboe, hch, linux-block, linux-kernel, rdunlap

Hi Randy, Hi Phil,

>>
>> Hi Lukas,
>>
>> Just a minor nit:
>>
>> On 9/10/21 2:16 PM, Lukas Prediger wrote:
>> > +#define MEDIA_CHANGED_FLAG   0x1     /* Last detected media change was more \
>> > +                                      * recent than last_media_change set by\
>> > +                                      * caller.                             \
>> > +                                      */
>>
>> Drop the "continuation" backslashes.
>> They are not needed.
>>
>> thanks.
>> --
>> ~Randy
>>

Hm, my IDE was complaining about these but I just tested building without and
that worked fine. No idea what that was about then..

>
> Dear Lukas,
>
> Happy to take these out for you and save you resubmitting.
> I'm very happy with patch anyway. Once I hear back from
> you I'll send onto Jens with my approval after one final test :-)
>

That would be very nice of you!

>
> Thanks again for the code.
>

My pleasure, and thanks for the helpful feedback!

>
> Regards,
> Phil

Best regards,
Lukas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] drivers/cdrom: improved ioctl for media change detection
  2021-09-10 21:41 ` Randy Dunlap
@ 2021-09-11 11:00   ` Phillip Potter
  2021-09-11 17:48     ` Lukas Prediger
  0 siblings, 1 reply; 12+ messages in thread
From: Phillip Potter @ 2021-09-11 11:00 UTC (permalink / raw)
  To: Lukas Prediger
  Cc: Jens Axboe, Linux Kernel Mailing List, linux-block,
	Christoph Hellwig, Randy Dunlap

On Fri, 10 Sept 2021 at 22:41, Randy Dunlap <rdunlap@infradead.org> wrote:
>
> Hi Lukas,
>
> Just a minor nit:
>
> On 9/10/21 2:16 PM, Lukas Prediger wrote:
> > +#define MEDIA_CHANGED_FLAG   0x1     /* Last detected media change was more \
> > +                                      * recent than last_media_change set by\
> > +                                      * caller.                             \
> > +                                      */
>
> Drop the "continuation" backslashes.
> They are not needed.
>
> thanks.
> --
> ~Randy
>

Dear Lukas,

Happy to take these out for you and save you resubmitting.
I'm very happy with patch anyway. Once I hear back from
you I'll send onto Jens with my approval after one final test :-)

Thanks again for the code.

Regards,
Phil

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] drivers/cdrom: improved ioctl for media change detection
  2021-09-10 21:16 Lukas Prediger
@ 2021-09-10 21:41 ` Randy Dunlap
  2021-09-11 11:00   ` Phillip Potter
  0 siblings, 1 reply; 12+ messages in thread
From: Randy Dunlap @ 2021-09-10 21:41 UTC (permalink / raw)
  To: Lukas Prediger, phil; +Cc: axboe, linux-kernel, linux-block, hch

Hi Lukas,

Just a minor nit:

On 9/10/21 2:16 PM, Lukas Prediger wrote:
> +#define MEDIA_CHANGED_FLAG	0x1	/* Last detected media change was more \
> +					 * recent than last_media_change set by\
> +					 * caller.                             \
> +					 */

Drop the "continuation" backslashes.
They are not needed.

thanks.
-- 
~Randy


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3] drivers/cdrom: improved ioctl for media change detection
@ 2021-09-10 21:16 Lukas Prediger
  2021-09-10 21:41 ` Randy Dunlap
  0 siblings, 1 reply; 12+ messages in thread
From: Lukas Prediger @ 2021-09-10 21:16 UTC (permalink / raw)
  To: phil; +Cc: axboe, linux-kernel, linux-block, hch, rdunlap, Lukas Prediger

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>
---
I have now incorporated the suggestions of Phil, Christoph and Randy for v2.
- fixed overly long lines
- has_changed is now media_flags in the new ioctl struct
- fixed brace on new line after struct definition
- using if instead of cryptic assignment for better readability

Hope everyone is happy with it now. However, if anything still comes up, let me know.
Thanks for all the feedback so far.

Best,
Lukas
---
 Documentation/cdrom/cdrom-standard.rst | 11 +++++
 Documentation/ioctl/cdrom.rst          |  3 ++
 drivers/cdrom/cdrom.c                  | 59 ++++++++++++++++++++++++--
 include/linux/cdrom.h                  |  1 +
 include/uapi/linux/cdrom.h             | 19 +++++++++
 5 files changed, 89 insertions(+), 4 deletions(-)

diff --git a/Documentation/cdrom/cdrom-standard.rst b/Documentation/cdrom/cdrom-standard.rst
index dde4f7f7fdbf..4d69515df864 100644
--- a/Documentation/cdrom/cdrom-standard.rst
+++ b/Documentation/cdrom/cdrom-standard.rst
@@ -923,6 +923,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/ioctl/cdrom.rst b/Documentation/ioctl/cdrom.rst
index 3b4c0506de46..bac5bbf93ca0 100644
--- a/Documentation/ioctl/cdrom.rst
+++ b/Documentation/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 ac42ae4651ce..0bc811a629e1 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)
 {
@@ -613,6 +619,7 @@ int register_cdrom(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;
@@ -1413,8 +1420,7 @@ static int cdrom_select_disc(struct cdrom_device_info *cdi, int slot)
 		cdi->ops->media_changed(cdi, 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);
 	}
 
@@ -1447,7 +1453,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;
 
@@ -1516,7 +1522,7 @@ int media_changed(struct cdrom_device_info *cdi, int queue)
 		changed = cdi->ops->media_changed(cdi, CDSL_CURRENT);
 
 	if (changed) {
-		cdi->mc_flags = 0x3;    /* set bit on both queues */
+		signal_media_change(cdi);
 		ret |= 1;
 		cdi->media_written = 0;
 	}
@@ -2389,6 +2395,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)
 {
@@ -3340,6 +3389,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 528271c60018..165d2c803670 100644
--- 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 2817230148fd..0c7043b5607f 100644
--- 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
  *******************************************************/
@@ -292,6 +294,23 @@ struct cdrom_generic_command
 	void			__user *reserved[1];	/* unused, actually */
 };
 
+/* 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.  
-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-10-07 23:05 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-13 23:09 [PATCH v3] drivers/cdrom: improved ioctl for media change detection Phillip Potter
2021-09-15  2:05 ` Jens Axboe
2021-09-16 23:07   ` Phillip Potter
  -- strict thread matches above, loose matches on Subject: below --
2021-09-10 21:16 Lukas Prediger
2021-09-10 21:41 ` Randy Dunlap
2021-09-11 11:00   ` Phillip Potter
2021-09-11 17:48     ` Lukas Prediger
2021-09-12 18:04       ` Phillip Potter
2021-09-12 19:12         ` Lukas Prediger
2021-09-13  7:50           ` Phillip Potter
2021-10-06 20:52           ` Randy Dunlap
2021-10-07 23:04             ` Phillip Potter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).