From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ie0-f173.google.com (mail-ie0-f173.google.com [209.85.223.173]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority" (verified OK)) by huchra.bufferbloat.net (Postfix) with ESMTPS id A391A21F182 for ; Mon, 14 Jan 2013 08:32:27 -0800 (PST) Received: by mail-ie0-f173.google.com with SMTP id e13so5232890iej.18 for ; Mon, 14 Jan 2013 08:32:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=E8MmWXm8tR6YxOl2E/UFWpruXcBNWbHJNjvC1vsBrLM=; b=GrCy03tvJzq8AfxHE4OG7ZNJ2GqAXZBX3vd6NbcFVM8FbndFaL0KhsHuQ8Y3omL4O/ ju5WVLwnez/FN7HwdBYSM4F6xGoBFy7hvQHKsENo6Lq8cXYCxOYHa8xLohx0pCvwMzCd S6huJMYHwT7V8uAavGWdCJy8kzzuai7ti3Z3xcbrE8/IQ4JjsYqNcoaqNEX7U8n+eCVB mhSxMre+OWL/Y2KyP+q30QKfW2gN3ubnd+BbqkDfm3uiJcb8ECHDu0foierrKXLWpZ6a gX/t2p4sAj6EHDHYQnEa1rS8t3bbCPN90CFHdk4qvv1E1diJjMb9YDOFYSijGaKHxcEH +tRA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:x-gm-message-state; bh=E8MmWXm8tR6YxOl2E/UFWpruXcBNWbHJNjvC1vsBrLM=; b=LzaDMRwhSGaSix2E7Jqj8McEICntE+L/go7GbYlcV90EcIYOPQG81xa8fd+UpIINt8 RwPcTUhyQZTIwCXmZLrxuGr1Z6mMqoCKml5osRctZQAhG/yKUYJIqXSr36WweN/LFdIv Za4XmmRqk378DlP5H+TS9Yomu09sH7Uf8UVylvhNjYCRLTQQ7O/jh20YvL6JdBLAGnIF bakVtLo1KjbNsKDPVs6X2F/pToKVWqKP5gKh8vJzbmFThG5Q8RBoA7WI9Veh5FUTwNUH PMQO+KaPEwFiKt/UgBpzwz32qJYYjpIK5ixztLqZNl1KB3moPtWP5Yjs1AZ3Kh8EegFm CbHg== MIME-Version: 1.0 Received: by 10.50.222.132 with SMTP id qm4mr2429544igc.99.1358181147037; Mon, 14 Jan 2013 08:32:27 -0800 (PST) Received: by 10.50.161.227 with HTTP; Mon, 14 Jan 2013 08:32:26 -0800 (PST) In-Reply-To: References: <50F32981.9080404@openwrt.org> Date: Mon, 14 Jan 2013 08:32:26 -0800 Message-ID: From: Eric Dumazet To: Jerry Chu Content-Type: multipart/alternative; boundary=14dae934093fcf4def04d34230c2 X-Gm-Message-State: ALoCoQkdnR19CvS+oPtUNXNRVcMLH5wTOOcf1j5a8MjheqL5KSUnegALXMuZxpm9vsjGN821e0CXZ/z0E+W7GLzdnIZgk8yZ90oD0As7MzmLUJOOJITzd09n9ZpP6ur5yiaYuGHs08aDJ7HG57ir//qQ8Dwxnha6OuvdWUrzE+M7rIcRfMPshqxfH+ORubizY81z8y4qDJOfnGoOeG52nPcrEkHczLHHWA== Cc: cerowrt-devel , Yuchung Cheng Subject: Re: [Cerowrt-devel] TFO crashes cerowrt 3.7.1-1 X-BeenThere: cerowrt-devel@lists.bufferbloat.net X-Mailman-Version: 2.1.13 Precedence: list List-Id: Development issues regarding the cerowrt test router project List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Jan 2013 16:32:27 -0000 --14dae934093fcf4def04d34230c2 Content-Type: text/plain; charset=ISO-8859-1 Some paths want to check a spinlock is held, others want to check if its not held, it depends on the context. So returning 1 on UP would break a bunch of code as well. On Mon, Jan 14, 2013 at 12:18 AM, Jerry Chu wrote: > > > On Sun, Jan 13, 2013 at 7:05 PM, Eric Dumazet wrote: > >> Oh well yes, this doesnt quite work on !SMP. >> > > Strange - how would one assert a spin lock is held, and obviously only for > SMP? (I almost think arch_spin_is_locked(lock) should be ((void)(lock), > 1) for UP for the purpose of assertion...) > > Also it looks like there are bunch of other places spin_is_locked() > assertion is made in the source tree. (Perhaps they are only configured for > MP?) > > Thanks, > > Jerry > > >> And this kind of bug is frequent.... >> >> See following example : >> >> commit b9980cdcf2524c5fe15d8cbae9c97b3ed6385563 >> Author: Hugh Dickins >> Date: Wed Feb 8 17:13:40 2012 -0800 >> >> mm: fix UP THP spin_is_locked BUGs >> >> Fix CONFIG_TRANSPARENT_HUGEPAGE=y CONFIG_SMP=n CONFIG_DEBUG_VM=y >> CONFIG_DEBUG_SPINLOCK=n kernel: spin_is_locked() is then always false, >> and so triggers some BUGs in Transparent HugePage codepaths. >> >> asm-generic/bug.h mentions this problem, and provides a >> WARN_ON_SMP(x); >> but being too lazy to add VM_BUG_ON_SMP, BUG_ON_SMP, WARN_ON_SMP_ONCE, >> VM_WARN_ON_SMP_ONCE, just test NR_CPUS != 1 in the existing >> VM_BUG_ONs. >> >> Signed-off-by: Hugh Dickins >> Cc: Andrea Arcangeli >> Cc: >> Signed-off-by: Andrew Morton >> Signed-off-by: Linus Torvalds >> >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c >> index b3ffc21..91d3efb 100644 >> --- a/mm/huge_memory.c >> +++ b/mm/huge_memory.c >> @@ -2083,7 +2083,7 @@ static void collect_mm_slot(struct mm_slot *mm_slot) >> { >> struct mm_struct *mm = mm_slot->mm; >> >> - VM_BUG_ON(!spin_is_locked(&khugepaged_mm_lock)); >> + VM_BUG_ON(NR_CPUS != 1 && !spin_is_locked(&khugepaged_mm_lock)); >> >> >> >> >> On Sun, Jan 13, 2013 at 1:39 PM, Felix Fietkau wrote: >> >>> On 2013-01-13 7:03 PM, Eric Dumazet wrote: >>> > I suspect a bug in the spin_is_locked() implementation on your arch, as >>> > he socket lock should be held at this point. >>> I don't think this is an arch implementation bug, this probably happens >>> on all !SMP systems. See this bit from include/linux/spinlock_up.h: >>> >>> #define arch_spin_is_locked(lock) ((void)(lock), 0) >>> >>> - Felix >>> >>> >> > --14dae934093fcf4def04d34230c2 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable

Some paths want to check a spinlock is held, others w= ant to check if its not held, it depends on the context.

So returning 1 on UP would bre= ak a bunch of code as well.


On Mon, Jan 14, 2013 at 12:18 AM, Jerry Chu <hkchu@go= ogle.com> wrote:
=

On Sun, Jan 13, 2013 a= t 7:05 PM, Eric Dumazet <edumazet@google.com> wrote:
Oh well yes,= this doesnt quite work on !SMP.

Strange - how would one assert= a spin lock is held, and obviously only for SMP? (I almost think=A0arch_spin_is_locked(lo= ck) should be ((void)(lock), 1) for UP for the purpose of assertion...)

Also it looks like there are bunch of other places spin= _is_locked() assertion is made in the source tree. (Perhaps they are only c= onfigured for MP?)

Thanks,

Jerry


And this kind of bug is frequent....

See following example :

commit= b9980cdcf2524c5fe15d8cbae9c97b3ed6385563
Author: Hugh Dickins <hughd@google.com>
Date: =A0 Wed Feb 8 17:13:40 20= 12 -0800

=A0 =A0 mm: fix UP THP spin_is_locked BUGs
=A0 =A0=A0
<= div>=A0 =A0 Fix CONFIG_TRANSPARENT_HUGEPAGE=3Dy CONFIG_SMP=3Dn CONFIG_DEBUG= _VM=3Dy
=A0 =A0 CONFIG_DEBUG_SPINLOCK=3Dn kernel: spin_is_locked() is then always f= alse,
=A0 =A0 and so triggers some BUGs in Transparent HugePage c= odepaths.
=A0 =A0=A0
=A0 =A0 asm-generic/bug.h mentions this problem, and provides a WARN_ON_SMP= (x);
=A0 =A0 but being too lazy to add VM_BUG_ON_SMP, BUG_ON_SMP,= WARN_ON_SMP_ONCE,
=A0 =A0 VM_WARN_ON_SMP_ONCE, just test NR_CPUS= !=3D 1 in the existing VM_BUG_ONs.
=A0 =A0=A0
=A0 =A0 Signed-off-by: Hugh Dickins <hughd@google.com>
=A0 =A0 Cc: Andrea Arcangeli <aarcange@redhat.com>
=A0 =A0 Signed-off-by: Andre= w Morton <akpm@linux-foundation.org>
=A0 =A0 Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>= ;

diff --git a/mm/huge_memory.c b/mm/huge_memory.c=
index b3ffc21..91d3efb 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2083,7 +2083,7 @@ static void colle= ct_mm_slot(struct mm_slot *mm_slot)
=A0{
=A0 =A0 =A0 =A0 struct mm_struct *mm =3D mm_slot->mm= ;
=A0
- =A0 =A0 =A0 VM_BUG_ON(!spin_is_locked(&khug= epaged_mm_lock));
+ =A0 =A0 =A0 VM_BUG_ON(NR_CPUS !=3D 1 && !spin_is_locked(&= ;khugepaged_mm_lock));




On Sun, Jan 13, 2013 at 1:39 PM, Felix Fietkau <nbd@openwrt.org> wrote:
On 2013-01-13 7:03 PM, Eric Dumazet wrote:
> I suspect a bug in the spin_is_locked() implementation on your arch, a= s
> he socket lock should be held at this point.
I don't think this is an arch implementation bug, this probably h= appens
on all !SMP systems. See this bit from include/linux/spinlock_up.h:

#define arch_spin_is_locked(lock) =A0 ((void)(lock), 0)

- Felix




--14dae934093fcf4def04d34230c2--