From patchwork Tue Nov 3 05:44:46 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Torokhov X-Patchwork-Id: 57218 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id nA35j9oD029829 for ; Tue, 3 Nov 2009 05:45:10 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750998AbZKCFos (ORCPT ); Tue, 3 Nov 2009 00:44:48 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1750852AbZKCFos (ORCPT ); Tue, 3 Nov 2009 00:44:48 -0500 Received: from mail-pw0-f42.google.com ([209.85.160.42]:58556 "EHLO mail-pw0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750791AbZKCFor (ORCPT ); Tue, 3 Nov 2009 00:44:47 -0500 Received: by pwj9 with SMTP id 9so2305947pwj.21 for ; Mon, 02 Nov 2009 21:44:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:date:from:to:cc:subject :message-id:references:mime-version:content-type:content-disposition :in-reply-to:user-agent; bh=RPcNbLYrXiOpDF5nQcfXJROTaq7BvVhuMLopDsgCse8=; b=pMu8SZ8pdMo99tPUNruneSgy2sWM6G4VXAaMC9RPNrm5VkVhyYGPUbmK8In1ds4uG1 NtU/3F1TnJpIuR87lPuMhBwPhM4yLZ0egHN4Gb8XcWdIH2v1n+jDehb059tk/IOo4qhv 0nUsozkJhtytV83QbQP90AypBZ/goXUFSBeHw= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; b=MsVCpLWdri8N3svPDkIlVWC8QYmAshDTJgHe/NEeBaXePcoT47AeZD/EGyKNvNDQCj CXfywAtw0TBsqoRCaJWb26aeJcY/MMZ8kBlpAWn1sSQY3y//AFySOMqd8lS/EKOCvv/V 1sIK9r6Z6Wccq7MV477xIGZwrwQoP71VoPoGg= Received: by 10.115.116.37 with SMTP id t37mr10250943wam.79.1257227092274; Mon, 02 Nov 2009 21:44:52 -0800 (PST) Received: from mailhub.coreip.homeip.net (c-24-6-153-137.hsd1.ca.comcast.net [24.6.153.137]) by mx.google.com with ESMTPS id 22sm251638pzk.6.2009.11.02.21.44.49 (version=TLSv1/SSLv3 cipher=RC4-MD5); Mon, 02 Nov 2009 21:44:50 -0800 (PST) Date: Mon, 2 Nov 2009 21:44:46 -0800 From: Dmitry Torokhov To: Arjan van de Ven Cc: linux-kernel@vger.kernel.org, akpm@linux-foundation.org, Anssi Hannula , Jussi Kivilinna , linux-input@vger.kernel.org Subject: Re: [PATCH] input: fix locking context in ml_ff_set_gain Message-ID: <20091103054446.GB3212@core.coreip.homeip.net> References: <20091031141925.149c9874@infradead.org> <20091102063818.GB3354@core.coreip.homeip.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20091102063818.GB3354@core.coreip.homeip.net> User-Agent: Mutt/1.5.19 (2009-01-05) Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org diff --git a/drivers/input/ff-core.c b/drivers/input/ff-core.c index 72c63e5..38df81f 100644 --- a/drivers/input/ff-core.c +++ b/drivers/input/ff-core.c @@ -337,16 +337,16 @@ int input_ff_create(struct input_dev *dev, int max_effects) dev->ff = ff; dev->flush = flush_effects; dev->event = input_ff_event; - set_bit(EV_FF, dev->evbit); + __set_bit(EV_FF, dev->evbit); /* Copy "true" bits into ff device bitmap */ for (i = 0; i <= FF_MAX; i++) if (test_bit(i, dev->ffbit)) - set_bit(i, ff->ffbit); + __set_bit(i, ff->ffbit); /* we can emulate RUMBLE with periodic effects */ if (test_bit(FF_PERIODIC, ff->ffbit)) - set_bit(FF_RUMBLE, dev->ffbit); + __set_bit(FF_RUMBLE, dev->ffbit); return 0; } @@ -362,12 +362,14 @@ EXPORT_SYMBOL_GPL(input_ff_create); */ void input_ff_destroy(struct input_dev *dev) { - clear_bit(EV_FF, dev->evbit); - if (dev->ff) { - if (dev->ff->destroy) - dev->ff->destroy(dev->ff); - kfree(dev->ff->private); - kfree(dev->ff); + struct ff_device *ff = dev->ff; + + __clear_bit(EV_FF, dev->evbit); + if (ff) { + if (ff->destroy) + ff->destroy(ff); + kfree(ff->private); + kfree(ff); dev->ff = NULL; } } diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c index 2d1415e..2e8b4e3 100644 --- a/drivers/input/ff-memless.c +++ b/drivers/input/ff-memless.c @@ -61,7 +61,6 @@ struct ml_device { struct ml_effect_state states[FF_MEMLESS_EFFECTS]; int gain; struct timer_list timer; - spinlock_t timer_lock; struct input_dev *dev; int (*play_effect)(struct input_dev *dev, void *data, @@ -371,35 +370,34 @@ static void ml_effect_timer(unsigned long timer_data) debug("timer: updating effects"); - spin_lock(&ml->timer_lock); + spin_lock_irq(&dev->event_lock); ml_play_effects(ml); - spin_unlock(&ml->timer_lock); + spin_unlock_irq(&dev->event_lock); } +/* + * Sets requested gain for FF effects. Called with dev->event_lock held. + */ static void ml_ff_set_gain(struct input_dev *dev, u16 gain) { struct ml_device *ml = dev->ff->private; int i; - spin_lock_bh(&ml->timer_lock); - ml->gain = gain; for (i = 0; i < FF_MEMLESS_EFFECTS; i++) __clear_bit(FF_EFFECT_PLAYING, &ml->states[i].flags); ml_play_effects(ml); - - spin_unlock_bh(&ml->timer_lock); } +/* + * Start/stop specified FF effect. Called with dev->event_lock held. + */ static int ml_ff_playback(struct input_dev *dev, int effect_id, int value) { struct ml_device *ml = dev->ff->private; struct ml_effect_state *state = &ml->states[effect_id]; - unsigned long flags; - - spin_lock_irqsave(&ml->timer_lock, flags); if (value > 0) { debug("initiated play"); @@ -425,8 +423,6 @@ static int ml_ff_playback(struct input_dev *dev, int effect_id, int value) ml_play_effects(ml); } - spin_unlock_irqrestore(&ml->timer_lock, flags); - return 0; } @@ -436,7 +432,7 @@ static int ml_ff_upload(struct input_dev *dev, struct ml_device *ml = dev->ff->private; struct ml_effect_state *state = &ml->states[effect->id]; - spin_lock_bh(&ml->timer_lock); + spin_lock_irq(&dev->event_lock); if (test_bit(FF_EFFECT_STARTED, &state->flags)) { __clear_bit(FF_EFFECT_PLAYING, &state->flags); @@ -448,7 +444,7 @@ static int ml_ff_upload(struct input_dev *dev, ml_schedule_timer(ml); } - spin_unlock_bh(&ml->timer_lock); + spin_unlock_irq(&dev->event_lock); return 0; } @@ -482,7 +478,6 @@ int input_ff_create_memless(struct input_dev *dev, void *data, ml->private = data; ml->play_effect = play_effect; ml->gain = 0xffff; - spin_lock_init(&ml->timer_lock); setup_timer(&ml->timer, ml_effect_timer, (unsigned long)dev); set_bit(FF_GAIN, dev->ffbit); diff --git a/include/linux/input.h b/include/linux/input.h index 0ccfc30..c2b1a7d 100644 --- a/include/linux/input.h +++ b/include/linux/input.h @@ -1377,6 +1377,10 @@ extern struct class input_class; * methods; erase() is optional. set_gain() and set_autocenter() need * only be implemented if driver sets up FF_GAIN and FF_AUTOCENTER * bits. + * + * Note that playback(), set_gain() and set_autocenter() are called with + * dev->event_lock spinlock held and interrupts off and thus may not + * sleep. */ struct ff_device { int (*upload)(struct input_dev *dev, struct ff_effect *effect,