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=-2.5 required=3.0 tests=MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no 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 7191FCA9EAE for ; Wed, 23 Oct 2019 06:15:16 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 316A12084C for ; Wed, 23 Oct 2019 06:15:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 316A12084C 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 B0AF56B0003; Wed, 23 Oct 2019 02:15:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A95786B0006; Wed, 23 Oct 2019 02:15:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 95BF46B0007; Wed, 23 Oct 2019 02:15:15 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0208.hostedemail.com [216.40.44.208]) by kanga.kvack.org (Postfix) with ESMTP id 6922F6B0003 for ; Wed, 23 Oct 2019 02:15:15 -0400 (EDT) Received: from smtpin06.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with SMTP id EE31E5000 for ; Wed, 23 Oct 2019 06:15:14 +0000 (UTC) X-FDA: 76074036948.06.pump27_2dc1ae4017127 X-HE-Tag: pump27_2dc1ae4017127 X-Filterd-Recvd-Size: 4802 Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) by imf26.hostedemail.com (Postfix) with ESMTP for ; Wed, 23 Oct 2019 06:15:14 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 92ED5AEF3; Wed, 23 Oct 2019 06:15:12 +0000 (UTC) Date: Wed, 23 Oct 2019 08:15:11 +0200 From: Michal Hocko To: Andrew Morton , Vlastimil Babka , Mel Gorman Cc: Waiman Long , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Johannes Weiner , Roman Gushchin , Konstantin Khlebnikov , Jann Horn , Song Liu , Greg Kroah-Hartman , Rafael Aquini Subject: Re: [PATCH] mm/vmstat: Reduce zone lock hold time when reading /proc/pagetypeinfo Message-ID: <20191023061511.GA754@dhcp22.suse.cz> References: <20191022162156.17316-1-longman@redhat.com> <20191022145902.d9c4a719c0b32175e06e4eee@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191022145902.d9c4a719c0b32175e06e4eee@linux-foundation.org> User-Agent: Mutt/1.10.1 (2018-07-13) X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue 22-10-19 14:59:02, Andrew Morton wrote: > On Tue, 22 Oct 2019 12:21:56 -0400 Waiman Long wrote: [...] > > - for (mtype = 0; mtype < MIGRATE_TYPES; mtype++) { > > - seq_printf(m, "Node %4d, zone %8s, type %12s ", > > - pgdat->node_id, > > - zone->name, > > - migratetype_names[mtype]); > > - for (order = 0; order < MAX_ORDER; ++order) { > > + lockdep_assert_held(&zone->lock); > > + lockdep_assert_irqs_disabled(); > > + > > + /* > > + * MIGRATE_MOVABLE is usually the largest one in large memory > > + * systems. We skip iterating that list. Instead, we compute it by > > + * subtracting the total of the rests from free_area->nr_free. > > + */ > > + for (order = 0; order < MAX_ORDER; ++order) { > > + unsigned long nr_total = 0; > > + struct free_area *area = &(zone->free_area[order]); > > + > > + for (mtype = 0; mtype < MIGRATE_TYPES; mtype++) { > > unsigned long freecount = 0; > > - struct free_area *area; > > struct list_head *curr; > > > > - area = &(zone->free_area[order]); > > - > > + if (mtype == MIGRATE_MOVABLE) > > + continue; > > list_for_each(curr, &area->free_list[mtype]) > > freecount++; > > - seq_printf(m, "%6lu ", freecount); > > + nfree[order][mtype] = freecount; > > + nr_total += freecount; > > } > > + nfree[order][MIGRATE_MOVABLE] = area->nr_free - nr_total; > > + > > + /* > > + * If we have already iterated more than 64k of list > > + * entries, we might have hold the zone lock for too long. > > + * Temporarily release the lock and reschedule before > > + * continuing so that other lock waiters have a chance > > + * to run. > > + */ > > + if (nr_total > (1 << 16)) { > > + spin_unlock_irq(&zone->lock); > > + cond_resched(); > > + spin_lock_irq(&zone->lock); > > + } > > + } > > + > > + for (mtype = 0; mtype < MIGRATE_TYPES; mtype++) { > > + seq_printf(m, "Node %4d, zone %8s, type %12s ", > > + pgdat->node_id, > > + zone->name, > > + migratetype_names[mtype]); > > + for (order = 0; order < MAX_ORDER; ++order) > > + seq_printf(m, "%6lu ", nfree[order][mtype]); > > seq_putc(m, '\n'); > > This is not exactly a thing of beauty :( Presumably there might still > be situations where the irq-off times remain excessive. Yes. It is the list_for_each over the free_list that needs the lock and that is the actual problem here. This can be really large with a _lot_ of memory. And this is why I objected to the patch. Because it doesn't really address this problem. I would like to hear from Mel and Vlastimil how would they feel about making free_list fully migrate type aware (including nr_free). > Why are we actually holding zone->lock so much? Can we get away with > holding it across the list_for_each() loop and nothing else? If so, > this still isn't a bulletproof fix. Maybe just terminate the list > walk if freecount reaches 1024. Would anyone really care? > > Sigh. I wonder if anyone really uses this thing for anything > important. Can we just remove it all? Vlastimil would know much better but I have seen this being used for fragmentation related debugging. That should imply that 0400 should be sufficient and a quick and easily backportable fix for the most pressing immediate problem. -- Michal Hocko SUSE Labs