On Tue, 2020-02-18 at 09:23 -0800, Christoph Hellwig wrote: > On Tue, Feb 18, 2020 at 09:20:28AM -0800, James Bottomley wrote: > > > > Replace the global mutex with per-sr-device mutex. > > > > > > Do we actually need the lock at all? What is protected by it? > > > > We do at least for cdrom_open. It modifies the cdi structure with > > no other protection and concurrent modification would at least > > screw up the use counter which is not atomic. Same reasoning for > > cdrom_release. > > Wouldn't the right fix to add locking to cdrom_open/release instead > of having an undocumented requirement for the callers? Yes ... but that's somewhat of a bigger patch because you now have to reason about the callbacks within cdrom. There's also the question of whether you can assume ops->generic_packet() has its own concurrency protections ... it's certainly true for SCSI, but is it for anything else? Although I suppose you can just not care and run the internal lock over it anyway. James
