From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A7A5BC433F5 for ; Fri, 10 Dec 2021 19:05:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242045AbhLJTJZ convert rfc822-to-8bit (ORCPT ); Fri, 10 Dec 2021 14:09:25 -0500 Received: from mail-oi1-f178.google.com ([209.85.167.178]:44617 "EHLO mail-oi1-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230463AbhLJTJU (ORCPT ); Fri, 10 Dec 2021 14:09:20 -0500 Received: by mail-oi1-f178.google.com with SMTP id be32so14445880oib.11; Fri, 10 Dec 2021 11:05:44 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=c+Gg3IcBrVO+p3DERDiz2wIK2rZh3JBEUqlmxz2Wt9w=; b=KF02GmbhAy9o852cKAkWGRsQiFbgHC75Qjm16bI7ZSmAfyJfROZofbcH5jI8CJ/ve+ eHhgitsnexLUIDGw5//Gk3z+Ok9evNj4B9kvpHUljhuupzNnGZFu5aFXSsX81N0DYWnQ IaBXbFwEerwNO3snRUpagshSzYMbeSIiztCbLnK54Zt/2TVaC3wOPO8BVEK8I1B434i9 jEc6iUXt9xaEyYM6x5//wTvcOkDy/tLV52Sdv83wiI5s/7cNfq7VeKJ7PH5p5hyHH/1h /CbxkevGDaw6vPTu14r/Zn3poSjqgobQdPA5+G75/glByywxNMFAgRWya2D4H+Hw9B5o Jl1w== X-Gm-Message-State: AOAM5333zryiN0f+sbmHmz1C8Wb+kAiv8RDMW2PlfbHCFvYf5HhFNokx 4ByWa50hvEfayrUN+aS2O+/98uHRohqSF0ta+/8= X-Google-Smtp-Source: ABdhPJyFnxvuCdo/SAuZsTrbeFY0ttrM6VIAZMHiyNGyFieUXOlcjM/63kmCFhhj1nwi5THbS12OxkRKZ5ROskKDe+g= X-Received: by 2002:aca:eb0b:: with SMTP id j11mr14151206oih.51.1639163143930; Fri, 10 Dec 2021 11:05:43 -0800 (PST) MIME-Version: 1.0 References: <20211126180101.27818-1-digetx@gmail.com> <20211126180101.27818-4-digetx@gmail.com> In-Reply-To: From: "Rafael J. Wysocki" Date: Fri, 10 Dec 2021 20:05:32 +0100 Message-ID: Subject: Re: [PATCH v4 03/25] notifier: Add atomic/blocking_notifier_has_unique_priority() To: Dmitry Osipenko Cc: "Rafael J. Wysocki" , Thierry Reding , Jonathan Hunter , Russell King , Catalin Marinas , Will Deacon , Guo Ren , Geert Uytterhoeven , Greg Ungerer , Joshua Thompson , Thomas Bogendoerfer , Sebastian Reichel , Linus Walleij , Philipp Zabel , Greentime Hu , Vincent Chen , "James E.J. Bottomley" , Helge Deller , Michael Ellerman , Benjamin Herrenschmidt , Paul Mackerras , Paul Walmsley , Palmer Dabbelt , Albert Ou , Yoshinori Sato , Rich Felker , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , "the arch/x86 maintainers" , "H. Peter Anvin" , Boris Ostrovsky , Juergen Gross , Stefano Stabellini , Len Brown , Santosh Shilimkar , Krzysztof Kozlowski , Liam Girdwood , Mark Brown , Pavel Machek , Lee Jones , Andrew Morton , Guenter Roeck , Daniel Lezcano , Andy Shevchenko , Ulf Hansson , alankao@andestech.com, "K . C . Kuen-Chern Lin" , Linux ARM , Linux Kernel Mailing List , linux-csky@vger.kernel.org, linux-ia64@vger.kernel.org, linux-m68k@lists.linux-m68k.org, linux-mips@vger.kernel.org, linux-parisc@vger.kernel.org, linuxppc-dev , linux-riscv@lists.infradead.org, Linux-sh list , xen-devel@lists.xenproject.org, ACPI Devel Maling List , Linux PM , linux-tegra Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Dec 10, 2021 at 7:52 PM Dmitry Osipenko wrote: > > 10.12.2021 21:19, Rafael J. Wysocki пишет: > ... > >> +bool atomic_notifier_has_unique_priority(struct atomic_notifier_head *nh, > >> + struct notifier_block *n) > >> +{ > >> + unsigned long flags; > >> + bool ret; > >> + > >> + spin_lock_irqsave(&nh->lock, flags); > >> + ret = notifier_has_unique_priority(&nh->head, n); > >> + spin_unlock_irqrestore(&nh->lock, flags); > > > > This only works if the caller can prevent new entries from being added > > to the list at this point or if the caller knows that they cannot be > > added for some reason, but the kerneldoc doesn't mention this > > limitation. > > I'll update the comment. > > .. > >> +bool blocking_notifier_has_unique_priority(struct blocking_notifier_head *nh, > >> + struct notifier_block *n) > >> +{ > >> + bool ret; > >> + > >> + /* > >> + * This code gets used during boot-up, when task switching is > >> + * not yet working and interrupts must remain disabled. At such > >> + * times we must not call down_read(). > >> + */ > >> + if (system_state != SYSTEM_BOOTING) > > > > No, please don't do this, it makes the whole thing error-prone. > > What should I do then? First of all, do you know of any users who may want to call this during early initialization? If so, then why may they want to do that? Depending on the above, I would consider adding a special mechanism for them. > >> + down_read(&nh->rwsem); > >> + > >> + ret = notifier_has_unique_priority(&nh->head, n); > >> + > >> + if (system_state != SYSTEM_BOOTING) > >> + up_read(&nh->rwsem); > > > > And still what if a new entry with a non-unique priority is added to > > the chain at this point? > > If entry with a non-unique priority is added after the check, then > obviously it won't be detected. Why isn't this a problem? > I don't understand the question. These > down/up_read() are the locks that prevent the race, if that's the question. Not really, they only prevent the race from occurring while notifier_has_unique_priority() is running. If anyone depends on this check for correctness, they need to lock the rwsem, do the check, do the thing depending on the check while holding the rwsem and then release the rwsem. Otherwise it is racy.