On Sun, Mar 08, 2020 at 04:14:26PM +0000, Jonathan Cameron wrote: > On Sun, 8 Mar 2020 02:32:56 +0530 > Rohit Sarkar <rohitsarkar5398@xxxxxxxxx> wrote: > > > On Sat, Mar 07, 2020 at 02:19:46PM +0000, Jonathan Cameron wrote: > > > On Sat, 7 Mar 2020 13:34:51 +0530 > > > Rohit Sarkar <rohitsarkar5398@xxxxxxxxx> wrote: > > > > > > > Replace usage indio_dev's mlock with either local lock or > > > > iio_device_claim_direct_mode. > > > > > > > > Signed-off-by: Rohit Sarkar <rohitsarkar5398@xxxxxxxxx> > > > > > > There is a subtlety in here (which is why this one never > > > got cleaned up before). We need to protect against: > > > > > > 1) Driver state being accessed from multiple places concurrently. > > > That will use your new lock. > > > 2) Doing actions that cannot occur if in buffered mode. The > > > claim_direct_mode stuff is for that. > > I did consider using both, the local driver lock and the claim_direct in > > some places, however I noticed that the claim_direct_mode internally uses > > the mlock, hence I didnt think it was necessary to set the local lock as > > well, as according to my understanding once a process acquires the mlock > > no other process can run the critical section before the initial process > > releases the mlock. Thus the driver state also remains consistent. > > Any state changes in the driver done under the local lock can still happen. > There is also a question of 'obviousness'. The driver code should not > 'care' what the internals of claim_direct_mode is doing. > That can be expected to protect against moving out of direct mode, but > not anything about 'how'. > > Hence, take them both. That does make sense, when I thought about it again I realised doing this is essentially what was wrong with the code in the first place. Thanks for the pointers. Will send out an update. Thanks, Rohit
