Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752369AbaLNAdr (ORCPT ); Sat, 13 Dec 2014 19:33:47 -0500 Received: from zeniv.linux.org.uk ([195.92.253.2]:58613 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750755AbaLNAdq (ORCPT ); Sat, 13 Dec 2014 19:33:46 -0500 Date: Sun, 14 Dec 2014 00:33:32 +0000 From: Al Viro To: Linus Torvalds Cc: Dave Jones , Chris Mason , Mike Galbraith , Ingo Molnar , Peter Zijlstra , =?iso-8859-1?Q?D=E2niel?= Fraga , Sasha Levin , "Paul E. McKenney" , Linux Kernel Mailing List , Thomas Gleixner Subject: Re: frequent lockups in 3.18rc4 Message-ID: <20141214003332.GQ22149@ZenIV.linux.org.uk> References: <20141213165915.GA12756@redhat.com> <20141213223616.GA22559@redhat.com> <20141213233508.GN22149@ZenIV.linux.org.uk> <20141213234730.GP22149@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Dec 13, 2014 at 04:14:58PM -0800, Linus Torvalds wrote: > On Sat, Dec 13, 2014 at 3:47 PM, Al Viro wrote: > > > > static inline void mnt_dec_writers(struct mount *mnt) > > { > > #ifdef CONFIG_SMP > > this_cpu_dec(mnt->mnt_pcp->mnt_writers); > > #else > > mnt->mnt_writers--; > > #endif > > } > > It's load/modify/store, without any kind of atomicity; get preempted in the > > middle of that sequence by another caller of mnt_dec_writers() and obvious bad > > things will happen... > > Ugh, yes ok, the UP case needs it for the actual counter itself. Ugh. > What an ugly mess. I'd rather have the preemption disable where it is > actually *needed*, in that function itself for the UP case (or just > make it "atomic_t", which would likely be better still. So does SMP - this_cpu_dec() relies on preemption being disabled. On x86 we might get away with that, what with having it compiled into decl %gs:const, but on generic it turns into *raw_cpu_ptr(&pcp) -= 1; and compiler has every right to turn it into p = raw_cpu_ptr(&pcp); (*p)--; again, with no locking. Lose the timeslice in the middle of that and you are risking to get a different CPU when you are scheduled again, with another process doing this_cpu_dec() on your old CPU. Have fun - two non-atomic decrements of the same variable by different CPUs in parallel... We really need preemtion disabled there, UP or no UP. -- 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/