2020-04-25 07:59:14

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 5/7] hfsplus: stop using ioctl_by_bdev

Instead just call the CDROM layer functionality directly.

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


2020-04-27 06:20:18

by Hannes Reinecke

[permalink] [raw]
Subject: Re: [PATCH 5/7] hfsplus: stop using ioctl_by_bdev

On 4/25/20 9:57 AM, Christoph Hellwig wrote:
> Instead just call the CDROM layer functionality directly.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> Reviewed-by: Damien Le Moal <[email protected]>
> ---
> fs/hfsplus/wrapper.c | 33 ++++++++++++++++++---------------
> 1 file changed, 18 insertions(+), 15 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-05-04 16:18:57

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 5/7] hfsplus: stop using ioctl_by_bdev

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

2020-05-04 16:23:53

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 5/7] hfsplus: stop using ioctl_by_bdev

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.

2020-05-04 16:45:49

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 5/7] hfsplus: stop using ioctl_by_bdev

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