From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9ECFB20F084 for ; Wed, 12 Feb 2025 14:10:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739369434; cv=none; b=NWDr3pCgZHOAHwHmK1WTfIbcChA/aioipPr/NHPs+dc7m5e8b+4Lrb4z2k+tJz++Lgw2MBs92EvwyPWiMZ4wgnhHQKW2ayoAXMuRJC/Jl53vzuj7eHjW0F/jYPTNjoqmfegL0OvLwLishmc315arlSI9F6RXGg7V3fHpehXAbS8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739369434; c=relaxed/simple; bh=P/vNrMUaz1jHHThzVkyhCQbgh6lpIub0ajM7DWOl6ao=; h=From:Message-ID:Date:MIME-Version:Subject:To:Cc:References: In-Reply-To:Content-Type; b=ptRCOPS4JIPJumlhld23LPogERWEMGz+sNmYkFCgrwfnrtdhruxxbSSrCTrt22v0JVWaD03A+/Nlx6qvZAGkBXMiVsbDQQadRMuUXC1PWUMNMxIQVYFJeDe/Eft+PwX2MeuVG9bFT+BarAkSVDcm12KIkXUv1qUZKqYcLUDympA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=ZC2Kg0h8; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="ZC2Kg0h8" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1739369431; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=f3xCL7oxgCvrGz6Ncj115FW+CHW40dQnECF6li9kzuU=; b=ZC2Kg0h80gYpDA5w84iwOPCE4GkHtBvRGJyeiAj5PYWv8FlZxtJ6yPR8Wa50JGxXFDvITF NyryzROT82nbc3jxxbFV02FFU4Xn5PnP5ugR17jv3ZI+e8agL+35yXc5JNPFIk/Kp8rOfn JhnXYeC6fKr2VDiD094jS39Ct6oCPuU= Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-53-FfMk5lgAN3mPfbKuoFBZTQ-1; Wed, 12 Feb 2025 09:10:30 -0500 X-MC-Unique: FfMk5lgAN3mPfbKuoFBZTQ-1 X-Mimecast-MFC-AGG-ID: FfMk5lgAN3mPfbKuoFBZTQ_1739369429 Received: by mail-qk1-f200.google.com with SMTP id af79cd13be357-7c07249127bso115527285a.2 for ; Wed, 12 Feb 2025 06:10:30 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1739369429; x=1739974229; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:user-agent:mime-version:date:message-id:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=f3xCL7oxgCvrGz6Ncj115FW+CHW40dQnECF6li9kzuU=; b=h8S6BXYn1tGD4Of+RbFkOsyAxWvPvI9PSCwi/Lt8/Eo9D4r7oAAktYC0xJg3riW7qZ i4JaFB3EG7KyIvNyLtcfyUUV8wF/VuIs2KH2Jjec+cECTewABcGUogpJo68dx8J7rXq0 R1vQWmALpEoGSvTHTthcOe2yg3RXR0HKilTGb7H1EAhnXaizLnd1yfoDM0zG22ZLYLi4 wtIOPGMSdjXas/6gquoCeAmQg+AXePWexAyIoT+enWE0oWhG3a/OhpupEt9CpMAhw8Lt N7re1tYfySKBX8POAPwfi/rpnyor1J2L0zx285kktqcmaLMBaG0B+pa6uinebzx6iexV xC4A== X-Forwarded-Encrypted: i=1; AJvYcCX8Fn/++s/MobAxpzFkXpBzfD985DFLOFimU/ivimMyFeUMJdA5s39ivn9k0djWGrmjtqfN7GPiZVZWcR8=@vger.kernel.org X-Gm-Message-State: AOJu0YxCXuZ1qcAvvRl+Z3SJ17sYtOnU5exWhDTobKUdkLfXbw9bTXIa e6Pyc6HrUFYXBCBSdHzuIc1cU0UrDzze2xwhr/u+JTMn/OFJlTNkgXnrGH48savQEsJEkW2m0Mj vvkFlMJLhu6Uuz+Q+7mrJ5pFsknNrSd/oHPOF1HR8SDdWk6UltpgdU2k9ek3nYw== X-Gm-Gg: ASbGncvj8YlE2xZA6zYff0v2sh68GZsjEDZkf0LFFog2gDsRXKutBD5nTTZPx0e+fe6 el78+7bQZFqyFTL/4k/JzSNeH0CHR7lTbBspBddEB9Y3rUN0IsuAfYtj0Odle9rnN+IjMv+r1GD INF54Q3iGlg5/Ekpcml6OWg+s4j0CjULa186oP0SIhAZG+yBb+xSggfUGsuZmTE0jbOxf0mPiDg qLIG2IFnaCYcgCGy9xFJ81p80Rey7l9bq5gPOugkRPXRpiTPLDT6IHDV5t060ApJXJf03DQtzQ+ UZrcLBAoRkTcmfzzFLX0toRipVRPr4WAiOLGXesC05r38TnW X-Received: by 2002:a05:620a:4399:b0:7c0:55ee:b391 with SMTP id af79cd13be357-7c06fce2a29mr767402585a.54.1739369429620; Wed, 12 Feb 2025 06:10:29 -0800 (PST) X-Google-Smtp-Source: AGHT+IFMJeLzoRLSrfSXokIx2dS2HcRzFwlNtLF/6j+doTDGOwIGoL21GGyKdh8Sd2fMP6RZDdLmQQ== X-Received: by 2002:a05:620a:4399:b0:7c0:55ee:b391 with SMTP id af79cd13be357-7c06fce2a29mr767398085a.54.1739369429265; Wed, 12 Feb 2025 06:10:29 -0800 (PST) Received: from ?IPV6:2601:188:c100:5710:627d:9ff:fe85:9ade? ([2601:188:c100:5710:627d:9ff:fe85:9ade]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7c05a703d71sm515644485a.46.2025.02.12.06.10.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 12 Feb 2025 06:10:28 -0800 (PST) From: Waiman Long X-Google-Original-From: Waiman Long Message-ID: <5f518ef0-2dbc-4d14-82ce-ad310a780598@redhat.com> Date: Wed, 12 Feb 2025 09:10:25 -0500 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3] locking/semaphore: Use wake_q to wake up processes outside lock critical section To: Boqun Feng , Waiman Long Cc: Peter Zijlstra , Ingo Molnar , Will Deacon , linux-kernel@vger.kernel.org References: <20250127013127.3913153-1-longman@redhat.com> <3e45144d-d147-4431-91be-63d0817fa2ce@redhat.com> Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 2/12/25 12:45 AM, Boqun Feng wrote: > On Tue, Feb 11, 2025 at 09:18:56PM -0500, Waiman Long wrote: >> On 1/26/25 8:31 PM, Waiman Long wrote: >>> A circular lock dependency splat has been seen involving down_trylock(). >>> >>> [ 4011.795602] ====================================================== >>> [ 4011.795603] WARNING: possible circular locking dependency detected >>> [ 4011.795607] 6.12.0-41.el10.s390x+debug >>> [ 4011.795612] ------------------------------------------------------ >>> [ 4011.795613] dd/32479 is trying to acquire lock: >>> [ 4011.795617] 0015a20accd0d4f8 ((console_sem).lock){-.-.}-{2:2}, at: down_trylock+0x26/0x90 >>> [ 4011.795636] >>> [ 4011.795636] but task is already holding lock: >>> [ 4011.795637] 000000017e461698 (&zone->lock){-.-.}-{2:2}, at: rmqueue_bulk+0xac/0x8f0 >>> >>> the existing dependency chain (in reverse order) is: >>> -> #4 (&zone->lock){-.-.}-{2:2}: >>> -> #3 (hrtimer_bases.lock){-.-.}-{2:2}: >>> -> #2 (&rq->__lock){-.-.}-{2:2}: >>> -> #1 (&p->pi_lock){-.-.}-{2:2}: >>> -> #0 ((console_sem).lock){-.-.}-{2:2}: >>> >>> The console_sem -> pi_lock dependency is due to calling try_to_wake_up() >>> while holding the console.sem raw_spinlock. This dependency can be broken >>> by using wake_q to do the wakeup instead of calling try_to_wake_up() >>> under the console_sem lock. This will also make the semaphore's >>> raw_spinlock become a terminal lock without taking any further locks >>> underneath it. >>> >>> The hrtimer_bases.lock is a raw_spinlock while zone->lock is a >>> spinlock. The hrtimer_bases.lock -> zone->lock dependency happens via >>> the debug_objects_fill_pool() helper function in the debugobjects code. >>> >>> [ 4011.795646] -> #4 (&zone->lock){-.-.}-{2:2}: >>> [ 4011.795650] __lock_acquire+0xe86/0x1cc0 >>> [ 4011.795655] lock_acquire.part.0+0x258/0x630 >>> [ 4011.795657] lock_acquire+0xb8/0xe0 >>> [ 4011.795659] _raw_spin_lock_irqsave+0xb4/0x120 >>> [ 4011.795663] rmqueue_bulk+0xac/0x8f0 >>> [ 4011.795665] __rmqueue_pcplist+0x580/0x830 >>> [ 4011.795667] rmqueue_pcplist+0xfc/0x470 >>> [ 4011.795669] rmqueue.isra.0+0xdec/0x11b0 >>> [ 4011.795671] get_page_from_freelist+0x2ee/0xeb0 >>> [ 4011.795673] __alloc_pages_noprof+0x2c2/0x520 >>> [ 4011.795676] alloc_pages_mpol_noprof+0x1fc/0x4d0 >>> [ 4011.795681] alloc_pages_noprof+0x8c/0xe0 >>> [ 4011.795684] allocate_slab+0x320/0x460 >>> [ 4011.795686] ___slab_alloc+0xa58/0x12b0 >>> [ 4011.795688] __slab_alloc.isra.0+0x42/0x60 >>> [ 4011.795690] kmem_cache_alloc_noprof+0x304/0x350 >>> [ 4011.795692] fill_pool+0xf6/0x450 >>> [ 4011.795697] debug_object_activate+0xfe/0x360 >>> [ 4011.795700] enqueue_hrtimer+0x34/0x190 >>> [ 4011.795703] __run_hrtimer+0x3c8/0x4c0 >>> [ 4011.795705] __hrtimer_run_queues+0x1b2/0x260 >>> [ 4011.795707] hrtimer_interrupt+0x316/0x760 >>> [ 4011.795709] do_IRQ+0x9a/0xe0 >>> [ 4011.795712] do_irq_async+0xf6/0x160 >>> >>> Normally raw_spinlock to spinlock dependency is not legit >>> and will be warned if PROVE_RAW_LOCK_NESTING is enabled, >>> but debug_objects_fill_pool() is an exception as it explicitly >>> allows this dependency for non-PREEMPT_RT kernel without causing >>> PROVE_RAW_LOCK_NESTING lockdep splat. As a result, this dependency is >>> legit and not a bug. >>> >>> Anyway, semaphore is the only locking primitive left that is still >>> using try_to_wake_up() to do wakeup inside critical section, all the >>> other locking primitives had been migrated to use wake_q to do wakeup >>> outside of the critical section. It is also possible that there are >>> other circular locking dependencies involving printk/console_sem or >>> other existing/new semaphores lurking somewhere which may show up in >>> the future. Let just do the migration now to wake_q to avoid headache >>> like this. >> I can also add the following as another instance where deadlock can happen. >> >> Reported-by:syzbot+ed801a886dfdbfe7136d@syzkaller.appspotmail.com >> > FWIW, I already queued in my lockdep-for-tip branch, will send it in a > PR to Peter in one or two weeks (in case he hasn't taken it before > then). > > BTW, do we need a "Fixes" tag for stable kernels? After some more thought, I realize that this patch doesn't really fix the circular lock dependency problem, it just remove console_sem.lock from it. The problem is that printk() can be called in any context. To really solve the problem, we will need some kind of deferred wakeup using workqueue, for instance. As printing to the console is inherently slow, adding some more latency to the wakeup process shouldn't really be a problem. This patch will be the first step, I will work on additional patches to complete this deferred wakeup functionality. So I don't need to add a Fixes tag for now. You can either take this patch out or just leave it there. Cheers, Longman