On Wed, Jan 06 2021, Marcus Glocker <mar...@nazgul.ch> wrote: > On Tue, Jan 05, 2021 at 11:54:31PM +0100, Jeremie Courreges-Anglas wrote:
[...] >> Here's the diff. IIUC the use of atomic operations isn't strictly >> needed here since open(2) runs with the kernel lock, but the result >> is easy to understand IMO. > > I don't agree with that, see my comments bellow in point a) and b). > >> Thoughts? ok? > > I think it doesn't harm to allow the same process to do multiple opens > on video(1) as a first improvement. But I'm not happy using > atomic_cas_ptr(9) and atomic_swap_ptr(9) for this because: > > a) As you already have mentioned, we don't require atomic > operations here. I checked that those functions are very > rarely used in the kernel, and only there were atomic > operation is required. I'm afraid that when we use those in > video(1), and somebody looks at the code later on to > eventually replace it, there will be confusion if there was > a specific reason why to use atomic functions. > > b) IMO it doesn't simplify the code. I first had to read the > manual pages to understand how those functions work. Fair enough, I also felt that it was a bit premature. > I would prefer to do something simpler, like in this adapted diff. > If it works for you, I'm OK if you commit this instead. Done, thanks. -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
