From patchwork Wed Apr 18 08:33:56 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anton Vorontsov X-Patchwork-Id: 7911 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 3584223E42 for ; Wed, 18 Apr 2012 08:35:12 +0000 (UTC) Received: from mail-iy0-f180.google.com (mail-iy0-f180.google.com [209.85.210.180]) by fiordland.canonical.com (Postfix) with ESMTP id C6069A18356 for ; Wed, 18 Apr 2012 08:35:11 +0000 (UTC) Received: by iage36 with SMTP id e36so13880205iag.11 for ; Wed, 18 Apr 2012 01:35:11 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-forwarded-to:x-forwarded-for:delivered-to:received-spf:date:from :to:cc:subject:message-id:references:mime-version:content-type :content-disposition:in-reply-to:user-agent:x-gm-message-state; bh=hxGlJ/qtN6J2jQYVjbfqXTi+7k0MjMibdsGL3hheQ8w=; b=NDIEj0+6Zy6p57Y4BuPxI6t+pCLb7+S/xVny3QNF2+eyz8oLS6afkw0fR2tpTYR2R4 MWX0JnWM6wHPG05nVQBLx+a+X1s6JDXjwYC/CrXGK7YUeQh6coptiwgLOtNWU8cYmqNm fxEedFa7Kfq3S3n6l7BDGq79rUVpXD65re3lWqAY22GvyL1fdf/AzKSg4ytTi2hKCrRY Lk6zZ2Aya0mO0Gb63i6cCFb/PDn5BF6ANbq90J3HY4JJpEhVVfczSCQa41HeyXZn3ZYE Gb0+roMz7j5qwBQgW4ql+IS+3gvP1vlhgXS02nSOBftVlUXtF5yDSAg0ktcEwGH/qjVf ZO5A== Received: by 10.50.193.199 with SMTP id hq7mr910346igc.49.1334738111259; Wed, 18 Apr 2012 01:35:11 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.231.137.198 with SMTP id x6csp202406ibt; Wed, 18 Apr 2012 01:35:10 -0700 (PDT) Received: by 10.182.40.7 with SMTP id t7mr1641030obk.55.1334738110712; Wed, 18 Apr 2012 01:35:10 -0700 (PDT) Received: from mail-ob0-f178.google.com (mail-ob0-f178.google.com [209.85.214.178]) by mx.google.com with ESMTPS id r9si15173686oee.6.2012.04.18.01.35.10 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 18 Apr 2012 01:35:10 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.214.178 is neither permitted nor denied by best guess record for domain of anton.vorontsov@linaro.org) client-ip=209.85.214.178; Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.214.178 is neither permitted nor denied by best guess record for domain of anton.vorontsov@linaro.org) smtp.mail=anton.vorontsov@linaro.org Received: by obbwc18 with SMTP id wc18so1143796obb.37 for ; Wed, 18 Apr 2012 01:35:10 -0700 (PDT) Received: by 10.182.12.6 with SMTP id u6mr1861392obb.12.1334738110476; Wed, 18 Apr 2012 01:35:10 -0700 (PDT) Received: from localhost ([69.199.155.45]) by mx.google.com with ESMTPS id s8sm21137589oec.1.2012.04.18.01.35.08 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 18 Apr 2012 01:35:09 -0700 (PDT) Date: Wed, 18 Apr 2012 01:33:56 -0700 From: Anton Vorontsov To: Pekka Enberg Cc: Leonid Moiseichuk , John Stultz , linux-mm@kvack.org, linux-kernel@vger.kernel.org, linaro-kernel@lists.linaro.org, patches@linaro.org Subject: [PATCH 1/2] vmevent: Should not grab mutex in the atomic context Message-ID: <20120418083356.GA31556@lizard> References: <20120418083208.GA24904@lizard> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20120418083208.GA24904@lizard> User-Agent: Mutt/1.5.21 (2010-09-15) X-Gm-Message-State: ALoCoQnvLgNH6VebB6pSF2lyNyzH9XfpbuISH5Qaz6K6xCJZmjqyenYZ7g2pxW+cPFlkeB8efPSu vmevent grabs a mutex in the atomic context, and so this pops up: BUG: sleeping function called from invalid context at kernel/mutex.c:271 in_atomic(): 1, irqs_disabled(): 0, pid: 0, name: swapper/0 1 lock held by swapper/0/0: #0: (&watch->timer){+.-...}, at: [] call_timer_fn+0x0/0xf0 Pid: 0, comm: swapper/0 Not tainted 3.2.0+ #6 Call Trace: [] __might_sleep+0x12a/0x1e0 [] ? vmevent_match+0xe0/0xe0 [] mutex_lock_nested+0x3c/0x340 [] ? lock_acquire+0xa3/0xc0 [] ? internal_add_timer+0x110/0x110 [] ? vmevent_match+0xe0/0xe0 [] vmevent_timer_fn+0x91/0xf0 [] ? vmevent_match+0xe0/0xe0 [] call_timer_fn+0x75/0xf0 [] ? internal_add_timer+0x110/0x110 [] ? trace_hardirqs_on_caller+0x7d/0x120 [] run_timer_softirq+0x10f/0x1e0 [] ? vmevent_match+0xe0/0xe0 [] __do_softirq+0xb0/0x160 [] ? tick_program_event+0x1f/0x30 [] call_softirq+0x1c/0x26 [] do_softirq+0x85/0xc0 This patch fixes the issue by removing the mutex and making the logic lock-free. Signed-off-by: Anton Vorontsov --- mm/vmevent.c | 52 ++++++++++++++++++++++++++-------------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/mm/vmevent.c b/mm/vmevent.c index 1847b56..9ed6aca 100644 --- a/mm/vmevent.c +++ b/mm/vmevent.c @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -22,8 +23,7 @@ struct vmevent_watch_event { struct vmevent_watch { struct vmevent_config config; - struct mutex mutex; - bool pending; + atomic_t pending; /* * Attributes that are exported as part of delivered VM events. @@ -99,24 +99,36 @@ static bool vmevent_match(struct vmevent_watch *watch) return false; } +/* + * This function is called from the timer context, which has the same + * guaranties as an interrupt handler: it can have only one execution + * thread (unlike bare softirq handler), so we don't need to worry + * about racing w/ ourselves. + * + * We also don't need to worry about several instances of timers + * accessing the same vmevent_watch, as we allocate vmevent_watch + * together w/ the timer instance in vmevent_fd(), so there is always + * one timer per vmevent_watch. + * + * All the above makes it possible to implement the lock-free logic, + * using just the atomic watch->pending variable. + */ static void vmevent_sample(struct vmevent_watch *watch) { int i; + if (atomic_read(&watch->pending)) + return; if (!vmevent_match(watch)) return; - mutex_lock(&watch->mutex); - - watch->pending = true; - for (i = 0; i < watch->nr_attrs; i++) { struct vmevent_attr *attr = &watch->sample_attrs[i]; attr->value = vmevent_sample_attr(watch, attr); } - mutex_unlock(&watch->mutex); + atomic_set(&watch->pending, 1); } static void vmevent_timer_fn(unsigned long data) @@ -125,7 +137,7 @@ static void vmevent_timer_fn(unsigned long data) vmevent_sample(watch); - if (watch->pending) + if (atomic_read(&watch->pending)) wake_up(&watch->waitq); mod_timer(&watch->timer, jiffies + nsecs_to_jiffies64(watch->config.sample_period_ns)); @@ -148,13 +160,9 @@ static unsigned int vmevent_poll(struct file *file, poll_table *wait) poll_wait(file, &watch->waitq, wait); - mutex_lock(&watch->mutex); - - if (watch->pending) + if (atomic_read(&watch->pending)) events |= POLLIN; - mutex_unlock(&watch->mutex); - return events; } @@ -171,15 +179,13 @@ static ssize_t vmevent_read(struct file *file, char __user *buf, size_t count, l if (count < size) return -EINVAL; - mutex_lock(&watch->mutex); - - if (!watch->pending) - goto out_unlock; + if (!atomic_read(&watch->pending)) + goto out; event = kmalloc(size, GFP_KERNEL); if (!event) { ret = -ENOMEM; - goto out_unlock; + goto out; } for (i = 0; i < watch->nr_attrs; i++) { @@ -195,14 +201,10 @@ static ssize_t vmevent_read(struct file *file, char __user *buf, size_t count, l ret = count; - watch->pending = false; - + atomic_set(&watch->pending, 0); out_free: kfree(event); - -out_unlock: - mutex_unlock(&watch->mutex); - +out: return ret; } @@ -231,8 +233,6 @@ static struct vmevent_watch *vmevent_watch_alloc(void) if (!watch) return NULL; - mutex_init(&watch->mutex); - init_waitqueue_head(&watch->waitq); return watch;