* [PATCH resend 1/6] delay: add poll_event_interruptible
2018-01-26 16:58 ` [PATCH resend 0/6] Fix cdrom autoclose Michal Suchanek
@ 2018-01-26 16:58 ` Michal Suchanek
2018-01-29 17:00 ` Bart Van Assche
2018-01-26 16:58 ` [PATCH resend 2/6] cdrom: factor out common open_for_* code Michal Suchanek
` (5 subsequent siblings)
6 siblings, 1 reply; 28+ messages in thread
From: Michal Suchanek @ 2018-01-26 16:58 UTC (permalink / raw)
To: Jens Axboe, Jonathan Corbet, Borislav Petkov, Tim Waugh,
David S. Miller, James E.J. Bottomley, Martin K. Petersen,
Michal Suchanek, Kees Cook, Christophe JAILLET, Thomas Gleixner,
Greg Kroah-Hartman, Kate Stewart, Philippe Ombredanne, linux-doc,
linux-kernel, linux-ide, linux-scsi
Add convenience macro for polling an event that does not have a
waitqueue.
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
include/linux/delay.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/include/linux/delay.h b/include/linux/delay.h
index b78bab4395d8..3ae9fa395628 100644
--- a/include/linux/delay.h
+++ b/include/linux/delay.h
@@ -64,4 +64,16 @@ static inline void ssleep(unsigned int seconds)
msleep(seconds * 1000);
}
+#define poll_event_interruptible(event, interval) ({ \
+ int ret = 0; \
+ while (!(event)) { \
+ if (signal_pending(current)) { \
+ ret = -ERESTARTSYS; \
+ break; \
+ } \
+ msleep_interruptible(interval); \
+ } \
+ ret; \
+})
+
#endif /* defined(_LINUX_DELAY_H) */
--
2.13.6
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH resend 1/6] delay: add poll_event_interruptible
2018-01-26 16:58 ` [PATCH resend 1/6] delay: add poll_event_interruptible Michal Suchanek
@ 2018-01-29 17:00 ` Bart Van Assche
0 siblings, 0 replies; 28+ messages in thread
From: Bart Van Assche @ 2018-01-29 17:00 UTC (permalink / raw)
To: corbet, linux-kernel, linux-ide, keescook, msuchanek, tglx,
martin.petersen, axboe, linux-scsi, kstewart, pombredanne, bp,
gregkh, jejb, christophe.jaillet, davem
On Fri, 2018-01-26 at 17:58 +0100, Michal Suchanek wrote:
> Add convenience macro for polling an event that does not have a
> waitqueue.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> include/linux/delay.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/linux/delay.h b/include/linux/delay.h
> index b78bab4395d8..3ae9fa395628 100644
> --- a/include/linux/delay.h
> +++ b/include/linux/delay.h
> @@ -64,4 +64,16 @@ static inline void ssleep(unsigned int seconds)
> msleep(seconds * 1000);
> }
>
> +#define poll_event_interruptible(event, interval) ({ \
> + int ret = 0; \
> + while (!(event)) { \
> + if (signal_pending(current)) { \
> + ret = -ERESTARTSYS; \
> + break; \
> + } \
> + msleep_interruptible(interval); \
> + } \
> + ret; \
> +})
> +
> #endif /* defined(_LINUX_DELAY_H) */
Sorry but I'm not sure we should encourage other kernel developers to use
busy-waiting by adding the poll_event_interruptible() macro to a system-wide
header file. Can that macro be moved into a CDROM-specific .c or .h file?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH resend 1/6] delay: add poll_event_interruptible
@ 2018-01-29 17:00 ` Bart Van Assche
0 siblings, 0 replies; 28+ messages in thread
From: Bart Van Assche @ 2018-01-29 17:00 UTC (permalink / raw)
To: corbet, linux-kernel, linux-ide, keescook, msuchanek, tglx,
martin.petersen, axboe, linux-scsi, kstewart, pombredanne, bp,
gregkh, jejb, christophe.jaillet, davem, linux-doc, tim
On Fri, 2018-01-26 at 17:58 +0100, Michal Suchanek wrote:
> Add convenience macro for polling an event that does not have a
> waitqueue.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> include/linux/delay.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/linux/delay.h b/include/linux/delay.h
> index b78bab4395d8..3ae9fa395628 100644
> --- a/include/linux/delay.h
> +++ b/include/linux/delay.h
> @@ -64,4 +64,16 @@ static inline void ssleep(unsigned int seconds)
> msleep(seconds * 1000);
> }
>
> +#define poll_event_interruptible(event, interval) ({ \
> + int ret = 0; \
> + while (!(event)) { \
> + if (signal_pending(current)) { \
> + ret = -ERESTARTSYS; \
> + break; \
> + } \
> + msleep_interruptible(interval); \
> + } \
> + ret; \
> +})
> +
> #endif /* defined(_LINUX_DELAY_H) */
Sorry but I'm not sure we should encourage other kernel developers to use
busy-waiting by adding the poll_event_interruptible() macro to a system-wide
header file. Can that macro be moved into a CDROM-specific .c or .h file?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH resend 2/6] cdrom: factor out common open_for_* code
2018-01-26 16:58 ` [PATCH resend 0/6] Fix cdrom autoclose Michal Suchanek
2018-01-26 16:58 ` [PATCH resend 1/6] delay: add poll_event_interruptible Michal Suchanek
@ 2018-01-26 16:58 ` Michal Suchanek
2018-01-29 17:02 ` Bart Van Assche
2018-01-26 16:58 ` [PATCH resend 3/6] cdrom: wait for tray to close Michal Suchanek
` (4 subsequent siblings)
6 siblings, 1 reply; 28+ messages in thread
From: Michal Suchanek @ 2018-01-26 16:58 UTC (permalink / raw)
To: Jens Axboe, Jonathan Corbet, Borislav Petkov, Tim Waugh,
David S. Miller, James E.J. Bottomley, Martin K. Petersen,
Michal Suchanek, Kees Cook, Christophe JAILLET, Thomas Gleixner,
Greg Kroah-Hartman, Kate Stewart, Philippe Ombredanne, linux-doc,
linux-kernel, linux-ide, linux-scsi
The open_for_audio and open_for_data copies are bitrotten in different
ways already and will need to update the autoclose logic in both.
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
drivers/cdrom/cdrom.c | 100 ++++++++++++++++++--------------------------------
1 file changed, 36 insertions(+), 64 deletions(-)
diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index e36d160c458f..89746b3d193f 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -1031,12 +1031,12 @@ static void cdrom_count_tracks(struct cdrom_device_info *cdi, tracktype *tracks)
}
static
-int open_for_data(struct cdrom_device_info *cdi)
+int open_for_common(struct cdrom_device_info *cdi, tracktype *tracks)
{
int ret;
const struct cdrom_device_ops *cdo = cdi->ops;
- tracktype tracks;
- cd_dbg(CD_OPEN, "entering open_for_data\n");
+
+ cd_dbg(CD_OPEN, "entering open_for_common\n");
/* Check if the driver can report drive status. If it can, we
can do clever things. If it can't, well, we at least tried! */
if (cdo->drive_status != NULL) {
@@ -1048,7 +1048,7 @@ int open_for_data(struct cdrom_device_info *cdi)
if (CDROM_CAN(CDC_CLOSE_TRAY) &&
cdi->options & CDO_AUTO_CLOSE) {
cd_dbg(CD_OPEN, "trying to close the tray\n");
- ret=cdo->tray_move(cdi,0);
+ ret = cdo->tray_move(cdi, 0);
if (ret) {
cd_dbg(CD_OPEN, "bummer. tried to close the tray but failed.\n");
/* Ignore the error from the low
@@ -1056,37 +1056,45 @@ int open_for_data(struct cdrom_device_info *cdi)
couldn't close the tray. We only care
that there is no disc in the drive,
since that is the _REAL_ problem here.*/
- ret=-ENOMEDIUM;
- goto clean_up_and_return;
+ return -ENOMEDIUM;
}
} else {
cd_dbg(CD_OPEN, "bummer. this drive can't close the tray.\n");
- ret=-ENOMEDIUM;
- goto clean_up_and_return;
+ return -ENOMEDIUM;
}
/* Ok, the door should be closed now.. Check again */
ret = cdo->drive_status(cdi, CDSL_CURRENT);
- if ((ret == CDS_NO_DISC) || (ret==CDS_TRAY_OPEN)) {
+ if ((ret == CDS_NO_DISC) || (ret == CDS_TRAY_OPEN)) {
cd_dbg(CD_OPEN, "bummer. the tray is still not closed.\n");
cd_dbg(CD_OPEN, "tray might not contain a medium\n");
- ret=-ENOMEDIUM;
- goto clean_up_and_return;
+ return -ENOMEDIUM;
}
cd_dbg(CD_OPEN, "the tray is now closed\n");
}
- /* the door should be closed now, check for the disc */
- ret = cdo->drive_status(cdi, CDSL_CURRENT);
- if (ret!=CDS_DISC_OK) {
- ret = -ENOMEDIUM;
- goto clean_up_and_return;
- }
+ if (ret != CDS_DISC_OK)
+ return -ENOMEDIUM;
}
- cdrom_count_tracks(cdi, &tracks);
- if (tracks.error == CDS_NO_DISC) {
+ cdrom_count_tracks(cdi, tracks);
+ if (tracks->error == CDS_NO_DISC) {
cd_dbg(CD_OPEN, "bummer. no disc.\n");
- ret=-ENOMEDIUM;
- goto clean_up_and_return;
+ return -ENOMEDIUM;
}
+
+ return 0;
+}
+
+static
+int open_for_data(struct cdrom_device_info *cdi)
+{
+ int ret;
+ const struct cdrom_device_ops *cdo = cdi->ops;
+ tracktype tracks;
+
+ cd_dbg(CD_OPEN, "entering open_for_data\n");
+ ret = open_for_common(cdi, &tracks);
+ if (ret)
+ goto clean_up_and_return;
+
/* CD-Players which don't use O_NONBLOCK, workman
* for example, need bit CDO_CHECK_TYPE cleared! */
if (tracks.data==0) {
@@ -1196,53 +1204,17 @@ int cdrom_open(struct cdrom_device_info *cdi, struct block_device *bdev,
/* This code is similar to that in open_for_data. The routine is called
whenever an audio play operation is requested.
*/
-static int check_for_audio_disc(struct cdrom_device_info *cdi,
- const struct cdrom_device_ops *cdo)
+static int check_for_audio_disc(struct cdrom_device_info *cdi)
{
int ret;
tracktype tracks;
cd_dbg(CD_OPEN, "entering check_for_audio_disc\n");
if (!(cdi->options & CDO_CHECK_TYPE))
return 0;
- if (cdo->drive_status != NULL) {
- ret = cdo->drive_status(cdi, CDSL_CURRENT);
- cd_dbg(CD_OPEN, "drive_status=%d\n", ret);
- if (ret == CDS_TRAY_OPEN) {
- cd_dbg(CD_OPEN, "the tray is open...\n");
- /* can/may i close it? */
- if (CDROM_CAN(CDC_CLOSE_TRAY) &&
- cdi->options & CDO_AUTO_CLOSE) {
- cd_dbg(CD_OPEN, "trying to close the tray\n");
- ret=cdo->tray_move(cdi,0);
- if (ret) {
- cd_dbg(CD_OPEN, "bummer. tried to close tray but failed.\n");
- /* Ignore the error from the low
- level driver. We don't care why it
- couldn't close the tray. We only care
- that there is no disc in the drive,
- since that is the _REAL_ problem here.*/
- return -ENOMEDIUM;
- }
- } else {
- cd_dbg(CD_OPEN, "bummer. this driver can't close the tray.\n");
- return -ENOMEDIUM;
- }
- /* Ok, the door should be closed now.. Check again */
- ret = cdo->drive_status(cdi, CDSL_CURRENT);
- if ((ret == CDS_NO_DISC) || (ret==CDS_TRAY_OPEN)) {
- cd_dbg(CD_OPEN, "bummer. the tray is still not closed.\n");
- return -ENOMEDIUM;
- }
- if (ret!=CDS_DISC_OK) {
- cd_dbg(CD_OPEN, "bummer. disc isn't ready.\n");
- return -EIO;
- }
- cd_dbg(CD_OPEN, "the tray is now closed\n");
- }
- }
- cdrom_count_tracks(cdi, &tracks);
- if (tracks.error)
- return(tracks.error);
+
+ ret = open_for_common(cdi, &tracks);
+ if (ret)
+ return ret;
if (tracks.audio==0)
return -EMEDIUMTYPE;
@@ -2710,7 +2682,7 @@ static int cdrom_ioctl_play_trkind(struct cdrom_device_info *cdi,
if (copy_from_user(&ti, argp, sizeof(ti)))
return -EFAULT;
- ret = check_for_audio_disc(cdi, cdi->ops);
+ ret = check_for_audio_disc(cdi);
if (ret)
return ret;
return cdi->ops->audio_ioctl(cdi, CDROMPLAYTRKIND, &ti);
@@ -2758,7 +2730,7 @@ static int cdrom_ioctl_audioctl(struct cdrom_device_info *cdi,
if (!CDROM_CAN(CDC_PLAY_AUDIO))
return -ENOSYS;
- ret = check_for_audio_disc(cdi, cdi->ops);
+ ret = check_for_audio_disc(cdi);
if (ret)
return ret;
return cdi->ops->audio_ioctl(cdi, cmd, NULL);
--
2.13.6
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH resend 2/6] cdrom: factor out common open_for_* code
2018-01-26 16:58 ` [PATCH resend 2/6] cdrom: factor out common open_for_* code Michal Suchanek
@ 2018-01-29 17:02 ` Bart Van Assche
0 siblings, 0 replies; 28+ messages in thread
From: Bart Van Assche @ 2018-01-29 17:02 UTC (permalink / raw)
To: corbet, linux-kernel, linux-ide, keescook, msuchanek, tglx,
martin.petersen, axboe, linux-scsi, kstewart, pombredanne, bp,
gregkh, jejb, christophe.jaillet, davem
On Fri, 2018-01-26 at 17:58 +0100, Michal Suchanek wrote:
> - ret=cdo->tray_move(cdi,0);
> + ret = cdo->tray_move(cdi, 0);
Please separate whitespace-only changes from functional changes such that
this patch series becomes easier to review.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH resend 2/6] cdrom: factor out common open_for_* code
@ 2018-01-29 17:02 ` Bart Van Assche
0 siblings, 0 replies; 28+ messages in thread
From: Bart Van Assche @ 2018-01-29 17:02 UTC (permalink / raw)
To: corbet, linux-kernel, linux-ide, keescook, msuchanek, tglx,
martin.petersen, axboe, linux-scsi, kstewart, pombredanne, bp,
gregkh, jejb, christophe.jaillet, davem, linux-doc, tim
On Fri, 2018-01-26 at 17:58 +0100, Michal Suchanek wrote:
> - ret=cdo->tray_move(cdi,0);
> + ret = cdo->tray_move(cdi, 0);
Please separate whitespace-only changes from functional changes such that
this patch series becomes easier to review.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH resend 3/6] cdrom: wait for tray to close
2018-01-26 16:58 ` [PATCH resend 0/6] Fix cdrom autoclose Michal Suchanek
2018-01-26 16:58 ` [PATCH resend 1/6] delay: add poll_event_interruptible Michal Suchanek
2018-01-26 16:58 ` [PATCH resend 2/6] cdrom: factor out common open_for_* code Michal Suchanek
@ 2018-01-26 16:58 ` Michal Suchanek
2018-01-29 17:05 ` Bart Van Assche
2018-01-26 16:58 ` [PATCH resend 4/6] cdrom: introduce CDS_DRIVE_ERROR Michal Suchanek
` (3 subsequent siblings)
6 siblings, 1 reply; 28+ messages in thread
From: Michal Suchanek @ 2018-01-26 16:58 UTC (permalink / raw)
To: Jens Axboe, Jonathan Corbet, Borislav Petkov, Tim Waugh,
David S. Miller, James E.J. Bottomley, Martin K. Petersen,
Michal Suchanek, Kees Cook, Christophe JAILLET, Thomas Gleixner,
Greg Kroah-Hartman, Kate Stewart, Philippe Ombredanne, linux-doc,
linux-kernel, linux-ide, linux-scsi
The scsi command to close tray only starts the motor and does not wait
for the tray to close. Wait until the state chages from TRAY_OPEN so
users do not race with the tray closing.
This looks like inifinte wait but unless the drive is broken it either
closes the tray within a few seconds or reports an error when it detects
the tray is blocked. At worst the wait can be interrupted by user.
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v2:
- check drive_status exists before using it
- rename tray_close -> cdrom_tray_close
---
drivers/cdrom/cdrom.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index 89746b3d193f..69e85c902373 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -281,7 +281,9 @@
#include <linux/fcntl.h>
#include <linux/blkdev.h>
#include <linux/times.h>
+#include <linux/delay.h>
#include <linux/uaccess.h>
+#include <linux/sched/signal.h>
#include <scsi/scsi_request.h>
/* used to tell the module to turn on full debugging messages */
@@ -1030,6 +1032,18 @@ static void cdrom_count_tracks(struct cdrom_device_info *cdi, tracktype *tracks)
tracks->cdi, tracks->xa);
}
+static int cdrom_tray_close(struct cdrom_device_info *cdi)
+{
+ int ret;
+
+ ret = cdi->ops->tray_move(cdi, 0);
+ if (ret || !cdi->ops->drive_status)
+ return ret;
+
+ return poll_event_interruptible(CDS_TRAY_OPEN !=
+ cdi->ops->drive_status(cdi, CDSL_CURRENT), 500);
+}
+
static
int open_for_common(struct cdrom_device_info *cdi, tracktype *tracks)
{
@@ -1048,7 +1062,9 @@ int open_for_common(struct cdrom_device_info *cdi, tracktype *tracks)
if (CDROM_CAN(CDC_CLOSE_TRAY) &&
cdi->options & CDO_AUTO_CLOSE) {
cd_dbg(CD_OPEN, "trying to close the tray\n");
- ret = cdo->tray_move(cdi, 0);
+ ret = cdrom_tray_close(cdi);
+ if (ret == -ERESTARTSYS)
+ return ret;
if (ret) {
cd_dbg(CD_OPEN, "bummer. tried to close the tray but failed.\n");
/* Ignore the error from the low
@@ -2312,7 +2328,8 @@ static int cdrom_ioctl_closetray(struct cdrom_device_info *cdi)
if (!CDROM_CAN(CDC_CLOSE_TRAY))
return -ENOSYS;
- return cdi->ops->tray_move(cdi, 0);
+
+ return cdrom_tray_close(cdi);
}
static int cdrom_ioctl_eject_sw(struct cdrom_device_info *cdi,
--
2.13.6
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH resend 3/6] cdrom: wait for tray to close
2018-01-26 16:58 ` [PATCH resend 3/6] cdrom: wait for tray to close Michal Suchanek
@ 2018-01-29 17:05 ` Bart Van Assche
0 siblings, 0 replies; 28+ messages in thread
From: Bart Van Assche @ 2018-01-29 17:05 UTC (permalink / raw)
To: corbet, linux-kernel, linux-ide, keescook, msuchanek, tglx,
martin.petersen, axboe, linux-scsi, kstewart, pombredanne, bp,
gregkh, jejb, christophe.jaillet, davem
On Fri, 2018-01-26 at 17:58 +0100, Michal Suchanek wrote:
> +static int cdrom_tray_close(struct cdrom_device_info *cdi)
> +{
> + int ret;
> +
> + ret = cdi->ops->tray_move(cdi, 0);
> + if (ret || !cdi->ops->drive_status)
> + return ret;
> +
> + return poll_event_interruptible(CDS_TRAY_OPEN !=
> + cdi->ops->drive_status(cdi, CDSL_CURRENT), 500);
> +}
> +
> static
> int open_for_common(struct cdrom_device_info *cdi, tracktype *tracks)
> {
> @@ -1048,7 +1062,9 @@ int open_for_common(struct cdrom_device_info *cdi, tracktype *tracks)
> if (CDROM_CAN(CDC_CLOSE_TRAY) &&
> cdi->options & CDO_AUTO_CLOSE) {
> cd_dbg(CD_OPEN, "trying to close the tray\n");
> - ret = cdo->tray_move(cdi, 0);
> + ret = cdrom_tray_close(cdi);
> + if (ret == -ERESTARTSYS)
> + return ret;
> if (ret) {
> cd_dbg(CD_OPEN, "bummer. tried to close the tray but failed.\n");
> /* Ignore the error from the low
> @@ -2312,7 +2328,8 @@ static int cdrom_ioctl_closetray(struct cdrom_device_info *cdi)
>
> if (!CDROM_CAN(CDC_CLOSE_TRAY))
> return -ENOSYS;
> - return cdi->ops->tray_move(cdi, 0);
> +
> + return cdrom_tray_close(cdi);
> }
So this patch changes code that does not wait into code that potentially waits
forever? Sorry but I don't think that's ideal. Please make sure that after a
certain time (a few seconds?) the loop finishes.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH resend 3/6] cdrom: wait for tray to close
@ 2018-01-29 17:05 ` Bart Van Assche
0 siblings, 0 replies; 28+ messages in thread
From: Bart Van Assche @ 2018-01-29 17:05 UTC (permalink / raw)
To: corbet, linux-kernel, linux-ide, keescook, msuchanek, tglx,
martin.petersen, axboe, linux-scsi, kstewart, pombredanne, bp,
gregkh, jejb, christophe.jaillet, davem, linux-doc, tim
On Fri, 2018-01-26 at 17:58 +0100, Michal Suchanek wrote:
> +static int cdrom_tray_close(struct cdrom_device_info *cdi)
> +{
> + int ret;
> +
> + ret = cdi->ops->tray_move(cdi, 0);
> + if (ret || !cdi->ops->drive_status)
> + return ret;
> +
> + return poll_event_interruptible(CDS_TRAY_OPEN !=
> + cdi->ops->drive_status(cdi, CDSL_CURRENT), 500);
> +}
> +
> static
> int open_for_common(struct cdrom_device_info *cdi, tracktype *tracks)
> {
> @@ -1048,7 +1062,9 @@ int open_for_common(struct cdrom_device_info *cdi, tracktype *tracks)
> if (CDROM_CAN(CDC_CLOSE_TRAY) &&
> cdi->options & CDO_AUTO_CLOSE) {
> cd_dbg(CD_OPEN, "trying to close the tray\n");
> - ret = cdo->tray_move(cdi, 0);
> + ret = cdrom_tray_close(cdi);
> + if (ret == -ERESTARTSYS)
> + return ret;
> if (ret) {
> cd_dbg(CD_OPEN, "bummer. tried to close the tray but failed.\n");
> /* Ignore the error from the low
> @@ -2312,7 +2328,8 @@ static int cdrom_ioctl_closetray(struct cdrom_device_info *cdi)
>
> if (!CDROM_CAN(CDC_CLOSE_TRAY))
> return -ENOSYS;
> - return cdi->ops->tray_move(cdi, 0);
> +
> + return cdrom_tray_close(cdi);
> }
So this patch changes code that does not wait into code that potentially waits
forever? Sorry but I don't think that's ideal. Please make sure that after a
certain time (a few seconds?) the loop finishes.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH resend 3/6] cdrom: wait for tray to close
2018-01-29 17:05 ` Bart Van Assche
@ 2018-01-31 18:20 ` Michal Suchánek
-1 siblings, 0 replies; 28+ messages in thread
From: Michal Suchánek @ 2018-01-31 18:20 UTC (permalink / raw)
To: Bart Van Assche
Cc: corbet, linux-kernel, linux-ide, keescook, tglx, martin.petersen,
axboe, linux-scsi, kstewart, pombredanne, bp, gregkh, jejb,
christophe.jaillet, davem, linux-doc
On Mon, 29 Jan 2018 17:05:47 +0000
Bart Van Assche <Bart.VanAssche@wdc.com> wrote:
> On Fri, 2018-01-26 at 17:58 +0100, Michal Suchanek wrote:
> > +static int cdrom_tray_close(struct cdrom_device_info *cdi)
> > +{
> > + int ret;
> > +
> > + ret = cdi->ops->tray_move(cdi, 0);
> > + if (ret || !cdi->ops->drive_status)
> > + return ret;
> > +
> > + return poll_event_interruptible(CDS_TRAY_OPEN !=
> > + cdi->ops->drive_status(cdi, CDSL_CURRENT),
> > 500); +}
> > +
> > static
> > int open_for_common(struct cdrom_device_info *cdi, tracktype
> > *tracks) {
> > @@ -1048,7 +1062,9 @@ int open_for_common(struct cdrom_device_info
> > *cdi, tracktype *tracks) if (CDROM_CAN(CDC_CLOSE_TRAY) &&
> > cdi->options & CDO_AUTO_CLOSE) {
> > cd_dbg(CD_OPEN, "trying to close
> > the tray\n");
> > - ret = cdo->tray_move(cdi, 0);
> > + ret = cdrom_tray_close(cdi);
> > + if (ret == -ERESTARTSYS)
> > + return ret;
> > if (ret) {
> > cd_dbg(CD_OPEN, "bummer.
> > tried to close the tray but failed.\n"); /* Ignore the error from
> > the low @@ -2312,7 +2328,8 @@ static int
> > cdrom_ioctl_closetray(struct cdrom_device_info *cdi)
> > if (!CDROM_CAN(CDC_CLOSE_TRAY))
> > return -ENOSYS;
> > - return cdi->ops->tray_move(cdi, 0);
> > +
> > + return cdrom_tray_close(cdi);
> > }
>
> So this patch changes code that does not wait into code that
> potentially waits forever? Sorry but I don't think that's ideal.
> Please make sure that after a certain time (a few seconds?) the loop
> finishes.
The problem is that few seconds does not cut it. We are waiting for a
mechanical tray or CD changer to move. On non-broken hardware the tray
either closes or an error is reported within a few seconds. For the
timeout to not race with the event we are waiting for it must be much
longer, though.
Also note that this code is only invoked when the user specifically
requested that the tray gets closed automatically which is off by
default.
Thanks
Michal
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH resend 3/6] cdrom: wait for tray to close
@ 2018-01-31 18:20 ` Michal Suchánek
0 siblings, 0 replies; 28+ messages in thread
From: Michal Suchánek @ 2018-01-31 18:20 UTC (permalink / raw)
To: Bart Van Assche
Cc: corbet, linux-kernel, linux-ide, keescook, tglx, martin.petersen,
axboe, linux-scsi, kstewart, pombredanne, bp, gregkh, jejb,
christophe.jaillet, davem, linux-doc, tim
On Mon, 29 Jan 2018 17:05:47 +0000
Bart Van Assche <Bart.VanAssche@wdc.com> wrote:
> On Fri, 2018-01-26 at 17:58 +0100, Michal Suchanek wrote:
> > +static int cdrom_tray_close(struct cdrom_device_info *cdi)
> > +{
> > + int ret;
> > +
> > + ret = cdi->ops->tray_move(cdi, 0);
> > + if (ret || !cdi->ops->drive_status)
> > + return ret;
> > +
> > + return poll_event_interruptible(CDS_TRAY_OPEN !=
> > + cdi->ops->drive_status(cdi, CDSL_CURRENT),
> > 500); +}
> > +
> > static
> > int open_for_common(struct cdrom_device_info *cdi, tracktype
> > *tracks) {
> > @@ -1048,7 +1062,9 @@ int open_for_common(struct cdrom_device_info
> > *cdi, tracktype *tracks) if (CDROM_CAN(CDC_CLOSE_TRAY) &&
> > cdi->options & CDO_AUTO_CLOSE) {
> > cd_dbg(CD_OPEN, "trying to close
> > the tray\n");
> > - ret = cdo->tray_move(cdi, 0);
> > + ret = cdrom_tray_close(cdi);
> > + if (ret == -ERESTARTSYS)
> > + return ret;
> > if (ret) {
> > cd_dbg(CD_OPEN, "bummer.
> > tried to close the tray but failed.\n"); /* Ignore the error from
> > the low @@ -2312,7 +2328,8 @@ static int
> > cdrom_ioctl_closetray(struct cdrom_device_info *cdi)
> > if (!CDROM_CAN(CDC_CLOSE_TRAY))
> > return -ENOSYS;
> > - return cdi->ops->tray_move(cdi, 0);
> > +
> > + return cdrom_tray_close(cdi);
> > }
>
> So this patch changes code that does not wait into code that
> potentially waits forever? Sorry but I don't think that's ideal.
> Please make sure that after a certain time (a few seconds?) the loop
> finishes.
The problem is that few seconds does not cut it. We are waiting for a
mechanical tray or CD changer to move. On non-broken hardware the tray
either closes or an error is reported within a few seconds. For the
timeout to not race with the event we are waiting for it must be much
longer, though.
Also note that this code is only invoked when the user specifically
requested that the tray gets closed automatically which is off by
default.
Thanks
Michal
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH resend 3/6] cdrom: wait for tray to close
2018-01-29 17:05 ` Bart Van Assche
(?)
(?)
@ 2019-10-23 12:19 ` Michal Suchánek
-1 siblings, 0 replies; 28+ messages in thread
From: Michal Suchánek @ 2019-10-23 12:19 UTC (permalink / raw)
To: Bart Van Assche
Cc: corbet, linux-kernel, linux-ide, keescook, tglx, martin.petersen,
axboe, linux-scsi, kstewart, pombredanne, bp, gregkh, jejb,
christophe.jaillet, davem, linux-doc, tim
On Mon, Jan 29, 2018 at 05:05:47PM +0000, Bart Van Assche wrote:
> On Fri, 2018-01-26 at 17:58 +0100, Michal Suchanek wrote:
> > +static int cdrom_tray_close(struct cdrom_device_info *cdi)
> > +{
> > + int ret;
> > +
> > + ret = cdi->ops->tray_move(cdi, 0);
> > + if (ret || !cdi->ops->drive_status)
> > + return ret;
> > +
> > + return poll_event_interruptible(CDS_TRAY_OPEN !=
> > + cdi->ops->drive_status(cdi, CDSL_CURRENT), 500);
> > +}
> > +
> > static
> > int open_for_common(struct cdrom_device_info *cdi, tracktype *tracks)
> > {
> > @@ -1048,7 +1062,9 @@ int open_for_common(struct cdrom_device_info *cdi, tracktype *tracks)
> > if (CDROM_CAN(CDC_CLOSE_TRAY) &&
> > cdi->options & CDO_AUTO_CLOSE) {
> > cd_dbg(CD_OPEN, "trying to close the tray\n");
> > - ret = cdo->tray_move(cdi, 0);
> > + ret = cdrom_tray_close(cdi);
> > + if (ret == -ERESTARTSYS)
> > + return ret;
> > if (ret) {
> > cd_dbg(CD_OPEN, "bummer. tried to close the tray but failed.\n");
> > /* Ignore the error from the low
> > @@ -2312,7 +2328,8 @@ static int cdrom_ioctl_closetray(struct cdrom_device_info *cdi)
> >
> > if (!CDROM_CAN(CDC_CLOSE_TRAY))
> > return -ENOSYS;
> > - return cdi->ops->tray_move(cdi, 0);
> > +
> > + return cdrom_tray_close(cdi);
> > }
>
> So this patch changes code that does not wait into code that potentially waits
> forever? Sorry but I don't think that's ideal. Please make sure that after a
> certain time (a few seconds?) the loop finishes.
Unfortunately, a few seconds is NOT sufficinet. I have no idea what is
the upper bound on the time it can take to close the tray taking into
account all hardware implementations like media changers. For the usual
desktop units it takes tens of seconds so you would need to wait minutes
to give some headroom - basically near-eternity.
Thanks
Michal
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH resend 4/6] cdrom: introduce CDS_DRIVE_ERROR
2018-01-26 16:58 ` [PATCH resend 0/6] Fix cdrom autoclose Michal Suchanek
` (2 preceding siblings ...)
2018-01-26 16:58 ` [PATCH resend 3/6] cdrom: wait for tray to close Michal Suchanek
@ 2018-01-26 16:58 ` Michal Suchanek
2018-01-26 16:58 ` [PATCH resend 5/6] Documentetion: " Michal Suchanek
` (2 subsequent siblings)
6 siblings, 0 replies; 28+ messages in thread
From: Michal Suchanek @ 2018-01-26 16:58 UTC (permalink / raw)
To: Jens Axboe, Jonathan Corbet, Borislav Petkov, Tim Waugh,
David S. Miller, James E.J. Bottomley, Martin K. Petersen,
Michal Suchanek, Kees Cook, Christophe JAILLET, Thomas Gleixner,
Greg Kroah-Hartman, Kate Stewart, Philippe Ombredanne, linux-doc,
linux-kernel, linux-ide, linux-scsi
CDS_DRIVE_NOT_READY is used for the state in which CDROM is 'becoming
ready' (typically analyzing the disc) but also as the fallback when
nothing else applies. Introduce CDS_DRIVE_ERROR for the fallback case.
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
drivers/block/paride/pcd.c | 2 +-
drivers/cdrom/gdrom.c | 2 +-
drivers/ide/ide-cd_ioctl.c | 12 ++++++++----
drivers/scsi/sr_ioctl.c | 2 +-
include/uapi/linux/cdrom.h | 1 +
5 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/block/paride/pcd.c b/drivers/block/paride/pcd.c
index 7b8c6368beb7..6e00093ff34e 100644
--- a/drivers/block/paride/pcd.c
+++ b/drivers/block/paride/pcd.c
@@ -605,7 +605,7 @@ static int pcd_drive_status(struct cdrom_device_info *cdi, int slot_nr)
struct pcd_unit *cd = cdi->handle;
if (pcd_ready_wait(cd, PCD_READY_TMO))
- return CDS_DRIVE_NOT_READY;
+ return CDS_DRIVE_ERROR;
if (pcd_atapi(cd, rc_cmd, 8, pcd_scratch, DBMSG("check media")))
return CDS_NO_DISC;
return CDS_DISC_OK;
diff --git a/drivers/cdrom/gdrom.c b/drivers/cdrom/gdrom.c
index 6495b03f576c..702f255bbe42 100644
--- a/drivers/cdrom/gdrom.c
+++ b/drivers/cdrom/gdrom.c
@@ -390,7 +390,7 @@ static int gdrom_drivestatus(struct cdrom_device_info *cd_info, int ignore)
if (sense == 0)
return CDS_DISC_OK;
if (sense == 0x20)
- return CDS_DRIVE_NOT_READY;
+ return CDS_DRIVE_ERROR;
/* default */
return CDS_NO_INFO;
}
diff --git a/drivers/ide/ide-cd_ioctl.c b/drivers/ide/ide-cd_ioctl.c
index 2acca12b9c94..9a26f50a2092 100644
--- a/drivers/ide/ide-cd_ioctl.c
+++ b/drivers/ide/ide-cd_ioctl.c
@@ -62,9 +62,13 @@ int ide_cdrom_drive_status(struct cdrom_device_info *cdi, int slot_nr)
return CDS_NO_DISC;
}
- if (sense.sense_key == NOT_READY && sense.asc == 0x04
- && sense.ascq == 0x04)
- return CDS_DISC_OK;
+ if (sense.sense_key == NOT_READY && sense.asc == 0x04)
+ switch (sense.ascq) {
+ case 0x01:
+ return CDS_DRIVE_NOT_READY;
+ case 0x04:
+ return CDS_DISC_OK;
+ }
/*
* If not using Mt Fuji extended media tray reports,
@@ -77,7 +81,7 @@ int ide_cdrom_drive_status(struct cdrom_device_info *cdi, int slot_nr)
else
return CDS_TRAY_OPEN;
}
- return CDS_DRIVE_NOT_READY;
+ return CDS_DRIVE_ERROR;
}
/*
diff --git a/drivers/scsi/sr_ioctl.c b/drivers/scsi/sr_ioctl.c
index 2a21f2d48592..7c93f12a9cb8 100644
--- a/drivers/scsi/sr_ioctl.c
+++ b/drivers/scsi/sr_ioctl.c
@@ -333,7 +333,7 @@ int sr_drive_status(struct cdrom_device_info *cdi, int slot)
else
return CDS_TRAY_OPEN;
- return CDS_DRIVE_NOT_READY;
+ return CDS_DRIVE_ERROR;
}
int sr_disk_status(struct cdrom_device_info *cdi)
diff --git a/include/uapi/linux/cdrom.h b/include/uapi/linux/cdrom.h
index 2817230148fd..339b1435f44e 100644
--- a/include/uapi/linux/cdrom.h
+++ b/include/uapi/linux/cdrom.h
@@ -398,6 +398,7 @@ struct cdrom_generic_command
#define CDS_TRAY_OPEN 2
#define CDS_DRIVE_NOT_READY 3
#define CDS_DISC_OK 4
+#define CDS_DRIVE_ERROR 5
/* return values for the CDROM_DISC_STATUS ioctl */
/* can also return CDS_NO_[INFO|DISC], from above */
--
2.13.6
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH resend 5/6] Documentetion: cdrom: introduce CDS_DRIVE_ERROR
2018-01-26 16:58 ` [PATCH resend 0/6] Fix cdrom autoclose Michal Suchanek
` (3 preceding siblings ...)
2018-01-26 16:58 ` [PATCH resend 4/6] cdrom: introduce CDS_DRIVE_ERROR Michal Suchanek
@ 2018-01-26 16:58 ` Michal Suchanek
2018-01-26 16:58 ` [PATCH resend 6/6] cdrom: wait for drive to become ready Michal Suchanek
2018-01-26 20:04 ` [PATCH resend 0/6] Fix cdrom autoclose James Bottomley
6 siblings, 0 replies; 28+ messages in thread
From: Michal Suchanek @ 2018-01-26 16:58 UTC (permalink / raw)
To: Jens Axboe, Jonathan Corbet, Borislav Petkov, Tim Waugh,
David S. Miller, James E.J. Bottomley, Martin K. Petersen,
Michal Suchanek, Kees Cook, Christophe JAILLET, Thomas Gleixner,
Greg Kroah-Hartman, Kate Stewart, Philippe Ombredanne, linux-doc,
linux-kernel, linux-ide, linux-scsi
CDS_DRIVE_NOT_READY is used for the state in which CDROM is 'becoming
ready' (typically analyzing the disc) but also as the fallback when
nothing else applies. Introduce CDS_DRIVE_ERROR for the fallback case.
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
Documentation/cdrom/cdrom-standard.tex | 8 +++++++-
Documentation/cdrom/ide-cd | 6 ++++++
Documentation/ioctl/cdrom.txt | 1 +
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/Documentation/cdrom/cdrom-standard.tex b/Documentation/cdrom/cdrom-standard.tex
index 8f85b0e41046..018284ba696a 100644
--- a/Documentation/cdrom/cdrom-standard.tex
+++ b/Documentation/cdrom/cdrom-standard.tex
@@ -371,11 +371,17 @@ $$
CDS_NO_INFO& no information available\cr
CDS_NO_DISC& no disc is inserted, tray is closed\cr
CDS_TRAY_OPEN& tray is opened\cr
-CDS_DRIVE_NOT_READY& something is wrong, tray is moving?\cr
+CDS_DRIVE_NOT_READY& tray just closed?\cr
CDS_DISC_OK& a disc is loaded and everything is fine\cr
+CDS_DRIVE_ERROR& something is wrong\cr
}
$$
+Note: The IDE and SCSI cdroms have a status code 'drive becoming ready' which
+is typically returned when the drive has just closed and is analyzing the disc.
+For other cdrom types this state is not reported by the hardware or not
+implemented by the driver.
+
\subsection{$Int\ media_changed(struct\ cdrom_device_info * cdi, int\ disc_nr)$}
This function is very similar to the original function in $struct\
diff --git a/Documentation/cdrom/ide-cd b/Documentation/cdrom/ide-cd
index a5f2a7f1ff46..9324a8fd9a39 100644
--- a/Documentation/cdrom/ide-cd
+++ b/Documentation/cdrom/ide-cd
@@ -455,6 +455,9 @@ main (int argc, char **argv)
case CDS_DRIVE_NOT_READY:
printf ("Drive Not Ready.\n");
break;
+ case CDS_DRIVE_ERROR:
+ printf ("Drive problem.\n");
+ break;
default:
printf ("This Should not happen!\n");
break;
@@ -481,6 +484,9 @@ main (int argc, char **argv)
case CDS_NO_INFO:
printf ("No Information available.");
break;
+ case CDS_DRIVE_ERROR:
+ printf ("Drive problem.\n");
+ break;
default:
printf ("This Should not happen!\n");
break;
diff --git a/Documentation/ioctl/cdrom.txt b/Documentation/ioctl/cdrom.txt
index a4d62a9d6771..7720d11807c3 100644
--- a/Documentation/ioctl/cdrom.txt
+++ b/Documentation/ioctl/cdrom.txt
@@ -700,6 +700,7 @@ CDROM_DRIVE_STATUS Get tray position, etc.
CDS_TRAY_OPEN
CDS_DRIVE_NOT_READY
CDS_DISC_OK
+ CDS_DRIVE_ERROR
-1 error
error returns:
--
2.13.6
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH resend 6/6] cdrom: wait for drive to become ready
2018-01-26 16:58 ` [PATCH resend 0/6] Fix cdrom autoclose Michal Suchanek
` (4 preceding siblings ...)
2018-01-26 16:58 ` [PATCH resend 5/6] Documentetion: " Michal Suchanek
@ 2018-01-26 16:58 ` Michal Suchanek
2018-01-29 17:11 ` Bart Van Assche
2018-01-26 20:04 ` [PATCH resend 0/6] Fix cdrom autoclose James Bottomley
6 siblings, 1 reply; 28+ messages in thread
From: Michal Suchanek @ 2018-01-26 16:58 UTC (permalink / raw)
To: Jens Axboe, Jonathan Corbet, Borislav Petkov, Tim Waugh,
David S. Miller, James E.J. Bottomley, Martin K. Petersen,
Michal Suchanek, Kees Cook, Christophe JAILLET, Thomas Gleixner,
Greg Kroah-Hartman, Kate Stewart, Philippe Ombredanne, linux-doc,
linux-kernel, linux-ide, linux-scsi
When the drive closes it can take tens of seconds until the disc is
analyzed. Wait for the drive to become ready or report an error.
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
drivers/cdrom/cdrom.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index 69e85c902373..9994441f5041 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -1087,6 +1087,15 @@ int open_for_common(struct cdrom_device_info *cdi, tracktype *tracks)
}
cd_dbg(CD_OPEN, "the tray is now closed\n");
}
+ /* the door should be closed now, check for the disc */
+ if (ret == CDS_DRIVE_NOT_READY) {
+ int poll_res = poll_event_interruptible(
+ CDS_DRIVE_NOT_READY !=
+ (ret = cdo->drive_status(cdi, CDSL_CURRENT)),
+ 500);
+ if (poll_res == -ERESTARTSYS)
+ return poll_res;
+ }
if (ret != CDS_DISC_OK)
return -ENOMEDIUM;
}
--
2.13.6
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH resend 6/6] cdrom: wait for drive to become ready
2018-01-26 16:58 ` [PATCH resend 6/6] cdrom: wait for drive to become ready Michal Suchanek
@ 2018-01-29 17:11 ` Bart Van Assche
0 siblings, 0 replies; 28+ messages in thread
From: Bart Van Assche @ 2018-01-29 17:11 UTC (permalink / raw)
To: corbet, linux-kernel, linux-ide, keescook, msuchanek, tglx,
martin.petersen, axboe, linux-scsi, kstewart, pombredanne, bp,
gregkh, jejb, christophe.jaillet, davem
On Fri, 2018-01-26 at 17:58 +0100, Michal Suchanek wrote:
> When the drive closes it can take tens of seconds until the disc is
> analyzed. Wait for the drive to become ready or report an error.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> drivers/cdrom/cdrom.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index 69e85c902373..9994441f5041 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -1087,6 +1087,15 @@ int open_for_common(struct cdrom_device_info *cdi, tracktype *tracks)
> }
> cd_dbg(CD_OPEN, "the tray is now closed\n");
> }
> + /* the door should be closed now, check for the disc */
> + if (ret == CDS_DRIVE_NOT_READY) {
> + int poll_res = poll_event_interruptible(
> + CDS_DRIVE_NOT_READY !=
> + (ret = cdo->drive_status(cdi, CDSL_CURRENT)),
> + 500);
> + if (poll_res == -ERESTARTSYS)
> + return poll_res;
> + }
> if (ret != CDS_DISC_OK)
> return -ENOMEDIUM;
> }
Same comment here as for a previous patch: although interruptible by a signal,
I'm not sure potentially infinite loops inside the kernel are really welcome.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH resend 6/6] cdrom: wait for drive to become ready
@ 2018-01-29 17:11 ` Bart Van Assche
0 siblings, 0 replies; 28+ messages in thread
From: Bart Van Assche @ 2018-01-29 17:11 UTC (permalink / raw)
To: corbet, linux-kernel, linux-ide, keescook, msuchanek, tglx,
martin.petersen, axboe, linux-scsi, kstewart, pombredanne, bp,
gregkh, jejb, christophe.jaillet, davem, linux-doc, tim
On Fri, 2018-01-26 at 17:58 +0100, Michal Suchanek wrote:
> When the drive closes it can take tens of seconds until the disc is
> analyzed. Wait for the drive to become ready or report an error.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> drivers/cdrom/cdrom.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index 69e85c902373..9994441f5041 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -1087,6 +1087,15 @@ int open_for_common(struct cdrom_device_info *cdi, tracktype *tracks)
> }
> cd_dbg(CD_OPEN, "the tray is now closed\n");
> }
> + /* the door should be closed now, check for the disc */
> + if (ret == CDS_DRIVE_NOT_READY) {
> + int poll_res = poll_event_interruptible(
> + CDS_DRIVE_NOT_READY !=
> + (ret = cdo->drive_status(cdi, CDSL_CURRENT)),
> + 500);
> + if (poll_res == -ERESTARTSYS)
> + return poll_res;
> + }
> if (ret != CDS_DISC_OK)
> return -ENOMEDIUM;
> }
Same comment here as for a previous patch: although interruptible by a signal,
I'm not sure potentially infinite loops inside the kernel are really welcome.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH resend 0/6] Fix cdrom autoclose
2018-01-26 16:58 ` [PATCH resend 0/6] Fix cdrom autoclose Michal Suchanek
` (5 preceding siblings ...)
2018-01-26 16:58 ` [PATCH resend 6/6] cdrom: wait for drive to become ready Michal Suchanek
@ 2018-01-26 20:04 ` James Bottomley
2018-01-27 22:53 ` Michal Suchánek
6 siblings, 1 reply; 28+ messages in thread
From: James Bottomley @ 2018-01-26 20:04 UTC (permalink / raw)
To: Michal Suchanek, Jens Axboe, Jonathan Corbet, Borislav Petkov,
Tim Waugh, David S. Miller, Martin K. Petersen, Kees Cook,
Christophe JAILLET, Thomas Gleixner, Greg Kroah-Hartman,
Kate Stewart, Philippe Ombredanne, linux-doc, linux-kernel,
linux-ide, linux-scsi
On Fri, 2018-01-26 at 17:58 +0100, Michal Suchanek wrote:
> First time I did not get any feedback for the patches.
This is likely because no-one who might inspect the code saw the
patches ... what list are they going to? I'm on the block, scsi and
ide mailing lists and I only saw a doc patch the last time.
James
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH resend 0/6] Fix cdrom autoclose
2018-01-26 20:04 ` [PATCH resend 0/6] Fix cdrom autoclose James Bottomley
@ 2018-01-27 22:53 ` Michal Suchánek
0 siblings, 0 replies; 28+ messages in thread
From: Michal Suchánek @ 2018-01-27 22:53 UTC (permalink / raw)
To: James Bottomley
Cc: Jens Axboe, Jonathan Corbet, Borislav Petkov, Tim Waugh,
David S. Miller, Martin K. Petersen, Kees Cook,
Christophe JAILLET, Thomas Gleixner, Greg Kroah-Hartman,
Kate Stewart, Philippe Ombredanne, linux-doc, linux-kernel,
linux-ide, linux-scsi
On Fri, 26 Jan 2018 12:04:56 -0800
James Bottomley <jejb@linux.vnet.ibm.com> wrote:
> On Fri, 2018-01-26 at 17:58 +0100, Michal Suchanek wrote:
> > First time I did not get any feedback for the patches.
>
> This is likely because no-one who might inspect the code saw the
> patches ... what list are they going to? I'm on the block, scsi and
> ide mailing lists and I only saw a doc patch the last time.
This is what the MAINTAINERS file says:
UNIFORM CDROM DRIVER
M: Jens Axboe <axboe@kernel.dk>
W: http://www.kernel.dk
S: Maintained
F: Documentation/cdrom/
F: drivers/cdrom/cdrom.c
F: include/linux/cdrom.h
F: include/uapi/linux/cdrom.h
Maybe adding some mainling lists there would be useful?
Thanks
Michal
^ permalink raw reply [flat|nested] 28+ messages in thread