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 X-Spam-Level: X-Spam-Status: No, score=-5.5 required=3.0 tests=MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A0052C33CB1 for ; Thu, 16 Jan 2020 14:28:32 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 461262051A for ; Thu, 16 Jan 2020 14:28:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 461262051A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id AF5FB8E0071; Thu, 16 Jan 2020 09:28:31 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id AA6148E003F; Thu, 16 Jan 2020 09:28:31 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9954B8E0071; Thu, 16 Jan 2020 09:28:31 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0102.hostedemail.com [216.40.44.102]) by kanga.kvack.org (Postfix) with ESMTP id 828758E003F for ; Thu, 16 Jan 2020 09:28:31 -0500 (EST) Received: from smtpin12.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with SMTP id 2C94F181AC9CC for ; Thu, 16 Jan 2020 14:28:31 +0000 (UTC) X-FDA: 76383728022.12.cart87_84843b7427a57 X-HE-Tag: cart87_84843b7427a57 X-Filterd-Recvd-Size: 5825 Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by imf29.hostedemail.com (Postfix) with ESMTP for ; Thu, 16 Jan 2020 14:28:30 +0000 (UTC) Received: by mail-wr1-f67.google.com with SMTP id j42so19321980wrj.12 for ; Thu, 16 Jan 2020 06:28:30 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=OzG4Wm6WZrsrZXG7EsPNbNhDAoAsn+FlgzMhOZKK7Bw=; b=J6Fs+su57IN2TS8Ynold0zlvbPGrvkUyFvo01vOXBSv+TDF3JZKKXkn5gt8jvuw2HY S129s3BT/muInYKOSygpc9cu2IMmYehL8jwcrL6s1R6YPSxhRvC1wKMXhGNMyRtaqoBx 0NSqmwpDKq4ZD50YfJRrIPeUMMVAn8cXATQiWfi+J4fmBFqOEzoixRLRrnCupFw4MLmY 4fAuGaKXoTCkrYj2zwale47XiuaujDtg5KQVfZWyt6gt5Wm/p4x/LXYvJTTUvtHvMssY Qg1q8naHe1VI6ldj/BSzDNXpzuZoxQ2/l6Z5gKVOcsKj70nrJTVdZuytR+o0Nue210Jy Jhag== X-Gm-Message-State: APjAAAVhLGIrNudWAHzCH1GXNOybisbYo1c58K/+XZlUi+4pQtov/MBm 2ELZb1o0tcslbik2gxx6VeQ= X-Google-Smtp-Source: APXvYqz9ml7zEul5JgtZtk5xrlAZN1wdyd+IOkJKp591wmb5Cwc09LYTQXX20MEd3rOfb7suiWzxjQ== X-Received: by 2002:adf:f70b:: with SMTP id r11mr3846446wrp.388.1579184909342; Thu, 16 Jan 2020 06:28:29 -0800 (PST) Received: from localhost (prg-ext-pat.suse.com. [213.151.95.130]) by smtp.gmail.com with ESMTPSA id y17sm593663wma.36.2020.01.16.06.28.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 16 Jan 2020 06:28:28 -0800 (PST) Date: Thu, 16 Jan 2020 15:28:27 +0100 From: Michal Hocko To: Qian Cai Cc: akpm@linux-foundation.org, sergey.senozhatsky.work@gmail.com, pmladek@suse.com, rostedt@goodmis.org, peterz@infradead.org, david@redhat.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH -next v3] mm/hotplug: silence a lockdep splat with printk() Message-ID: <20200116142827.GU19428@dhcp22.suse.cz> References: <20200115172916.16277-1-cai@lca.pw> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200115172916.16277-1-cai@lca.pw> User-Agent: Mutt/1.12.2 (2019-09-21) X-Bogosity: Ham, tests=bogofilter, spamicity=0.000005, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed 15-01-20 12:29:16, Qian Cai wrote: > It is guaranteed to trigger a lockdep splat if calling printk() with > zone->lock held because there are many places (tty, console drivers, > debugobjects etc) would allocate some memory with another lock > held which is proved to be difficult to fix them all. I am still not happy with the above much. What would say about something like below instead? " It is not that hard to trigger lockdep splats by calling printk from under zone->lock. Most of them are false positives caused by lock chains introduced early in the boot process and they do not cause any real problems. There are some console drivers which do allocate from the printk context as well and those should be fixed. In any case false positives are not that trivial to workaround and it is far from optimal to lose lockdep functionality for something that is a non-issue. " > A common workaround until the onging effort to make all printk() as > deferred happens is to use printk_deferred() in those places similar to > the recent commit [1] merged into the random and -next trees, but memory > offline will call dump_page() which needs to be deferred after the lock. I would remove this paragraph altogether. The real problem is that we do not really want to make dump_page deferred. > So change has_unmovable_pages() so that it no longer calls dump_page() > itself - instead it returns a "struct page *" of the unmovable page back > to the caller so that in the case of a has_unmovable_pages() failure, > the caller can call dump_page() after releasing zone->lock. Again this begs for some more explanation why this is ok. Something like the following: " Even though has_unmovable_pages doesn't hold any reference to the returned page this should be reasonably safe for the purpose of reporting the page (dump_page) because it cannot be hotremoved. The state of the page might change but that is the case even with the existing code as zone->lock only plays role for free pages. " > Also, make > dump_page() is able to report a CMA page as well, so the reason string > from has_unmovable_pages() can be removed. > > While at it, remove a similar but unnecessary debug-only printk() as > well. A few sample lockdep splats can be founnd here [2]. > > [1] https://lore.kernel.org/lkml/1573679785-21068-1-git-send-email-cai@lca.pw/ > [2] https://lore.kernel.org/lkml/7CD27FC6-CFFF-4519-A57D-85179E9815FE@lca.pw/ > > Signed-off-by: Qian Cai With improved changelog and after addressing one more thing below, feel free to add Acked-by: Michal Hocko Thanks for working on this. I have to say I have disliked the initial version because they were really hacky. This one can be reasoned about at least. has_unmovable_pages returning an unmovable page actually makes some conceptual sense to me. [...] > @@ -8302,12 +8304,10 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int migratetype, > */ > goto unmovable; > } > - return false; > + return NULL; > unmovable: > WARN_ON_ONCE(zone_idx(zone) == ZONE_MOVABLE); You want to move this WARN_ON as well. Not only because it is using printk as well but also because we do not really want to trigger the warning from userspace via is_mem_section_removable. Maybe worth a patch on its own? -- Michal Hocko SUSE Labs