2020-04-25 07:59:14

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 6/7] isofs: stop using ioctl_by_bdev

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 <[email protected]>
Reviewed-by: Damien Le Moal <[email protected]>
---
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


2020-04-27 06:21:04

by Hannes Reinecke

[permalink] [raw]
Subject: Re: [PATCH 6/7] isofs: stop using ioctl_by_bdev

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 <[email protected]>
> Reviewed-by: Damien Le Moal <[email protected]>
> ---
> fs/isofs/inode.c | 54 +++++++++++++++++++++++-------------------------
> 1 file changed, 26 insertions(+), 28 deletions(-)
>
Reviewed-by: Hannes Reinecke <[email protected]>

Cheers,

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

2020-04-27 09:52:48

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 6/7] isofs: stop using ioctl_by_bdev

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 <[email protected]>
> Reviewed-by: Damien Le Moal <[email protected]>

Looks good to me. You can add:

Reviewed-by: Jan Kara <[email protected]>

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 <[email protected]>
SUSE Labs, CR