From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752431AbbDAJK7 (ORCPT ); Wed, 1 Apr 2015 05:10:59 -0400 Received: from smtp105.iad3a.emailsrvr.com ([173.203.187.105]:50788 "EHLO smtp105.iad3a.emailsrvr.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751005AbbDAJK4 (ORCPT ); Wed, 1 Apr 2015 05:10:56 -0400 X-Sender-Id: abbotti@mev.co.uk Message-ID: <551BB61D.8090306@mev.co.uk> Date: Wed, 01 Apr 2015 10:10:53 +0100 From: Ian Abbott User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.5.0 MIME-Version: 1.0 To: Hartley Sweeten , "driverdev-devel@linuxdriverproject.org" CC: Greg Kroah-Hartman , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH 7/7] staging: comedi: comedi_fops: extend spin-lock scope in comedi_event() References: <1427469186-11222-1-git-send-email-abbotti@mev.co.uk> <1427469186-11222-8-git-send-email-abbotti@mev.co.uk> <551A6C22.7050301@mev.co.uk> In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >>> >>> 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: )=- -=( Web: http://www.mev.co.uk/ )=-