Re: [PATCH 05/31] cdrom: track if a cdrom_device_info was opened for data

Hannes Reinecke
2023-06-07T12:14:09Z

On 6/6/23 09:39, Christoph Hellwig wrote:
> Set a flag when a cdrom_device_info is opened for writing, instead of
> trying to figure out this at release time. This will allow to eventually
> remove the mode argument to the ->release block_device_operation as
> nothing but the CDROM drivers uses that argument.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/cdrom/cdrom.c | 12 +++++-------
> include/linux/cdrom.h | 1 +
> 2 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index 08abf1ffede002..adebac1bd210d9 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -1172,6 +1172,7 @@ int cdrom_open(struct cdrom_device_info *cdi, fmode_t mode)
> ret = 0;
> cdi->media_written = 0;
> }
> + cdi->opened_for_data = true;
> }
>
> if (ret)
> @@ -1252,7 +1253,6 @@ static int check_for_audio_disc(struct cdrom_device_info *cdi,
> void cdrom_release(struct cdrom_device_info *cdi, fmode_t mode)
> {
> const struct cdrom_device_ops *cdo = cdi->ops;
> - int opened_for_data;
>
> cd_dbg(CD_CLOSE, "entering cdrom_release\n");
>
> @@ -1270,14 +1270,12 @@ void cdrom_release(struct cdrom_device_info *cdi, fmode_t mode)
> }
> }
>
> - opened_for_data = !(cdi->options & CDO_USE_FFLAGS) ||
> - !(mode & FMODE_NDELAY);
> -
> cdo->release(cdi);
> - if (cdi->use_count == 0) { /* last process that closes dev*/
> - if (opened_for_data &&
> - cdi->options & CDO_AUTO_EJECT && CDROM_CAN(CDC_OPEN_TRAY))
> +
> + if (cdi->use_count == 0 && cdi->opened_for_data) {
> + if (cdi->options & CDO_AUTO_EJECT && CDROM_CAN(CDC_OPEN_TRAY))
> cdo->tray_move(cdi, 1);
> + cdi->opened_for_data = false;
> }
> }
> EXPORT_SYMBOL(cdrom_release);
> diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
> index 0a5db0b0c958a1..385e94732b2cf1 100644
> --- a/include/linux/cdrom.h
> +++ b/include/linux/cdrom.h
> @@ -64,6 +64,7 @@ struct cdrom_device_info {
> int (*exit)(struct cdrom_device_info *);
> int mrw_mode_page;
> __s64 last_media_change_ms;
> + bool opened_for_data;
> };
>
> struct cdrom_device_ops {

Do we care about alignment here?
integer followed by a 64 bit value followed by a bool seems
like an automatic padding to me ...

Cheers,

Hannes