linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* stop using ioctl_by_bdev for file system access to CDROMs v2
@ 2020-04-25  7:56 Christoph Hellwig
  2020-04-25  7:57 ` [PATCH 1/7] block: add a cdrom_device_info pointer to struct gendisk Christoph Hellwig
                   ` (8 more replies)
  0 siblings, 9 replies; 24+ messages in thread
From: Christoph Hellwig @ 2020-04-25  7:56 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel

Hi Jens,

except for the DASD case under discussion the last users of ioctl_by_bdev
are the file system drivers that want to query CDROM information using
ioctls.  This series switches them to use function calls directly into
the CDROM midlayer instead, which implies:

 - adding a cdrom_device_info pointer to the gendisk, so that file systems
   can find it without going to the low-level driver first
 - ensuring that the CDROM midlayer (which isn't a lot of code) is built
   in if the file systems are built in so that they can actually call the
   exported functions

Changes since v1:
 - fix up the no-CDROM error case in isofs_get_last_session to return 0
   instead of -EINVAL.

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

* [PATCH 1/7] block: add a cdrom_device_info pointer to struct gendisk
  2020-04-25  7:56 stop using ioctl_by_bdev for file system access to CDROMs v2 Christoph Hellwig
@ 2020-04-25  7:57 ` Christoph Hellwig
  2020-04-27  6:15   ` Hannes Reinecke
  2020-04-25  7:57 ` [PATCH 2/7] ide-cd: rename cdrom_read_tocentry Christoph Hellwig
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2020-04-25  7:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel, Damien Le Moal

Add a pointer to the CDROM information structure to struct gendisk.
This will allow various removable media file systems to call directly
into the CDROM layer instead of abusing ioctls with kernel pointers.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/block/paride/pcd.c | 2 +-
 drivers/cdrom/cdrom.c      | 5 ++++-
 drivers/cdrom/gdrom.c      | 2 +-
 drivers/ide/ide-cd.c       | 3 +--
 drivers/scsi/sr.c          | 3 +--
 include/linux/cdrom.h      | 2 +-
 include/linux/genhd.h      | 9 +++++++++
 7 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/block/paride/pcd.c b/drivers/block/paride/pcd.c
index cda5cf917e9af..5124eca90e833 100644
--- a/drivers/block/paride/pcd.c
+++ b/drivers/block/paride/pcd.c
@@ -1032,7 +1032,7 @@ static int __init pcd_init(void)
 
 	for (unit = 0, cd = pcd; unit < PCD_UNITS; unit++, cd++) {
 		if (cd->present) {
-			register_cdrom(&cd->info);
+			register_cdrom(cd->disk, &cd->info);
 			cd->disk->private_data = cd;
 			add_disk(cd->disk);
 		}
diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index faca0f346fff2..a1d2112fd283f 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -586,7 +586,7 @@ static int cdrom_mrw_set_lba_space(struct cdrom_device_info *cdi, int space)
 	return 0;
 }
 
-int register_cdrom(struct cdrom_device_info *cdi)
+int register_cdrom(struct gendisk *disk, struct cdrom_device_info *cdi)
 {
 	static char banner_printed;
 	const struct cdrom_device_ops *cdo = cdi->ops;
@@ -601,6 +601,9 @@ int register_cdrom(struct cdrom_device_info *cdi)
 		cdrom_sysctl_register();
 	}
 
+	cdi->disk = disk;
+	disk->cdi = cdi;
+
 	ENSURE(cdo, drive_status, CDC_DRIVE_STATUS);
 	if (cdo->check_events == NULL && cdo->media_changed == NULL)
 		WARN_ON_ONCE(cdo->capability & (CDC_MEDIA_CHANGED | CDC_SELECT_DISC));
diff --git a/drivers/cdrom/gdrom.c b/drivers/cdrom/gdrom.c
index c51292c2a131e..09b0cd292720f 100644
--- a/drivers/cdrom/gdrom.c
+++ b/drivers/cdrom/gdrom.c
@@ -770,7 +770,7 @@ static int probe_gdrom(struct platform_device *devptr)
 		goto probe_fail_no_disk;
 	}
 	probe_gdrom_setupdisk();
-	if (register_cdrom(gd.cd_info)) {
+	if (register_cdrom(gd.disk, gd.cd_info)) {
 		err = -ENODEV;
 		goto probe_fail_cdrom_register;
 	}
diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index dcf8b51b47fda..40e124eb918aa 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -1305,8 +1305,7 @@ static int ide_cdrom_register(ide_drive_t *drive, int nslots)
 	if (drive->atapi_flags & IDE_AFLAG_NO_SPEED_SELECT)
 		devinfo->mask |= CDC_SELECT_SPEED;
 
-	devinfo->disk = info->disk;
-	return register_cdrom(devinfo);
+	return register_cdrom(info->disk, devinfo);
 }
 
 static int ide_cdrom_probe_capabilities(ide_drive_t *drive)
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index d2fe3fa470f95..f9b589d60a460 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -794,9 +794,8 @@ static int sr_probe(struct device *dev)
 	set_capacity(disk, cd->capacity);
 	disk->private_data = &cd->driver;
 	disk->queue = sdev->request_queue;
-	cd->cdi.disk = disk;
 
-	if (register_cdrom(&cd->cdi))
+	if (register_cdrom(disk, &cd->cdi))
 		goto fail_put;
 
 	/*
diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
index 528271c600182..4f74ce050253d 100644
--- a/include/linux/cdrom.h
+++ b/include/linux/cdrom.h
@@ -104,7 +104,7 @@ extern unsigned int cdrom_check_events(struct cdrom_device_info *cdi,
 				       unsigned int clearing);
 extern int cdrom_media_changed(struct cdrom_device_info *);
 
-extern int register_cdrom(struct cdrom_device_info *cdi);
+extern int register_cdrom(struct gendisk *disk, struct cdrom_device_info *cdi);
 extern void unregister_cdrom(struct cdrom_device_info *cdi);
 
 typedef struct {
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 058d895544c75..f9c226f9546af 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -217,11 +217,20 @@ struct gendisk {
 #ifdef  CONFIG_BLK_DEV_INTEGRITY
 	struct kobject integrity_kobj;
 #endif	/* CONFIG_BLK_DEV_INTEGRITY */
+#if IS_ENABLED(CONFIG_CDROM)
+	struct cdrom_device_info *cdi;
+#endif
 	int node_id;
 	struct badblocks *bb;
 	struct lockdep_map lockdep_map;
 };
 
+#if IS_REACHABLE(CONFIG_CDROM)
+#define disk_to_cdi(disk)	((disk)->cdi)
+#else
+#define disk_to_cdi(disk)	NULL
+#endif
+
 static inline struct gendisk *part_to_disk(struct hd_struct *part)
 {
 	if (likely(part)) {
-- 
2.26.1


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

* [PATCH 2/7] ide-cd: rename cdrom_read_tocentry
  2020-04-25  7:56 stop using ioctl_by_bdev for file system access to CDROMs v2 Christoph Hellwig
  2020-04-25  7:57 ` [PATCH 1/7] block: add a cdrom_device_info pointer to struct gendisk Christoph Hellwig
@ 2020-04-25  7:57 ` Christoph Hellwig
  2020-04-27  6:16   ` Hannes Reinecke
  2020-04-25  7:57 ` [PATCH 3/7] cdrom: factor out a cdrom_read_tocentry helper Christoph Hellwig
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2020-04-25  7:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel, Damien Le Moal

Give the cdrom_read_tocentry function and ide_ prefix to not conflict
with the soon to be added generic function.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/ide/ide-cd.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 40e124eb918aa..7f17f83039888 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -1034,8 +1034,8 @@ static int cdrom_read_capacity(ide_drive_t *drive, unsigned long *capacity,
 	return 0;
 }
 
-static int cdrom_read_tocentry(ide_drive_t *drive, int trackno, int msf_flag,
-				int format, char *buf, int buflen)
+static int ide_cdrom_read_tocentry(ide_drive_t *drive, int trackno,
+		int msf_flag, int format, char *buf, int buflen)
 {
 	unsigned char cmd[BLK_MAX_CDB];
 
@@ -1104,7 +1104,7 @@ int ide_cd_read_toc(ide_drive_t *drive)
 				     sectors_per_frame << SECTOR_SHIFT);
 
 	/* first read just the header, so we know how long the TOC is */
-	stat = cdrom_read_tocentry(drive, 0, 1, 0, (char *) &toc->hdr,
+	stat = ide_cdrom_read_tocentry(drive, 0, 1, 0, (char *) &toc->hdr,
 				    sizeof(struct atapi_toc_header));
 	if (stat)
 		return stat;
@@ -1121,7 +1121,7 @@ int ide_cd_read_toc(ide_drive_t *drive)
 		ntracks = MAX_TRACKS;
 
 	/* now read the whole schmeer */
-	stat = cdrom_read_tocentry(drive, toc->hdr.first_track, 1, 0,
+	stat = ide_cdrom_read_tocentry(drive, toc->hdr.first_track, 1, 0,
 				  (char *)&toc->hdr,
 				   sizeof(struct atapi_toc_header) +
 				   (ntracks + 1) *
@@ -1141,7 +1141,7 @@ int ide_cd_read_toc(ide_drive_t *drive)
 		 * Heiko Eißfeldt.
 		 */
 		ntracks = 0;
-		stat = cdrom_read_tocentry(drive, CDROM_LEADOUT, 1, 0,
+		stat = ide_cdrom_read_tocentry(drive, CDROM_LEADOUT, 1, 0,
 					   (char *)&toc->hdr,
 					   sizeof(struct atapi_toc_header) +
 					   (ntracks + 1) *
@@ -1181,7 +1181,7 @@ int ide_cd_read_toc(ide_drive_t *drive)
 
 	if (toc->hdr.first_track != CDROM_LEADOUT) {
 		/* read the multisession information */
-		stat = cdrom_read_tocentry(drive, 0, 0, 1, (char *)&ms_tmp,
+		stat = ide_cdrom_read_tocentry(drive, 0, 0, 1, (char *)&ms_tmp,
 					   sizeof(ms_tmp));
 		if (stat)
 			return stat;
@@ -1195,7 +1195,7 @@ int ide_cd_read_toc(ide_drive_t *drive)
 
 	if (drive->atapi_flags & IDE_AFLAG_TOCADDR_AS_BCD) {
 		/* re-read multisession information using MSF format */
-		stat = cdrom_read_tocentry(drive, 0, 1, 1, (char *)&ms_tmp,
+		stat = ide_cdrom_read_tocentry(drive, 0, 1, 1, (char *)&ms_tmp,
 					   sizeof(ms_tmp));
 		if (stat)
 			return stat;
-- 
2.26.1


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

* [PATCH 3/7] cdrom: factor out a cdrom_read_tocentry helper
  2020-04-25  7:56 stop using ioctl_by_bdev for file system access to CDROMs v2 Christoph Hellwig
  2020-04-25  7:57 ` [PATCH 1/7] block: add a cdrom_device_info pointer to struct gendisk Christoph Hellwig
  2020-04-25  7:57 ` [PATCH 2/7] ide-cd: rename cdrom_read_tocentry Christoph Hellwig
@ 2020-04-25  7:57 ` Christoph Hellwig
  2020-04-27  6:17   ` Hannes Reinecke
  2020-04-25  7:57 ` [PATCH 4/7] cdrom: factor out a cdrom_multisession helper Christoph Hellwig
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2020-04-25  7:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel, Damien Le Moal

Factor out a version of the CDROMREADTOCENTRY ioctl handler that can
be called directly from kernel space.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/cdrom/cdrom.c | 39 ++++++++++++++++++++++-----------------
 include/linux/cdrom.h |  3 +++
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index a1d2112fd283f..c91d1e1382142 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -2666,32 +2666,37 @@ static int cdrom_ioctl_read_tochdr(struct cdrom_device_info *cdi,
 	return 0;
 }
 
+int cdrom_read_tocentry(struct cdrom_device_info *cdi,
+		struct cdrom_tocentry *entry)
+{
+	u8 requested_format = entry->cdte_format;
+	int ret;
+
+	if (requested_format != CDROM_MSF && requested_format != CDROM_LBA)
+		return -EINVAL;
+
+	/* make interface to low-level uniform */
+	entry->cdte_format = CDROM_MSF;
+	ret = cdi->ops->audio_ioctl(cdi, CDROMREADTOCENTRY, entry);
+	if (!ret)
+		sanitize_format(&entry->cdte_addr, &entry->cdte_format,
+				requested_format);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cdrom_read_tocentry);
+
 static int cdrom_ioctl_read_tocentry(struct cdrom_device_info *cdi,
 		void __user *argp)
 {
 	struct cdrom_tocentry entry;
-	u8 requested_format;
 	int ret;
 
-	/* cd_dbg(CD_DO_IOCTL, "entering CDROMREADTOCENTRY\n"); */
-
 	if (copy_from_user(&entry, argp, sizeof(entry)))
 		return -EFAULT;
-
-	requested_format = entry.cdte_format;
-	if (requested_format != CDROM_MSF && requested_format != CDROM_LBA)
-		return -EINVAL;
-	/* make interface to low-level uniform */
-	entry.cdte_format = CDROM_MSF;
-	ret = cdi->ops->audio_ioctl(cdi, CDROMREADTOCENTRY, &entry);
-	if (ret)
-		return ret;
-	sanitize_format(&entry.cdte_addr, &entry.cdte_format, requested_format);
-
-	if (copy_to_user(argp, &entry, sizeof(entry)))
+	ret = cdrom_read_tocentry(cdi, &entry);
+	if (!ret && copy_to_user(argp, &entry, sizeof(entry)))
 		return -EFAULT;
-	/* cd_dbg(CD_DO_IOCTL, "CDROMREADTOCENTRY successful\n"); */
-	return 0;
+	return ret;
 }
 
 static int cdrom_ioctl_play_msf(struct cdrom_device_info *cdi,
diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
index 4f74ce050253d..008c4d79fa332 100644
--- a/include/linux/cdrom.h
+++ b/include/linux/cdrom.h
@@ -94,6 +94,9 @@ struct cdrom_device_ops {
 			       struct packet_command *);
 };
 
+int cdrom_read_tocentry(struct cdrom_device_info *cdi,
+		struct cdrom_tocentry *entry);
+
 /* the general block_device operations structure: */
 extern int cdrom_open(struct cdrom_device_info *cdi, struct block_device *bdev,
 			fmode_t mode);
-- 
2.26.1


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

* [PATCH 4/7] cdrom: factor out a cdrom_multisession helper
  2020-04-25  7:56 stop using ioctl_by_bdev for file system access to CDROMs v2 Christoph Hellwig
                   ` (2 preceding siblings ...)
  2020-04-25  7:57 ` [PATCH 3/7] cdrom: factor out a cdrom_read_tocentry helper Christoph Hellwig
@ 2020-04-25  7:57 ` Christoph Hellwig
  2020-04-27  6:17   ` Hannes Reinecke
  2020-04-25  7:57 ` [PATCH 5/7] hfsplus: stop using ioctl_by_bdev Christoph Hellwig
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2020-04-25  7:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel, Damien Le Moal

Factor out a version of the CDROMMULTISESSION ioctl handler that can
be called directly from kernel space.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/cdrom/cdrom.c | 41 +++++++++++++++++++++++++----------------
 include/linux/cdrom.h |  2 ++
 2 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index c91d1e1382142..06896c07b1333 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -2295,37 +2295,46 @@ static int cdrom_read_cdda(struct cdrom_device_info *cdi, __u8 __user *ubuf,
 	return cdrom_read_cdda_old(cdi, ubuf, lba, nframes);	
 }
 
-static int cdrom_ioctl_multisession(struct cdrom_device_info *cdi,
-		void __user *argp)
+int cdrom_multisession(struct cdrom_device_info *cdi,
+		struct cdrom_multisession *info)
 {
-	struct cdrom_multisession ms_info;
 	u8 requested_format;
 	int ret;
 
-	cd_dbg(CD_DO_IOCTL, "entering CDROMMULTISESSION\n");
-
 	if (!(cdi->ops->capability & CDC_MULTI_SESSION))
 		return -ENOSYS;
 
-	if (copy_from_user(&ms_info, argp, sizeof(ms_info)))
-		return -EFAULT;
-
-	requested_format = ms_info.addr_format;
+	requested_format = info->addr_format;
 	if (requested_format != CDROM_MSF && requested_format != CDROM_LBA)
 		return -EINVAL;
-	ms_info.addr_format = CDROM_LBA;
+	info->addr_format = CDROM_LBA;
 
-	ret = cdi->ops->get_last_session(cdi, &ms_info);
-	if (ret)
-		return ret;
+	ret = cdi->ops->get_last_session(cdi, info);
+	if (!ret)
+		sanitize_format(&info->addr, &info->addr_format,
+				requested_format);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cdrom_multisession);
 
-	sanitize_format(&ms_info.addr, &ms_info.addr_format, requested_format);
+static int cdrom_ioctl_multisession(struct cdrom_device_info *cdi,
+		void __user *argp)
+{
+	struct cdrom_multisession info;
+	int ret;
+
+	cd_dbg(CD_DO_IOCTL, "entering CDROMMULTISESSION\n");
 
-	if (copy_to_user(argp, &ms_info, sizeof(ms_info)))
+	if (copy_from_user(&info, argp, sizeof(info)))
+		return -EFAULT;
+	ret = cdrom_multisession(cdi, &info);
+	if (ret)
+		return ret;
+	if (copy_to_user(argp, &info, sizeof(info)))
 		return -EFAULT;
 
 	cd_dbg(CD_DO_IOCTL, "CDROMMULTISESSION successful\n");
-	return 0;
+	return ret;
 }
 
 static int cdrom_ioctl_eject(struct cdrom_device_info *cdi)
diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
index 008c4d79fa332..8543fa59da720 100644
--- a/include/linux/cdrom.h
+++ b/include/linux/cdrom.h
@@ -94,6 +94,8 @@ struct cdrom_device_ops {
 			       struct packet_command *);
 };
 
+int cdrom_multisession(struct cdrom_device_info *cdi,
+		struct cdrom_multisession *info);
 int cdrom_read_tocentry(struct cdrom_device_info *cdi,
 		struct cdrom_tocentry *entry);
 
-- 
2.26.1


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

* [PATCH 5/7] hfsplus: stop using ioctl_by_bdev
  2020-04-25  7:56 stop using ioctl_by_bdev for file system access to CDROMs v2 Christoph Hellwig
                   ` (3 preceding siblings ...)
  2020-04-25  7:57 ` [PATCH 4/7] cdrom: factor out a cdrom_multisession helper Christoph Hellwig
@ 2020-04-25  7:57 ` Christoph Hellwig
  2020-04-27  6:18   ` Hannes Reinecke
  2020-05-04 16:16   ` Jens Axboe
  2020-04-25  7:57 ` [PATCH 6/7] isofs: " Christoph Hellwig
                   ` (3 subsequent siblings)
  8 siblings, 2 replies; 24+ messages in thread
From: Christoph Hellwig @ 2020-04-25  7:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel, Damien Le Moal

Instead just call the CDROM layer functionality directly.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 fs/hfsplus/wrapper.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/fs/hfsplus/wrapper.c b/fs/hfsplus/wrapper.c
index 08c1580bdf7ad..61eec628805de 100644
--- a/fs/hfsplus/wrapper.c
+++ b/fs/hfsplus/wrapper.c
@@ -127,31 +127,34 @@ static int hfsplus_read_mdb(void *bufptr, struct hfsplus_wd *wd)
 static int hfsplus_get_last_session(struct super_block *sb,
 				    sector_t *start, sector_t *size)
 {
-	struct cdrom_multisession ms_info;
-	struct cdrom_tocentry te;
-	int res;
+	struct cdrom_device_info *cdi = disk_to_cdi(sb->s_bdev->bd_disk);
 
 	/* default values */
 	*start = 0;
 	*size = i_size_read(sb->s_bdev->bd_inode) >> 9;
 
 	if (HFSPLUS_SB(sb)->session >= 0) {
+		struct cdrom_tocentry te;
+
+		if (!cdi)
+			return -EINVAL;
+
 		te.cdte_track = HFSPLUS_SB(sb)->session;
 		te.cdte_format = CDROM_LBA;
-		res = ioctl_by_bdev(sb->s_bdev,
-			CDROMREADTOCENTRY, (unsigned long)&te);
-		if (!res && (te.cdte_ctrl & CDROM_DATA_TRACK) == 4) {
-			*start = (sector_t)te.cdte_addr.lba << 2;
-			return 0;
+		if (cdrom_read_tocentry(cdi, &te) ||
+		    (te.cdte_ctrl & CDROM_DATA_TRACK) != 4) {
+			pr_err("invalid session number or type of track\n");
+			return -EINVAL;
 		}
-		pr_err("invalid session number or type of track\n");
-		return -EINVAL;
+		*start = (sector_t)te.cdte_addr.lba << 2;
+	} else if (cdi) {
+		struct cdrom_multisession ms_info;
+
+		ms_info.addr_format = CDROM_LBA;
+		if (cdrom_multisession(cdi, &ms_info) == 0 && ms_info.xa_flag)
+			*start = (sector_t)ms_info.addr.lba << 2;
 	}
-	ms_info.addr_format = CDROM_LBA;
-	res = ioctl_by_bdev(sb->s_bdev, CDROMMULTISESSION,
-		(unsigned long)&ms_info);
-	if (!res && ms_info.xa_flag)
-		*start = (sector_t)ms_info.addr.lba << 2;
+
 	return 0;
 }
 
-- 
2.26.1


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

* [PATCH 6/7] isofs: stop using ioctl_by_bdev
  2020-04-25  7:56 stop using ioctl_by_bdev for file system access to CDROMs v2 Christoph Hellwig
                   ` (4 preceding siblings ...)
  2020-04-25  7:57 ` [PATCH 5/7] hfsplus: stop using ioctl_by_bdev Christoph Hellwig
@ 2020-04-25  7:57 ` Christoph Hellwig
  2020-04-27  6:18   ` Hannes Reinecke
  2020-04-27  9:50   ` Jan Kara
  2020-04-25  7:57 ` [PATCH 7/7] udf: " Christoph Hellwig
                   ` (2 subsequent siblings)
  8 siblings, 2 replies; 24+ messages in thread
From: Christoph Hellwig @ 2020-04-25  7:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel, Damien Le Moal

Instead just call the CDROM layer functionality directly, and turn the
hot mess in isofs_get_last_session into remotely readable code.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 fs/isofs/inode.c | 54 +++++++++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
index 62c0462dc89f3..276107cdaaf13 100644
--- a/fs/isofs/inode.c
+++ b/fs/isofs/inode.c
@@ -544,43 +544,41 @@ static int isofs_show_options(struct seq_file *m, struct dentry *root)
 
 static unsigned int isofs_get_last_session(struct super_block *sb, s32 session)
 {
-	struct cdrom_multisession ms_info;
-	unsigned int vol_desc_start;
-	struct block_device *bdev = sb->s_bdev;
-	int i;
+	struct cdrom_device_info *cdi = disk_to_cdi(sb->s_bdev->bd_disk);
+	unsigned int vol_desc_start = 0;
 
-	vol_desc_start=0;
-	ms_info.addr_format=CDROM_LBA;
 	if (session > 0) {
-		struct cdrom_tocentry Te;
-		Te.cdte_track=session;
-		Te.cdte_format=CDROM_LBA;
-		i = ioctl_by_bdev(bdev, CDROMREADTOCENTRY, (unsigned long) &Te);
-		if (!i) {
+		struct cdrom_tocentry te;
+
+		if (!cdi)
+			return 0;
+
+		te.cdte_track = session;
+		te.cdte_format = CDROM_LBA;
+		if (cdrom_read_tocentry(cdi, &te) == 0) {
 			printk(KERN_DEBUG "ISOFS: Session %d start %d type %d\n",
-				session, Te.cdte_addr.lba,
-				Te.cdte_ctrl&CDROM_DATA_TRACK);
-			if ((Te.cdte_ctrl&CDROM_DATA_TRACK) == 4)
-				return Te.cdte_addr.lba;
+				session, te.cdte_addr.lba,
+				te.cdte_ctrl & CDROM_DATA_TRACK);
+			if ((te.cdte_ctrl & CDROM_DATA_TRACK) == 4)
+				return te.cdte_addr.lba;
 		}
 
 		printk(KERN_ERR "ISOFS: Invalid session number or type of track\n");
 	}
-	i = ioctl_by_bdev(bdev, CDROMMULTISESSION, (unsigned long) &ms_info);
-	if (session > 0)
-		printk(KERN_ERR "ISOFS: Invalid session number\n");
-#if 0
-	printk(KERN_DEBUG "isofs.inode: CDROMMULTISESSION: rc=%d\n",i);
-	if (i==0) {
-		printk(KERN_DEBUG "isofs.inode: XA disk: %s\n",ms_info.xa_flag?"yes":"no");
-		printk(KERN_DEBUG "isofs.inode: vol_desc_start = %d\n", ms_info.addr.lba);
-	}
-#endif
-	if (i==0)
+
+	if (cdi) {
+		struct cdrom_multisession ms_info;
+
+		ms_info.addr_format = CDROM_LBA;
+		if (cdrom_multisession(cdi, &ms_info) == 0) {
 #if WE_OBEY_THE_WRITTEN_STANDARDS
-		if (ms_info.xa_flag) /* necessary for a valid ms_info.addr */
+			/* necessary for a valid ms_info.addr */
+			if (ms_info.xa_flag)
 #endif
-			vol_desc_start=ms_info.addr.lba;
+				vol_desc_start = ms_info.addr.lba;
+		}
+	}
+
 	return vol_desc_start;
 }
 
-- 
2.26.1


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

* [PATCH 7/7] udf: stop using ioctl_by_bdev
  2020-04-25  7:56 stop using ioctl_by_bdev for file system access to CDROMs v2 Christoph Hellwig
                   ` (5 preceding siblings ...)
  2020-04-25  7:57 ` [PATCH 6/7] isofs: " Christoph Hellwig
@ 2020-04-25  7:57 ` Christoph Hellwig
  2020-04-27  6:18   ` Hannes Reinecke
  2020-04-28  6:53 ` stop using ioctl_by_bdev for file system access to CDROMs v2 Christoph Hellwig
  2020-05-04 16:42 ` Jens Axboe
  8 siblings, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2020-04-25  7:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel, Jan Kara,
	Damien Le Moal

Instead just call the CDROM layer functionality directly.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 fs/udf/lowlevel.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/fs/udf/lowlevel.c b/fs/udf/lowlevel.c
index 5c7ec121990da..f1094cdcd6cde 100644
--- a/fs/udf/lowlevel.c
+++ b/fs/udf/lowlevel.c
@@ -27,41 +27,38 @@
 
 unsigned int udf_get_last_session(struct super_block *sb)
 {
+	struct cdrom_device_info *cdi = disk_to_cdi(sb->s_bdev->bd_disk);
 	struct cdrom_multisession ms_info;
-	unsigned int vol_desc_start;
-	struct block_device *bdev = sb->s_bdev;
-	int i;
 
-	vol_desc_start = 0;
-	ms_info.addr_format = CDROM_LBA;
-	i = ioctl_by_bdev(bdev, CDROMMULTISESSION, (unsigned long)&ms_info);
+	if (!cdi) {
+		udf_debug("CDROMMULTISESSION not supported.\n");
+		return 0;
+	}
 
-	if (i == 0) {
+	ms_info.addr_format = CDROM_LBA;
+	if (cdrom_multisession(cdi, &ms_info) == 0) {
 		udf_debug("XA disk: %s, vol_desc_start=%d\n",
 			  ms_info.xa_flag ? "yes" : "no", ms_info.addr.lba);
 		if (ms_info.xa_flag) /* necessary for a valid ms_info.addr */
-			vol_desc_start = ms_info.addr.lba;
-	} else {
-		udf_debug("CDROMMULTISESSION not supported: rc=%d\n", i);
+			return ms_info.addr.lba;
 	}
-	return vol_desc_start;
+	return 0;
 }
 
 unsigned long udf_get_last_block(struct super_block *sb)
 {
 	struct block_device *bdev = sb->s_bdev;
+	struct cdrom_device_info *cdi = disk_to_cdi(bdev->bd_disk);
 	unsigned long lblock = 0;
 
 	/*
-	 * ioctl failed or returned obviously bogus value?
+	 * The cdrom layer call failed or returned obviously bogus value?
 	 * Try using the device size...
 	 */
-	if (ioctl_by_bdev(bdev, CDROM_LAST_WRITTEN, (unsigned long) &lblock) ||
-	    lblock == 0)
+	if (!cdi || cdrom_get_last_written(cdi, &lblock) || lblock == 0)
 		lblock = i_size_read(bdev->bd_inode) >> sb->s_blocksize_bits;
 
 	if (lblock)
 		return lblock - 1;
-	else
-		return 0;
+	return 0;
 }
-- 
2.26.1


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

* Re: [PATCH 1/7] block: add a cdrom_device_info pointer to struct gendisk
  2020-04-25  7:57 ` [PATCH 1/7] block: add a cdrom_device_info pointer to struct gendisk Christoph Hellwig
@ 2020-04-27  6:15   ` Hannes Reinecke
  0 siblings, 0 replies; 24+ messages in thread
From: Hannes Reinecke @ 2020-04-27  6:15 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel, Damien Le Moal

On 4/25/20 9:57 AM, Christoph Hellwig wrote:
> Add a pointer to the CDROM information structure to struct gendisk.
> This will allow various removable media file systems to call directly
> into the CDROM layer instead of abusing ioctls with kernel pointers.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>   drivers/block/paride/pcd.c | 2 +-
>   drivers/cdrom/cdrom.c      | 5 ++++-
>   drivers/cdrom/gdrom.c      | 2 +-
>   drivers/ide/ide-cd.c       | 3 +--
>   drivers/scsi/sr.c          | 3 +--
>   include/linux/cdrom.h      | 2 +-
>   include/linux/genhd.h      | 9 +++++++++
>   7 files changed, 18 insertions(+), 8 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

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

* Re: [PATCH 2/7] ide-cd: rename cdrom_read_tocentry
  2020-04-25  7:57 ` [PATCH 2/7] ide-cd: rename cdrom_read_tocentry Christoph Hellwig
@ 2020-04-27  6:16   ` Hannes Reinecke
  0 siblings, 0 replies; 24+ messages in thread
From: Hannes Reinecke @ 2020-04-27  6:16 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel, Damien Le Moal

On 4/25/20 9:57 AM, Christoph Hellwig wrote:
> Give the cdrom_read_tocentry function and ide_ prefix to not conflict
> with the soon to be added generic function.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>   drivers/ide/ide-cd.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

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

* Re: [PATCH 3/7] cdrom: factor out a cdrom_read_tocentry helper
  2020-04-25  7:57 ` [PATCH 3/7] cdrom: factor out a cdrom_read_tocentry helper Christoph Hellwig
@ 2020-04-27  6:17   ` Hannes Reinecke
  0 siblings, 0 replies; 24+ messages in thread
From: Hannes Reinecke @ 2020-04-27  6:17 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel, Damien Le Moal

On 4/25/20 9:57 AM, Christoph Hellwig wrote:
> Factor out a version of the CDROMREADTOCENTRY ioctl handler that can
> be called directly from kernel space.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>   drivers/cdrom/cdrom.c | 39 ++++++++++++++++++++++-----------------
>   include/linux/cdrom.h |  3 +++
>   2 files changed, 25 insertions(+), 17 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

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

* Re: [PATCH 4/7] cdrom: factor out a cdrom_multisession helper
  2020-04-25  7:57 ` [PATCH 4/7] cdrom: factor out a cdrom_multisession helper Christoph Hellwig
@ 2020-04-27  6:17   ` Hannes Reinecke
  0 siblings, 0 replies; 24+ messages in thread
From: Hannes Reinecke @ 2020-04-27  6:17 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel, Damien Le Moal

On 4/25/20 9:57 AM, Christoph Hellwig wrote:
> Factor out a version of the CDROMMULTISESSION ioctl handler that can
> be called directly from kernel space.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>   drivers/cdrom/cdrom.c | 41 +++++++++++++++++++++++++----------------
>   include/linux/cdrom.h |  2 ++
>   2 files changed, 27 insertions(+), 16 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

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

* Re: [PATCH 5/7] hfsplus: stop using ioctl_by_bdev
  2020-04-25  7:57 ` [PATCH 5/7] hfsplus: stop using ioctl_by_bdev Christoph Hellwig
@ 2020-04-27  6:18   ` Hannes Reinecke
  2020-05-04 16:16   ` Jens Axboe
  1 sibling, 0 replies; 24+ messages in thread
From: Hannes Reinecke @ 2020-04-27  6:18 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel, Damien Le Moal

On 4/25/20 9:57 AM, Christoph Hellwig wrote:
> Instead just call the CDROM layer functionality directly.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>   fs/hfsplus/wrapper.c | 33 ++++++++++++++++++---------------
>   1 file changed, 18 insertions(+), 15 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

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

* Re: [PATCH 6/7] isofs: stop using ioctl_by_bdev
  2020-04-25  7:57 ` [PATCH 6/7] isofs: " Christoph Hellwig
@ 2020-04-27  6:18   ` Hannes Reinecke
  2020-04-27  9:50   ` Jan Kara
  1 sibling, 0 replies; 24+ messages in thread
From: Hannes Reinecke @ 2020-04-27  6:18 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel, Damien Le Moal

On 4/25/20 9:57 AM, Christoph Hellwig wrote:
> Instead just call the CDROM layer functionality directly, and turn the
> hot mess in isofs_get_last_session into remotely readable code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>   fs/isofs/inode.c | 54 +++++++++++++++++++++++-------------------------
>   1 file changed, 26 insertions(+), 28 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

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

* Re: [PATCH 7/7] udf: stop using ioctl_by_bdev
  2020-04-25  7:57 ` [PATCH 7/7] udf: " Christoph Hellwig
@ 2020-04-27  6:18   ` Hannes Reinecke
  0 siblings, 0 replies; 24+ messages in thread
From: Hannes Reinecke @ 2020-04-27  6:18 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel, Jan Kara,
	Damien Le Moal

On 4/25/20 9:57 AM, Christoph Hellwig wrote:
> Instead just call the CDROM layer functionality directly.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Jan Kara <jack@suse.cz>
> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>   fs/udf/lowlevel.c | 29 +++++++++++++----------------
>   1 file changed, 13 insertions(+), 16 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

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

* Re: [PATCH 6/7] isofs: stop using ioctl_by_bdev
  2020-04-25  7:57 ` [PATCH 6/7] isofs: " Christoph Hellwig
  2020-04-27  6:18   ` Hannes Reinecke
@ 2020-04-27  9:50   ` Jan Kara
  1 sibling, 0 replies; 24+ messages in thread
From: Jan Kara @ 2020-04-27  9:50 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Tim Waugh, Borislav Petkov, Jan Kara, linux-block,
	linux-ide, linux-scsi, linux-fsdevel, linux-kernel,
	Damien Le Moal

On Sat 25-04-20 09:57:05, Christoph Hellwig wrote:
> Instead just call the CDROM layer functionality directly, and turn the
> hot mess in isofs_get_last_session into remotely readable code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>

Looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/isofs/inode.c | 54 +++++++++++++++++++++++-------------------------
>  1 file changed, 26 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
> index 62c0462dc89f3..276107cdaaf13 100644
> --- a/fs/isofs/inode.c
> +++ b/fs/isofs/inode.c
> @@ -544,43 +544,41 @@ static int isofs_show_options(struct seq_file *m, struct dentry *root)
>  
>  static unsigned int isofs_get_last_session(struct super_block *sb, s32 session)
>  {
> -	struct cdrom_multisession ms_info;
> -	unsigned int vol_desc_start;
> -	struct block_device *bdev = sb->s_bdev;
> -	int i;
> +	struct cdrom_device_info *cdi = disk_to_cdi(sb->s_bdev->bd_disk);
> +	unsigned int vol_desc_start = 0;
>  
> -	vol_desc_start=0;
> -	ms_info.addr_format=CDROM_LBA;
>  	if (session > 0) {
> -		struct cdrom_tocentry Te;
> -		Te.cdte_track=session;
> -		Te.cdte_format=CDROM_LBA;
> -		i = ioctl_by_bdev(bdev, CDROMREADTOCENTRY, (unsigned long) &Te);
> -		if (!i) {
> +		struct cdrom_tocentry te;
> +
> +		if (!cdi)
> +			return 0;
> +
> +		te.cdte_track = session;
> +		te.cdte_format = CDROM_LBA;
> +		if (cdrom_read_tocentry(cdi, &te) == 0) {
>  			printk(KERN_DEBUG "ISOFS: Session %d start %d type %d\n",
> -				session, Te.cdte_addr.lba,
> -				Te.cdte_ctrl&CDROM_DATA_TRACK);
> -			if ((Te.cdte_ctrl&CDROM_DATA_TRACK) == 4)
> -				return Te.cdte_addr.lba;
> +				session, te.cdte_addr.lba,
> +				te.cdte_ctrl & CDROM_DATA_TRACK);
> +			if ((te.cdte_ctrl & CDROM_DATA_TRACK) == 4)
> +				return te.cdte_addr.lba;
>  		}
>  
>  		printk(KERN_ERR "ISOFS: Invalid session number or type of track\n");
>  	}
> -	i = ioctl_by_bdev(bdev, CDROMMULTISESSION, (unsigned long) &ms_info);
> -	if (session > 0)
> -		printk(KERN_ERR "ISOFS: Invalid session number\n");
> -#if 0
> -	printk(KERN_DEBUG "isofs.inode: CDROMMULTISESSION: rc=%d\n",i);
> -	if (i==0) {
> -		printk(KERN_DEBUG "isofs.inode: XA disk: %s\n",ms_info.xa_flag?"yes":"no");
> -		printk(KERN_DEBUG "isofs.inode: vol_desc_start = %d\n", ms_info.addr.lba);
> -	}
> -#endif
> -	if (i==0)
> +
> +	if (cdi) {
> +		struct cdrom_multisession ms_info;
> +
> +		ms_info.addr_format = CDROM_LBA;
> +		if (cdrom_multisession(cdi, &ms_info) == 0) {
>  #if WE_OBEY_THE_WRITTEN_STANDARDS
> -		if (ms_info.xa_flag) /* necessary for a valid ms_info.addr */
> +			/* necessary for a valid ms_info.addr */
> +			if (ms_info.xa_flag)
>  #endif
> -			vol_desc_start=ms_info.addr.lba;
> +				vol_desc_start = ms_info.addr.lba;
> +		}
> +	}
> +
>  	return vol_desc_start;
>  }
>  
> -- 
> 2.26.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: stop using ioctl_by_bdev for file system access to CDROMs v2
  2020-04-25  7:56 stop using ioctl_by_bdev for file system access to CDROMs v2 Christoph Hellwig
                   ` (6 preceding siblings ...)
  2020-04-25  7:57 ` [PATCH 7/7] udf: " Christoph Hellwig
@ 2020-04-28  6:53 ` Christoph Hellwig
  2020-05-04 16:42 ` Jens Axboe
  8 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2020-04-28  6:53 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel

Jens,

can you pick this up for the for-5.8/block tree?  We're about ready
to kill ioctl_by_bdev after this series.

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

* Re: [PATCH 5/7] hfsplus: stop using ioctl_by_bdev
  2020-04-25  7:57 ` [PATCH 5/7] hfsplus: stop using ioctl_by_bdev Christoph Hellwig
  2020-04-27  6:18   ` Hannes Reinecke
@ 2020-05-04 16:16   ` Jens Axboe
  2020-05-04 16:21     ` Christoph Hellwig
  1 sibling, 1 reply; 24+ messages in thread
From: Jens Axboe @ 2020-05-04 16:16 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel, Damien Le Moal

On 4/25/20 1:57 AM, Christoph Hellwig wrote:
>  	if (HFSPLUS_SB(sb)->session >= 0) {
> +		struct cdrom_tocentry te;
> +
> +		if (!cdi)
> +			return -EINVAL;
> +
>  		te.cdte_track = HFSPLUS_SB(sb)->session;
>  		te.cdte_format = CDROM_LBA;
> -		res = ioctl_by_bdev(sb->s_bdev,
> -			CDROMREADTOCENTRY, (unsigned long)&te);
> -		if (!res && (te.cdte_ctrl & CDROM_DATA_TRACK) == 4) {
> -			*start = (sector_t)te.cdte_addr.lba << 2;
> -			return 0;
> +		if (cdrom_read_tocentry(cdi, &te) ||
> +		    (te.cdte_ctrl & CDROM_DATA_TRACK) != 4) {
> +			pr_err("invalid session number or type of track\n");
> +			return -EINVAL;
>  		}

I must be missing something obvious from just looking over the patches,
but how does this work if cdrom is modular and hfsplus is builtin?

-- 
Jens Axboe


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

* Re: [PATCH 5/7] hfsplus: stop using ioctl_by_bdev
  2020-05-04 16:16   ` Jens Axboe
@ 2020-05-04 16:21     ` Christoph Hellwig
  2020-05-04 16:41       ` Jens Axboe
  0 siblings, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2020-05-04 16:21 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Tim Waugh, Borislav Petkov, Jan Kara,
	linux-block, linux-ide, linux-scsi, linux-fsdevel, linux-kernel,
	Damien Le Moal

On Mon, May 04, 2020 at 10:16:40AM -0600, Jens Axboe wrote:
> On 4/25/20 1:57 AM, Christoph Hellwig wrote:
> >  	if (HFSPLUS_SB(sb)->session >= 0) {
> > +		struct cdrom_tocentry te;
> > +
> > +		if (!cdi)
> > +			return -EINVAL;
> > +
> >  		te.cdte_track = HFSPLUS_SB(sb)->session;
> >  		te.cdte_format = CDROM_LBA;
> > -		res = ioctl_by_bdev(sb->s_bdev,
> > -			CDROMREADTOCENTRY, (unsigned long)&te);
> > -		if (!res && (te.cdte_ctrl & CDROM_DATA_TRACK) == 4) {
> > -			*start = (sector_t)te.cdte_addr.lba << 2;
> > -			return 0;
> > +		if (cdrom_read_tocentry(cdi, &te) ||
> > +		    (te.cdte_ctrl & CDROM_DATA_TRACK) != 4) {
> > +			pr_err("invalid session number or type of track\n");
> > +			return -EINVAL;
> >  		}
> 
> I must be missing something obvious from just looking over the patches,
> but how does this work if cdrom is modular and hfsplus is builtin?

In that case disk_to_cdi will return NULL as it uses IS_REACHABLE
and the file systems won't query the CD-ROM specific information.

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

* Re: [PATCH 5/7] hfsplus: stop using ioctl_by_bdev
  2020-05-04 16:21     ` Christoph Hellwig
@ 2020-05-04 16:41       ` Jens Axboe
  0 siblings, 0 replies; 24+ messages in thread
From: Jens Axboe @ 2020-05-04 16:41 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel, Damien Le Moal

On 5/4/20 10:21 AM, Christoph Hellwig wrote:
> On Mon, May 04, 2020 at 10:16:40AM -0600, Jens Axboe wrote:
>> On 4/25/20 1:57 AM, Christoph Hellwig wrote:
>>>  	if (HFSPLUS_SB(sb)->session >= 0) {
>>> +		struct cdrom_tocentry te;
>>> +
>>> +		if (!cdi)
>>> +			return -EINVAL;
>>> +
>>>  		te.cdte_track = HFSPLUS_SB(sb)->session;
>>>  		te.cdte_format = CDROM_LBA;
>>> -		res = ioctl_by_bdev(sb->s_bdev,
>>> -			CDROMREADTOCENTRY, (unsigned long)&te);
>>> -		if (!res && (te.cdte_ctrl & CDROM_DATA_TRACK) == 4) {
>>> -			*start = (sector_t)te.cdte_addr.lba << 2;
>>> -			return 0;
>>> +		if (cdrom_read_tocentry(cdi, &te) ||
>>> +		    (te.cdte_ctrl & CDROM_DATA_TRACK) != 4) {
>>> +			pr_err("invalid session number or type of track\n");
>>> +			return -EINVAL;
>>>  		}
>>
>> I must be missing something obvious from just looking over the patches,
>> but how does this work if cdrom is modular and hfsplus is builtin?
> 
> In that case disk_to_cdi will return NULL as it uses IS_REACHABLE
> and the file systems won't query the CD-ROM specific information.

Got it, looks like that'll do the trick without nasty Kconfig
dependencies.

-- 
Jens Axboe


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

* Re: stop using ioctl_by_bdev for file system access to CDROMs v2
  2020-04-25  7:56 stop using ioctl_by_bdev for file system access to CDROMs v2 Christoph Hellwig
                   ` (7 preceding siblings ...)
  2020-04-28  6:53 ` stop using ioctl_by_bdev for file system access to CDROMs v2 Christoph Hellwig
@ 2020-05-04 16:42 ` Jens Axboe
  8 siblings, 0 replies; 24+ messages in thread
From: Jens Axboe @ 2020-05-04 16:42 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel

On 4/25/20 1:56 AM, Christoph Hellwig wrote:
> Hi Jens,
> 
> except for the DASD case under discussion the last users of ioctl_by_bdev
> are the file system drivers that want to query CDROM information using
> ioctls.  This series switches them to use function calls directly into
> the CDROM midlayer instead, which implies:
> 
>  - adding a cdrom_device_info pointer to the gendisk, so that file systems
>    can find it without going to the low-level driver first
>  - ensuring that the CDROM midlayer (which isn't a lot of code) is built
>    in if the file systems are built in so that they can actually call the
>    exported functions
> 
> Changes since v1:
>  - fix up the no-CDROM error case in isofs_get_last_session to return 0
>    instead of -EINVAL.

Applied for 5.8, thanks.

-- 
Jens Axboe


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

* Re: [PATCH 7/7] udf: stop using ioctl_by_bdev
  2020-04-23  7:12 ` [PATCH 7/7] udf: stop using ioctl_by_bdev Christoph Hellwig
  2020-04-23  7:43   ` Damien Le Moal
@ 2020-04-23 11:05   ` Jan Kara
  1 sibling, 0 replies; 24+ messages in thread
From: Jan Kara @ 2020-04-23 11:05 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Tim Waugh, Borislav Petkov, Jan Kara, linux-block,
	linux-ide, linux-scsi, linux-fsdevel, linux-kernel

On Thu 23-04-20 09:12:24, Christoph Hellwig wrote:
> Instead just call the CD-ROM layer functionality directly.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

The patch looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/udf/lowlevel.c | 29 +++++++++++++----------------
>  1 file changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/udf/lowlevel.c b/fs/udf/lowlevel.c
> index 5c7ec121990d..f1094cdcd6cd 100644
> --- a/fs/udf/lowlevel.c
> +++ b/fs/udf/lowlevel.c
> @@ -27,41 +27,38 @@
>  
>  unsigned int udf_get_last_session(struct super_block *sb)
>  {
> +	struct cdrom_device_info *cdi = disk_to_cdi(sb->s_bdev->bd_disk);
>  	struct cdrom_multisession ms_info;
> -	unsigned int vol_desc_start;
> -	struct block_device *bdev = sb->s_bdev;
> -	int i;
>  
> -	vol_desc_start = 0;
> -	ms_info.addr_format = CDROM_LBA;
> -	i = ioctl_by_bdev(bdev, CDROMMULTISESSION, (unsigned long)&ms_info);
> +	if (!cdi) {
> +		udf_debug("CDROMMULTISESSION not supported.\n");
> +		return 0;
> +	}
>  
> -	if (i == 0) {
> +	ms_info.addr_format = CDROM_LBA;
> +	if (cdrom_multisession(cdi, &ms_info) == 0) {
>  		udf_debug("XA disk: %s, vol_desc_start=%d\n",
>  			  ms_info.xa_flag ? "yes" : "no", ms_info.addr.lba);
>  		if (ms_info.xa_flag) /* necessary for a valid ms_info.addr */
> -			vol_desc_start = ms_info.addr.lba;
> -	} else {
> -		udf_debug("CDROMMULTISESSION not supported: rc=%d\n", i);
> +			return ms_info.addr.lba;
>  	}
> -	return vol_desc_start;
> +	return 0;
>  }
>  
>  unsigned long udf_get_last_block(struct super_block *sb)
>  {
>  	struct block_device *bdev = sb->s_bdev;
> +	struct cdrom_device_info *cdi = disk_to_cdi(bdev->bd_disk);
>  	unsigned long lblock = 0;
>  
>  	/*
> -	 * ioctl failed or returned obviously bogus value?
> +	 * The cdrom layer call failed or returned obviously bogus value?
>  	 * Try using the device size...
>  	 */
> -	if (ioctl_by_bdev(bdev, CDROM_LAST_WRITTEN, (unsigned long) &lblock) ||
> -	    lblock == 0)
> +	if (!cdi || cdrom_get_last_written(cdi, &lblock) || lblock == 0)
>  		lblock = i_size_read(bdev->bd_inode) >> sb->s_blocksize_bits;
>  
>  	if (lblock)
>  		return lblock - 1;
> -	else
> -		return 0;
> +	return 0;
>  }
> -- 
> 2.26.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: [PATCH 7/7] udf: stop using ioctl_by_bdev
  2020-04-23  7:12 ` [PATCH 7/7] udf: stop using ioctl_by_bdev Christoph Hellwig
@ 2020-04-23  7:43   ` Damien Le Moal
  2020-04-23 11:05   ` Jan Kara
  1 sibling, 0 replies; 24+ messages in thread
From: Damien Le Moal @ 2020-04-23  7:43 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel

On 2020/04/23 16:15, Christoph Hellwig wrote:
> Instead just call the CD-ROM layer functionality directly.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/udf/lowlevel.c | 29 +++++++++++++----------------
>  1 file changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/udf/lowlevel.c b/fs/udf/lowlevel.c
> index 5c7ec121990d..f1094cdcd6cd 100644
> --- a/fs/udf/lowlevel.c
> +++ b/fs/udf/lowlevel.c
> @@ -27,41 +27,38 @@
>  
>  unsigned int udf_get_last_session(struct super_block *sb)
>  {
> +	struct cdrom_device_info *cdi = disk_to_cdi(sb->s_bdev->bd_disk);
>  	struct cdrom_multisession ms_info;
> -	unsigned int vol_desc_start;
> -	struct block_device *bdev = sb->s_bdev;
> -	int i;
>  
> -	vol_desc_start = 0;
> -	ms_info.addr_format = CDROM_LBA;
> -	i = ioctl_by_bdev(bdev, CDROMMULTISESSION, (unsigned long)&ms_info);
> +	if (!cdi) {
> +		udf_debug("CDROMMULTISESSION not supported.\n");
> +		return 0;
> +	}
>  
> -	if (i == 0) {
> +	ms_info.addr_format = CDROM_LBA;
> +	if (cdrom_multisession(cdi, &ms_info) == 0) {
>  		udf_debug("XA disk: %s, vol_desc_start=%d\n",
>  			  ms_info.xa_flag ? "yes" : "no", ms_info.addr.lba);
>  		if (ms_info.xa_flag) /* necessary for a valid ms_info.addr */
> -			vol_desc_start = ms_info.addr.lba;
> -	} else {
> -		udf_debug("CDROMMULTISESSION not supported: rc=%d\n", i);
> +			return ms_info.addr.lba;
>  	}
> -	return vol_desc_start;
> +	return 0;
>  }
>  
>  unsigned long udf_get_last_block(struct super_block *sb)
>  {
>  	struct block_device *bdev = sb->s_bdev;
> +	struct cdrom_device_info *cdi = disk_to_cdi(bdev->bd_disk);
>  	unsigned long lblock = 0;
>  
>  	/*
> -	 * ioctl failed or returned obviously bogus value?
> +	 * The cdrom layer call failed or returned obviously bogus value?
>  	 * Try using the device size...
>  	 */
> -	if (ioctl_by_bdev(bdev, CDROM_LAST_WRITTEN, (unsigned long) &lblock) ||
> -	    lblock == 0)
> +	if (!cdi || cdrom_get_last_written(cdi, &lblock) || lblock == 0)
>  		lblock = i_size_read(bdev->bd_inode) >> sb->s_blocksize_bits;
>  
>  	if (lblock)
>  		return lblock - 1;
> -	else
> -		return 0;
> +	return 0;
>  }
> 

Looks OK to me.

Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>

-- 
Damien Le Moal
Western Digital Research

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

* [PATCH 7/7] udf: stop using ioctl_by_bdev
  2020-04-23  7:12 stop using ioctl_by_bdev for file system access to CDROMs Christoph Hellwig
@ 2020-04-23  7:12 ` Christoph Hellwig
  2020-04-23  7:43   ` Damien Le Moal
  2020-04-23 11:05   ` Jan Kara
  0 siblings, 2 replies; 24+ messages in thread
From: Christoph Hellwig @ 2020-04-23  7:12 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Tim Waugh, Borislav Petkov, Jan Kara, linux-block, linux-ide,
	linux-scsi, linux-fsdevel, linux-kernel

Instead just call the CD-ROM layer functionality directly.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/udf/lowlevel.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/fs/udf/lowlevel.c b/fs/udf/lowlevel.c
index 5c7ec121990d..f1094cdcd6cd 100644
--- a/fs/udf/lowlevel.c
+++ b/fs/udf/lowlevel.c
@@ -27,41 +27,38 @@
 
 unsigned int udf_get_last_session(struct super_block *sb)
 {
+	struct cdrom_device_info *cdi = disk_to_cdi(sb->s_bdev->bd_disk);
 	struct cdrom_multisession ms_info;
-	unsigned int vol_desc_start;
-	struct block_device *bdev = sb->s_bdev;
-	int i;
 
-	vol_desc_start = 0;
-	ms_info.addr_format = CDROM_LBA;
-	i = ioctl_by_bdev(bdev, CDROMMULTISESSION, (unsigned long)&ms_info);
+	if (!cdi) {
+		udf_debug("CDROMMULTISESSION not supported.\n");
+		return 0;
+	}
 
-	if (i == 0) {
+	ms_info.addr_format = CDROM_LBA;
+	if (cdrom_multisession(cdi, &ms_info) == 0) {
 		udf_debug("XA disk: %s, vol_desc_start=%d\n",
 			  ms_info.xa_flag ? "yes" : "no", ms_info.addr.lba);
 		if (ms_info.xa_flag) /* necessary for a valid ms_info.addr */
-			vol_desc_start = ms_info.addr.lba;
-	} else {
-		udf_debug("CDROMMULTISESSION not supported: rc=%d\n", i);
+			return ms_info.addr.lba;
 	}
-	return vol_desc_start;
+	return 0;
 }
 
 unsigned long udf_get_last_block(struct super_block *sb)
 {
 	struct block_device *bdev = sb->s_bdev;
+	struct cdrom_device_info *cdi = disk_to_cdi(bdev->bd_disk);
 	unsigned long lblock = 0;
 
 	/*
-	 * ioctl failed or returned obviously bogus value?
+	 * The cdrom layer call failed or returned obviously bogus value?
 	 * Try using the device size...
 	 */
-	if (ioctl_by_bdev(bdev, CDROM_LAST_WRITTEN, (unsigned long) &lblock) ||
-	    lblock == 0)
+	if (!cdi || cdrom_get_last_written(cdi, &lblock) || lblock == 0)
 		lblock = i_size_read(bdev->bd_inode) >> sb->s_blocksize_bits;
 
 	if (lblock)
 		return lblock - 1;
-	else
-		return 0;
+	return 0;
 }
-- 
2.26.1


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

end of thread, other threads:[~2020-05-04 16:42 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-25  7:56 stop using ioctl_by_bdev for file system access to CDROMs v2 Christoph Hellwig
2020-04-25  7:57 ` [PATCH 1/7] block: add a cdrom_device_info pointer to struct gendisk Christoph Hellwig
2020-04-27  6:15   ` Hannes Reinecke
2020-04-25  7:57 ` [PATCH 2/7] ide-cd: rename cdrom_read_tocentry Christoph Hellwig
2020-04-27  6:16   ` Hannes Reinecke
2020-04-25  7:57 ` [PATCH 3/7] cdrom: factor out a cdrom_read_tocentry helper Christoph Hellwig
2020-04-27  6:17   ` Hannes Reinecke
2020-04-25  7:57 ` [PATCH 4/7] cdrom: factor out a cdrom_multisession helper Christoph Hellwig
2020-04-27  6:17   ` Hannes Reinecke
2020-04-25  7:57 ` [PATCH 5/7] hfsplus: stop using ioctl_by_bdev Christoph Hellwig
2020-04-27  6:18   ` Hannes Reinecke
2020-05-04 16:16   ` Jens Axboe
2020-05-04 16:21     ` Christoph Hellwig
2020-05-04 16:41       ` Jens Axboe
2020-04-25  7:57 ` [PATCH 6/7] isofs: " Christoph Hellwig
2020-04-27  6:18   ` Hannes Reinecke
2020-04-27  9:50   ` Jan Kara
2020-04-25  7:57 ` [PATCH 7/7] udf: " Christoph Hellwig
2020-04-27  6:18   ` Hannes Reinecke
2020-04-28  6:53 ` stop using ioctl_by_bdev for file system access to CDROMs v2 Christoph Hellwig
2020-05-04 16:42 ` Jens Axboe
  -- strict thread matches above, loose matches on Subject: below --
2020-04-23  7:12 stop using ioctl_by_bdev for file system access to CDROMs Christoph Hellwig
2020-04-23  7:12 ` [PATCH 7/7] udf: stop using ioctl_by_bdev Christoph Hellwig
2020-04-23  7:43   ` Damien Le Moal
2020-04-23 11:05   ` Jan Kara

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