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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7F198C19F2D for ; Tue, 9 Aug 2022 10:37:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 169D68E0001; Tue, 9 Aug 2022 06:37:13 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 119126B0072; Tue, 9 Aug 2022 06:37:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F21B28E0001; Tue, 9 Aug 2022 06:37:12 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id E039F6B0071 for ; Tue, 9 Aug 2022 06:37:12 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id B0897C0C04 for ; Tue, 9 Aug 2022 10:37:12 +0000 (UTC) X-FDA: 79779701904.21.D5BF05D Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by imf31.hostedemail.com (Postfix) with ESMTP id 209B420164 for ; Tue, 9 Aug 2022 10:37:11 +0000 (UTC) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 413FFB81065; Tue, 9 Aug 2022 10:37:10 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 52C1FC433C1; Tue, 9 Aug 2022 10:37:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1660041429; bh=Bl1E6LuUAfgRCpSmoKLdYUJd7WzSDU3hNUhyj3UsvtI=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=mRDe0cJQN6JyUqTYl77VhCTV4ubMevOyzRghXWnDksmxCyf59Kv0/94MzhAhe9EQc 6tc+AO1CJETCVkG857o3cBtMxKExXmVqIqv15vGh+NLNkBIcyiSD0iaaIGaPeYpHYW RCnau97Q9aIBHDfWAt/LXepml0rKK4yiinXIWWmAjLcjxvSdmKt+toc5oQGQoQO2lu WT1iiXcXz/3k9LHaNMPQOy9ZQCvf8PZoOj5RAihWfuUlS/vQATQGO1LyVbAlE+Ucw7 eTzhY9nan02cRV9uSy5U7yr8mrH+ctwrvwVpRm4/U0si0ppd3CPjIgivB2IFP2kC42 FZw21Oh24FicQ== Message-ID: <70832bc0-cc98-69fe-b8a1-c10b1847d415@kernel.org> Date: Tue, 9 Aug 2022 12:37:06 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.0.3 Subject: Re: [PATCH 5/6] slab: Allocate frozen pages Content-Language: en-US To: David Hildenbrand , Matthew Wilcox Cc: linux-mm@kvack.org References: <20220531150611.1303156-1-willy@infradead.org> <20220531150611.1303156-6-willy@infradead.org> <40d658da-6220-e05e-ba0b-d95c82f6bfb3@redhat.com> <88604b32-d13c-40e2-3ef6-9defcec805cb@redhat.com> From: "Vlastimil Babka (SUSE)" In-Reply-To: <88604b32-d13c-40e2-3ef6-9defcec805cb@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1660041432; h=from:from:sender: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:dkim-signature; bh=AFW4ldE6jOi6KWOC54HBYRF9WEZXQEWj/b9GZGHNvYM=; b=E3HSjy5P4GCQU4Mb8vok2gG8lcDH1cRgJOfM/6zHQGvrSe2NruEDMi7xOY4CuL7fFNex+y OFRmxZsAukdkhZed591MQv6WlwB2a2P0dJjgxbNFpp3F4j6eOIfT0whS7u7sDvR+SuB6HR sSreho1Y4c+LVSwz4yT9L83eNUxxfrE= ARC-Authentication-Results: i=1; imf31.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=mRDe0cJQ; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf31.hostedemail.com: domain of vbabka@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=vbabka@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1660041432; a=rsa-sha256; cv=none; b=XU1itD3cutHJVAcK6l6UL6BmkWQ3aPiymCkHtMq9Jd2sWordrWIVjnsrfLURwvPNba7toW 5s6xQH5Qp/pyVJxxnRMlF1QYc3fm62nPio/ByN0HTdOLrN3Jp1JndyX1gsKoHCI5wb+d0y 87gJLo+BM2by8A3WWhVBAeSHDQfGTxc= Authentication-Results: imf31.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=mRDe0cJQ; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf31.hostedemail.com: domain of vbabka@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=vbabka@kernel.org X-Rspamd-Server: rspam02 X-Stat-Signature: e4fdbre3d7y3cmjtrqjs6ckm76xk7et9 X-Rspamd-Queue-Id: 209B420164 X-Rspam-User: X-HE-Tag: 1660041431-308271 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 6/1/22 14:14, David Hildenbrand wrote: > On 31.05.22 19:33, Matthew Wilcox wrote: >> On Tue, May 31, 2022 at 07:15:14PM +0200, David Hildenbrand wrote: >>> On 31.05.22 17:06, Matthew Wilcox (Oracle) wrote: >>>> Since slab does not use the page refcount, it can allocate and >>>> free frozen pages, saving one atomic operation per free. >>> >>> I assume that implies that pages that are actually allocated *from* the >>> buddy now have a refcount == 0. >> >> Yes. >> >>> IIRC, page isolation code (e.g., !page_ref_count check in >>> has_unmovable_pages()) assumes that any page with a refcount of 0 is >>> essentially either already free (buddy) or is on its way of getting >>> freed (!buddy). >> >> That would be a bad assumption. We already freeze pages for things like >> splitting, migration, and replacement with a THP. If the usage is just >> an optimisation, then that's OK (and maybe the optimisation needs to be >> tweaked to check PageSlab), but if the code depends on that being true, >> it was already broken. > > Yes, it's an optimization to not go ahead and mess with the migratetype > of pageblocks (especially, setting it MIGRATE_ISOLATE to later reset it > to MIGRATE_MOVABLE) and so forth when it's obvious that there is > unmovable data. > > However, freezing the refcount -- as used in existing code -- is only a > temporary thing. And it's frequently used on movable pages. Agreed. > If migration froze the refcount, the page is most certainly movable. > THPs should be movable, so it doesn't matter if we froze the refcount or > not. Pages that we collapse into a THP should be mostly LRU pages and, > therefore, movable. > > So the frozen refcount doesn't result in additional "false negatives" in > e.g., has_unmovable_pages(), at least for these users. > > >> >> For this particular case, I think has_unmovable_pages() is only called >> for ZONE_MOVEABLE and Slab never allocates from ZONE_MOVEABLE, so it's >> not an issue? > > > has_unmovable_pages() is primarily useful for ZONE_NORMAL. For > ZONE_MOVABLE, we really only check if there are any boot time > allocations (PageReserved()). > > For example, alloc_contig_range() implicitly calls has_unmovable_pages() > when isolating pageblocks and ends up getting called on ZONE_NORMAL for > example for runtime allocation of gigantic pages or via virtio-mem > (nowadays even in pageblock granularity). > > > Long story short, we should document accordingly what it actually means > when we allocate without increasing the refcount, and that people should > be careful with that. Regarding has_unmovable_pages() and frozen > allocations, we might just be able to keep the existing detection > unmodified by special-casing PageSlab() pages and detecting them as > unmovable immediately. I wonder if it makes sense to encourage long-term freezing anyway, if it complicates other checks that could for now rely on the relatively simple rules. For slab it might save some atomics, but allocating/freeing a new slab page is already a very slow path, so that will be negligible. So I wouldn't mind if only up to 4/6 of this series was merged. OTOH once the API is available somebody will eventually use it for a long-term frozen allocation (but they achieve do that now too, so maybe not), so agree about documenting the implications.