| Message ID | 20230606073950.225178-6-hch@lst.de (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [01/31] block: also call ->open for incremental partition opens | expand |
On Tue, Jun 06, 2023 at 09:39:24AM +0200, 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 { > -- > 2.39.2 > Looks good, thanks. Signed-off-by: Phillip Potter <phil@philpotter.co.uk> Regards, Phil
On Tue, Jun 06, 2023 at 09:39:24AM +0200, 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> > --- Looks good to me, Acked-by: Christian Brauner <brauner@kernel.org>
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
On Wed, Jun 07, 2023 at 02:13:33PM +0200, Hannes Reinecke wrote: >> +++ 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 ... I don't think the structure matter much here, but placing it before last_media_change_ms still seems better. I'll change it.
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 {
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(-)