Messages in this thread | | | Date | Wed, 01 Apr 2015 10:10:53 +0100 | From | Ian Abbott <> | Subject | Re: [PATCH 7/7] staging: comedi: comedi_fops: extend spin-lock scope in comedi_event() |
| |
On 31/03/15 17:13, Hartley Sweeten wrote: > On Tuesday, March 31, 2015 2:43 AM, Ian Abbott wrote: >> On 30/03/15 17:47, Hartley Sweeten wrote: >>> On Friday, March 27, 2015 8:13 AM, Ian Abbott wrote: >>>> `comedi_event()` is called from low-level drivers to handle comedi >>>> asynchronous command event flags. As a safety check, it checks the >>>> subdevice's "run" flags to make sure an asynchronous command is running. >>>> It can also change the run flags to mark the command as no longer >>>> running (possibly also marking it as terminated with an error). >>>> Checking the runflags and modifying them involves two uses of the >>>> subdevice's spin-lock. It seems better to do it with a single use of >>>> the spin-lock. This also avoids possible interactions with >>>> `do_become_nonbusy()`. >>>> >>>> Acquire the subdevice's spin-lock at the start of `comedi_event()` and >>>> release it near the end, before a possible call to `kill_fasync()` (but >>>> after it's parameter values have been determined). >>>> >>>> Add and make use of few new inline helper functions: >>>> >>>> * `__comedi_clear_subdevice_runflags()` -- clears some run flags without >>>> using the spin-lock >>>> * `__comedi_set_subdevice_runflags()` -- sets some run flags without >>>> using the spin-lock >>>> * `__comedi_get_subdevice_runflags()` -- a spin-lockless version of >>>> `comedi_get_subdevice_runflags() >>>> * `__comedi_is_subdevice_running()` -- a spin-lockless version of >>>> * `comedi_is_subdevice_running()` >>>> >>>> Signed-off-by: Ian Abbott <abbotti@mev.co.uk> >>> >>> Ian, >>> >>> For completeness, the comedi_alloc_spriv() helper should probably use >>> __comedi_set_subdevice_runflags() to set the COMEDI_SRF_FREE_SPRIV >>> bit. >> >> Good point. "drivers/staging/comedi/drivers.c" also reads the runflags >> directly, so perhaps __comedi_clear_subdevice_runflags(), >> __comedi_set_subdevice_runflags() and __comedi_get_subdevice_runflags() >> should be placed in "drivers/staging/comedi/comedi_internal.h". Or we >> could ditch all three of those inline functions as they are just simple >> one-liners. > > comedi_internal.h works for mw. We just don't want to expose those functions > to the drivers.
Okay, I'll do that in a separate patch since you've already signed off on this one.
-- -=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@mev.co.uk> )=- -=( Web: http://www.mev.co.uk/ )=-
| |