| Message ID | 94dc98dc67b1d183d04c338c7978efa0556db6ac.1571834862.git.msuchanek@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix cdrom autoclose. | expand |
On Wed, Oct 23, 2019 at 02:52:47PM +0200, Michal Suchanek wrote: > +static int sr_block_open_finish(struct block_device *bdev, fmode_t mode, > + int ret) > +{ > + struct scsi_cd *cd = scsi_cd(bdev->bd_disk); > + > + /* wait for drive to get ready */ > + if ((ret == -ENOMEDIUM) && !(mode & FMODE_NDELAY)) { > + struct scsi_device *sdev = cd->device; > + /* > + * Cannot use sr_block_ioctl because it locks sr_mutex blocking > + * out any processes trying to access the drive > + */ > + scsi_autopm_get_device(sdev); > + cdrom_ioctl(&cd->cdi, bdev, mode, CDROM_AUTOCLOSE, 0); > + ret = __sr_block_open(bdev, mode); > + scsi_autopm_put_device(sdev); Ioctls should never be used from kernel space. We have a few leftovers, but we need to get rid of that and not add more.
On Wed, Oct 23, 2019 at 07:24:06PM -0700, Christoph Hellwig wrote: > On Wed, Oct 23, 2019 at 02:52:47PM +0200, Michal Suchanek wrote: > > +static int sr_block_open_finish(struct block_device *bdev, fmode_t mode, > > + int ret) > > +{ > > + struct scsi_cd *cd = scsi_cd(bdev->bd_disk); > > + > > + /* wait for drive to get ready */ > > + if ((ret == -ENOMEDIUM) && !(mode & FMODE_NDELAY)) { > > + struct scsi_device *sdev = cd->device; > > + /* > > + * Cannot use sr_block_ioctl because it locks sr_mutex blocking > > + * out any processes trying to access the drive > > + */ > > + scsi_autopm_get_device(sdev); > > + cdrom_ioctl(&cd->cdi, bdev, mode, CDROM_AUTOCLOSE, 0); > > + ret = __sr_block_open(bdev, mode); > > + scsi_autopm_put_device(sdev); > > Ioctls should never be used from kernel space. We have a few leftovers, > but we need to get rid of that and not add more. What is the alternative? Thanks Michal
On Thu, Oct 24, 2019 at 10:51:36AM +0200, Michal Suchánek wrote: > On Wed, Oct 23, 2019 at 07:24:06PM -0700, Christoph Hellwig wrote: > > On Wed, Oct 23, 2019 at 02:52:47PM +0200, Michal Suchanek wrote: > > > +static int sr_block_open_finish(struct block_device *bdev, fmode_t mode, > > > + int ret) > > > +{ > > > + struct scsi_cd *cd = scsi_cd(bdev->bd_disk); > > > + > > > + /* wait for drive to get ready */ > > > + if ((ret == -ENOMEDIUM) && !(mode & FMODE_NDELAY)) { > > > + struct scsi_device *sdev = cd->device; > > > + /* > > > + * Cannot use sr_block_ioctl because it locks sr_mutex blocking > > > + * out any processes trying to access the drive > > > + */ > > > + scsi_autopm_get_device(sdev); > > > + cdrom_ioctl(&cd->cdi, bdev, mode, CDROM_AUTOCLOSE, 0); > > > + ret = __sr_block_open(bdev, mode); > > > + scsi_autopm_put_device(sdev); > > > > Ioctls should never be used from kernel space. We have a few leftovers, > > but we need to get rid of that and not add more. > > What is the alternative? Call the function that would be called by the ioctl instead.
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c index 8090c5bdec09..34d9a818b9e0 100644 --- a/drivers/scsi/sr.c +++ b/drivers/scsi/sr.c @@ -521,29 +521,58 @@ static blk_status_t sr_init_command(struct scsi_cmnd *SCpnt) return ret; } -static int sr_block_open(struct block_device *bdev, fmode_t mode) +static int __sr_block_open(struct block_device *bdev, fmode_t mode) { - struct scsi_cd *cd; - struct scsi_device *sdev; - int ret = -ENXIO; - - cd = scsi_cd_get(bdev->bd_disk); - if (!cd) - goto out; + struct scsi_cd *cd = scsi_cd(bdev->bd_disk); + int ret; - sdev = cd->device; - scsi_autopm_get_device(sdev); check_disk_change(bdev); mutex_lock(&sr_mutex); ret = cdrom_open(&cd->cdi, bdev, mode); mutex_unlock(&sr_mutex); + return ret; +} + +static int sr_block_open(struct block_device *bdev, fmode_t mode) +{ + struct scsi_cd *cd = scsi_cd_get(bdev->bd_disk); + struct scsi_device *sdev; + int ret; + + if (!cd) + return -ENXIO; + + sdev = cd->device; + scsi_autopm_get_device(sdev); + ret = __sr_block_open(bdev, mode); scsi_autopm_put_device(sdev); - if (ret) + + if (ret == -ERESTARTSYS) scsi_cd_put(cd); -out: + return ret; +} + +static int sr_block_open_finish(struct block_device *bdev, fmode_t mode, + int ret) +{ + struct scsi_cd *cd = scsi_cd(bdev->bd_disk); + + /* wait for drive to get ready */ + if ((ret == -ENOMEDIUM) && !(mode & FMODE_NDELAY)) { + struct scsi_device *sdev = cd->device; + /* + * Cannot use sr_block_ioctl because it locks sr_mutex blocking + * out any processes trying to access the drive + */ + scsi_autopm_get_device(sdev); + cdrom_ioctl(&cd->cdi, bdev, mode, CDROM_AUTOCLOSE, 0); + ret = __sr_block_open(bdev, mode); + scsi_autopm_put_device(sdev); + } + return ret; } @@ -639,6 +668,7 @@ static const struct block_device_operations sr_bdops = { .owner = THIS_MODULE, .open = sr_block_open, + .open_finish = sr_block_open_finish, .release = sr_block_release, .ioctl = sr_block_ioctl, .check_events = sr_block_check_events,
Use the autoclose IOCLT provided by cdrom driver to wait for drive to close in open_finish, and attempt to open once more after the door closes. Signed-off-by: Michal Suchanek <msuchanek@suse.de> --- drivers/scsi/sr.c | 54 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 12 deletions(-)