Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755470Ab2BFQKN (ORCPT ); Mon, 6 Feb 2012 11:10:13 -0500 Received: from merlin.infradead.org ([205.233.59.134]:37957 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755353Ab2BFQKM (ORCPT ); Mon, 6 Feb 2012 11:10:12 -0500 Message-ID: <4F2FFB21.9000202@kernel.dk> Date: Mon, 06 Feb 2012 17:09:05 +0100 From: Jens Axboe MIME-Version: 1.0 To: Vivek Goyal CC: Shaohua Li , lkml , Linus Torvalds , Tejun Heo , Knut Petersen , mroos@linux.ee Subject: Re: [patch]block: fix ioc locking warning References: <1328514611.21268.66.camel@sli10-conroe> <20120206151219.GC30752@redhat.com> In-Reply-To: <20120206151219.GC30752@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3315 Lines: 68 On 02/06/2012 04:12 PM, Vivek Goyal wrote: > On Mon, Feb 06, 2012 at 03:50:11PM +0800, Shaohua Li wrote: >> Meelis reported a warning: >> >> WARNING: at kernel/timer.c:1122 run_timer_softirq+0x199/0x1ec() >> Hardware name: 939Dual-SATA2 >> timer: cfq_idle_slice_timer+0x0/0xaa preempt leak: 00000102 -> 00000103 >> Modules linked in: sr_mod cdrom videodev media drm_kms_helper ohci_hcd ehci_hcd v4l2_compat_ioctl32 usbcore i2c_ali15x3 snd_seq drm snd_timer snd_seq >> Pid: 0, comm: swapper Not tainted 3.3.0-rc2-00110-gd125666 #176 >> Call Trace: >> [] warn_slowpath_common+0x7e/0x96 >> [] ? cfq_slice_expired+0x1d/0x1d >> [] warn_slowpath_fmt+0x41/0x43 >> [] ? cfq_idle_slice_timer+0xa1/0xaa >> [] ? cfq_slice_expired+0x1d/0x1d >> [] run_timer_softirq+0x199/0x1ec >> [] ? timekeeping_get_ns+0x12/0x31 >> [] ? apic_write+0x11/0x13 >> [] __do_softirq+0x74/0xfa >> [] call_softirq+0x1a/0x30 >> [] do_softirq+0x31/0x68 >> [] irq_exit+0x3d/0xa3 >> [] smp_apic_timer_interrupt+0x6b/0x77 >> [] apic_timer_interrupt+0x69/0x70 >> [] ? sched_clock_cpu+0x73/0x7d >> [] ? sched_clock_cpu+0x73/0x7d >> [] ? default_idle+0x1e/0x32 >> [] ? default_idle+0x18/0x32 >> [] cpu_idle+0x87/0xd1 >> [] rest_init+0x85/0x89 >> [] start_kernel+0x2eb/0x2f8 >> [] x86_64_start_reservations+0x7e/0x82 >> [] x86_64_start_kernel+0xf0/0xf7 >> >> this_q == locked_q is possible. There are two problems here: >> 1. In UP case, there is preemption counter issue as spin_trylock always >> successes. >> 2. In SMP case, the loop breaks too earlier. > > Thanks Shaohua. So is it the case where there are more than one cic's on > ioc->ioc_list and first cic's queue is not same as locked_queue. But some > other cic other than first has queue same as locked_queue. > > In that case current code will still defer freeing of ioc and cic to a > worker thread. So this patch will introduce one optimization to handle > those cases and avoid calling worker thread. > > Secondly it fixes the discrepancy of preemption count on UP machines, > where we have one extra preemption count after finish of function > put_io_context(). So for UP case spin_trylock() increases the preemption > count and always returns success. As this_q == locked_q we never try to do > unlock on this queue and hence never decrement the preemption count > hence resulting in preemption count warning. > > Changlog was not obivious atleast to me. I wished it was little more > descriptive. Anyway, patch is already committed.. We can always amend the changelog, so don't worry about it already being committed. If you want to add/change something, just send it in. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/