Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: gOHZ2TBbHsjMXSjQMyCNJsf88fHgRsb+87CytUeoRHmR/nvY8V7Xiz11ih4moL2WdFMYWgfEC3
 /W2zxmOMnonbYmBydKqceKkJr7oR2T7l0sLmfe5ATIaVEvWUe0o0ENueXl+bwDC/SFw4JFQguN
 WwWqHkEIWxPcWgKnN0YB3o+K4brQsLFccEWXetuvAhHwssL6G819BH4lGSO1BEwL8W905rWjD/
 28iTFwoCq9jDZpTcI1ZgdODQ0rt/cPsu0g9/TMv5HAOg9QdTznLsa8xv+K91QMMYCCvIt1cCI6
 iPM=
From:   Damien Le Moal <Damien.LeMoal@wdc.com>
To:     Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>
CC:     Tim Waugh <tim@cyberelk.net>, Borislav Petkov <bp@alien8.de>,
        Jan Kara <jack@suse.com>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        "linux-ide@vger.kernel.org" <linux-ide@vger.kernel.org>,
        "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 5/7] hfsplus: stop using ioctl_by_bdev
Thread-Topic: [PATCH 5/7] hfsplus: stop using ioctl_by_bdev
Thread-Index: AQHWGT8NPdskV3ohNU+tD+3v4fgM/A==
Date:   Thu, 23 Apr 2020 07:42:26 +0000
Message-ID: <BY5PR04MB690068E22B67AB977BDB8046E7D30@BY5PR04MB6900.namprd04.prod.outlook.com>
References: <20200423071224.500849-1-hch@lst.de>
 <20200423071224.500849-6-hch@lst.de>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: fa7290e9-9e7d-4f27-57dd-08d7e759de6b
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Apr 2020 07:42:26.3895
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: aIdRNDFjtTaVUjviDigg0SsBNycixSqx4yjVqG0ZcEsjBIOF/8fzVdDg156xvsBPk/5xoPatcLOloAXHsp2igQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR04MB6723
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2020/04/23 16:16, Christoph Hellwig wrote:=0A=
> Instead just call the CD-ROM layer functionality directly.=0A=
> =0A=
> Signed-off-by: Christoph Hellwig <hch@lst.de>=0A=
> ---=0A=
>  fs/hfsplus/wrapper.c | 33 ++++++++++++++++++---------------=0A=
>  1 file changed, 18 insertions(+), 15 deletions(-)=0A=
> =0A=
> diff --git a/fs/hfsplus/wrapper.c b/fs/hfsplus/wrapper.c=0A=
> index 08c1580bdf7a..61eec628805d 100644=0A=
> --- a/fs/hfsplus/wrapper.c=0A=
> +++ b/fs/hfsplus/wrapper.c=0A=
> @@ -127,31 +127,34 @@ static int hfsplus_read_mdb(void *bufptr, struct hf=
splus_wd *wd)=0A=
>  static int hfsplus_get_last_session(struct super_block *sb,=0A=
>  				    sector_t *start, sector_t *size)=0A=
>  {=0A=
> -	struct cdrom_multisession ms_info;=0A=
> -	struct cdrom_tocentry te;=0A=
> -	int res;=0A=
> +	struct cdrom_device_info *cdi =3D disk_to_cdi(sb->s_bdev->bd_disk);=0A=
>  =0A=
>  	/* default values */=0A=
>  	*start =3D 0;=0A=
>  	*size =3D i_size_read(sb->s_bdev->bd_inode) >> 9;=0A=
>  =0A=
>  	if (HFSPLUS_SB(sb)->session >=3D 0) {=0A=
> +		struct cdrom_tocentry te;=0A=
> +=0A=
> +		if (!cdi)=0A=
> +			return -EINVAL;=0A=
> +=0A=
>  		te.cdte_track =3D HFSPLUS_SB(sb)->session;=0A=
>  		te.cdte_format =3D CDROM_LBA;=0A=
> -		res =3D ioctl_by_bdev(sb->s_bdev,=0A=
> -			CDROMREADTOCENTRY, (unsigned long)&te);=0A=
> -		if (!res && (te.cdte_ctrl & CDROM_DATA_TRACK) =3D=3D 4) {=0A=
> -			*start =3D (sector_t)te.cdte_addr.lba << 2;=0A=
> -			return 0;=0A=
> +		if (cdrom_read_tocentry(cdi, &te) ||=0A=
> +		    (te.cdte_ctrl & CDROM_DATA_TRACK) !=3D 4) {=0A=
> +			pr_err("invalid session number or type of track\n");=0A=
> +			return -EINVAL;=0A=
>  		}=0A=
> -		pr_err("invalid session number or type of track\n");=0A=
> -		return -EINVAL;=0A=
> +		*start =3D (sector_t)te.cdte_addr.lba << 2;=0A=
> +	} else if (cdi) {=0A=
> +		struct cdrom_multisession ms_info;=0A=
> +=0A=
> +		ms_info.addr_format =3D CDROM_LBA;=0A=
> +		if (cdrom_multisession(cdi, &ms_info) =3D=3D 0 && ms_info.xa_flag)=0A=
> +			*start =3D (sector_t)ms_info.addr.lba << 2;=0A=
>  	}=0A=
> -	ms_info.addr_format =3D CDROM_LBA;=0A=
> -	res =3D ioctl_by_bdev(sb->s_bdev, CDROMMULTISESSION,=0A=
> -		(unsigned long)&ms_info);=0A=
> -	if (!res && ms_info.xa_flag)=0A=
> -		*start =3D (sector_t)ms_info.addr.lba << 2;=0A=
> +=0A=
=0A=
White space change here, but I think it's ok. I do like a blank line before=
=0A=
return :)=0A=
=0A=
>  	return 0;=0A=
>  }=0A=
>  =0A=
> =0A=
=0A=
Looks OK to me.=0A=
=0A=
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>=0A=
=0A=
=0A=
-- =0A=
Damien Le Moal=0A=
Western Digital Research=0A=