On Wed, 2019-03-20 at 12:39 +0100, Hannes Reinecke wrote:
> The 'ch_mutex" is meant to guard against modifications of
> file->private_data, so we need to take in in ch_release() as
> well as in ch_open().
>
> Signed-off-by: Hannes Reinecke <h...@suse.de>
> ---
> drivers/scsi/ch.c | 2 ﭗ橸ṷ梧뇪觬()
>
> diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
> index 1c5051b1c125..8f426903d7e4 100644
> --- a/drivers/scsi/ch.c
> +++ b/drivers/scsi/ch.c
> @@ -577,9 +577,11 @@ ch_release(struct inode *inode, struct file *file)
> {
> scsi_changer *ch = file->private_data;
>
> + mutex_lock(&ch_mutex);
> scsi_device_put(ch->device);
> ch->device = NULL;
> file->private_data = NULL;
> + mutex_unlock(&ch_mutex);
> kref_put(&ch->ref, ch_destroy);
> return 0;
> }
Hi Hannes,
My interpretation of the open() syscall code (do_sys_open()) is that a new
struct file is allocated every time the open() is called. Does that mean that
the lock and unlock calls for ch_mutex can be removed from ch_open()?
Regarding the above patch: why do you think that file->private_data changes
need to be serialized? I don't know any other file_operations::open / close
callback implementations that implement similar serialization.
Thanks,
Bart.
