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 75311C021B2 for ; Thu, 20 Feb 2025 21:12:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 094662802F7; Thu, 20 Feb 2025 16:12:45 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 01CFD2802A7; Thu, 20 Feb 2025 16:12:44 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DD8992802F7; Thu, 20 Feb 2025 16:12:44 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id B73202802A7 for ; Thu, 20 Feb 2025 16:12:44 -0500 (EST) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 40798B92A0 for ; Thu, 20 Feb 2025 21:12:44 +0000 (UTC) X-FDA: 83141572248.09.A39A1D0 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf09.hostedemail.com (Postfix) with ESMTP id DC38B14001B for ; Thu, 20 Feb 2025 21:12:41 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=BPVheoO5; spf=pass (imf09.hostedemail.com: domain of luizcap@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=luizcap@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1740085962; 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=MGcNCNTpXqgcRd/FOw7KJikXMi7I5/lkRCqCXZ2bgxA=; b=25U7vsuan/+1L/2C5/OGBpS/DG/++c2TMAY/V797Cz+uSmY5+e8iZdG4qWxc0V78bsrSa/ 1hbtEulKK4wL/OD5mzNujEZAW7MImbdj7JwSOfHpnyDhyP8YDz5CAqvhXTLUTkHnpuiKcs tcCi+nUYpWeB6c1Uqi5JGMUrNcLj44w= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1740085962; a=rsa-sha256; cv=none; b=wupJ/g+NKaiH5WpaHpwvC+cH2fGe+v49d2BRW1ZmAFQ2agjaKz8qVFHTVc8Fc6LKphSmrV cuF5d4NW54bKVCch0iQps5oLRo4cen43BdVMjspfok0v1sf27P0UqRAikezuo+WsKSi+04 pyTjbCHdrLvH1sqcpDrkGJTstoYGfMk= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=BPVheoO5; spf=pass (imf09.hostedemail.com: domain of luizcap@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=luizcap@redhat.com; dmarc=pass (policy=none) header.from=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1740085961; 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=MGcNCNTpXqgcRd/FOw7KJikXMi7I5/lkRCqCXZ2bgxA=; b=BPVheoO5pQxmIlz3gA347b+kzgE7qnQHXPUy3PwP55bodCgJdB3WXoUFE6XlwJjN9BNjpP Buv1h8pqyO6944I+KuZ9OPao/17zmn0dGQwBctEV75OaDBiDhavXuK+YJOkLBvwg6/NeTF zl5auk6rmZuu5cROMecLWLwBYX62iEY= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-184-s4CHi109NHOjD_BZFmbmOg-1; Thu, 20 Feb 2025 16:12:40 -0500 X-MC-Unique: s4CHi109NHOjD_BZFmbmOg-1 X-Mimecast-MFC-AGG-ID: s4CHi109NHOjD_BZFmbmOg_1740085960 Received: by mail-qt1-f199.google.com with SMTP id d75a77b69052e-47217c14be9so37528441cf.1 for ; Thu, 20 Feb 2025 13:12:40 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740085959; x=1740690759; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=MGcNCNTpXqgcRd/FOw7KJikXMi7I5/lkRCqCXZ2bgxA=; b=iNwCBHJhVifX83CT/2hU0yHJ0jXSq8oj9nPCDnmkkXwduW0HnmszDvOjS7lGnd/NsN W72l8a6Bs07s8ULuFqoE/ypEQHM9bdTWayFpBB2LnkghL0+pIjO6PIVhyebUXjWBj7+K q0JYpW5WJX+jq/aZ7uxql0jmfJTyCA8f2Af01I4flQ7SwieV+BdxyPYcAvLNn6Cd2Fm+ 74Vm9NtGJWt7Uv+2aUAZbLpDacFQgN/S07N7Uwy1xhc+jsbOeocw3u1U0b3f74xPRb+X LqCDWxdBtbbNIBNHukh907blxVTqZTSkRTywRw0tiDOCckOmVDFdnl+itqPSIjL2hTv2 m8hQ== X-Forwarded-Encrypted: i=1; AJvYcCXRztMzm0MTOTajc47tqXO5qZvYwgj4kYwqE4vJRp3YZ6iI/QreA18V4bBakgI7PMiin0nqV1WvnA==@kvack.org X-Gm-Message-State: AOJu0YzfgnOKGq0ZDKwwwzsh9PtCKnKAoHj9d4CFinhXfaXVgCoQK7L7 slSRAEMbcgS8JvgDYrSs08a6UfMmmO7sVBPcjv5mR5DUyIcNl2m7zE6wOFpSFnLQQ9Yh+hWGGbI jXqN1vw+HYa8smy7oAMxiImG3GwZCRuHYC6Fvl1B+uI/Z6teK5hO34FWu X-Gm-Gg: ASbGncup9oAvptqx9L4oW/BWRq9h9qjjDvS6bpNRxaSkQ0YqzJa2wU4WiAfK4qiU2E7 EJqotq1Nq4kAVk6fFrbpcIdHCJhisbARfcQSs1QpO9z5UAlkGV8joI91jZIvQrbN3C2BIl3TAU0 o+G3Kt17wjzRW4DfWzbCRLSb0UIzvH3YLfq2HKLn0QPYLUJhBs/LUwWiULDaunHHwFr54coOUrz KUjti3GloZaHD8jfvAaY3PgNiv1IqPjCQaw96yc440nzzqsUOwq0lx/crgPRzXOi/pyJr7L2bSv PCRh X-Received: by 2002:ac8:7d12:0:b0:471:f9e7:5042 with SMTP id d75a77b69052e-472228c8a05mr12768571cf.14.1740085959470; Thu, 20 Feb 2025 13:12:39 -0800 (PST) X-Google-Smtp-Source: AGHT+IEeQfu058LR1kRfqUXheBmLeMkDI9GrPLeFQePU0jOVNla+OHHae6F1M0ual5fuW+EP0kWHsA== X-Received: by 2002:ac8:7d12:0:b0:471:f9e7:5042 with SMTP id d75a77b69052e-472228c8a05mr12768241cf.14.1740085959143; Thu, 20 Feb 2025 13:12:39 -0800 (PST) Received: from [192.168.2.110] ([70.52.22.87]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-472042563edsm31506991cf.2.2025.02.20.13.12.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 20 Feb 2025 13:12:38 -0800 (PST) Message-ID: <14156018-a4ad-48ba-98e1-4b1f6e732dd2@redhat.com> Date: Thu, 20 Feb 2025 16:12:28 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 1/4] mm: page_ext: add an iteration API for page extensions To: David Hildenbrand , linux-kernel@vger.kernel.org, linux-mm@kvack.org, yuzhao@google.com, pasha.tatashin@soleen.com Cc: akpm@linux-foundation.org, hannes@cmpxchg.org, muchun.song@linux.dev References: <3f0e058aef3951b39cf6bb4259c247352d4fe736.1739931468.git.luizcap@redhat.com> <4cb93166-29fd-4aea-965b-5dfb62d4dc8c@redhat.com> <0a8bd481-0b97-416d-935e-84828016445d@redhat.com> <70971ae0-3933-4e55-983a-24c6b65ef913@redhat.com> From: Luiz Capitulino In-Reply-To: <70971ae0-3933-4e55-983a-24c6b65ef913@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: N-9XAiSe5gic70COWPyzEoGfZizVntRckouCnbmfFSc_1740085960 X-Mimecast-Originator: redhat.com Content-Language: en-US, en-CA Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Queue-Id: DC38B14001B X-Rspamd-Server: rspam07 X-Stat-Signature: fnh8y3ipauabdezzdkx1g5uap7pc9r59 X-HE-Tag: 1740085961-518951 X-HE-Meta: U2FsdGVkX18h4vdJnYLGF+A3CVW/1Cpm2oBmfG/HaH0bpZs8HXU7pUjiyxp9siEvbIlD/Xa8RGGbv1+eyzr1JE9eSJjBSzq5/XgfDh2svL911tN+jc6K/8clk5JehfRc7vNj0kaCBdK+HE8eu7IF41MdIF+UXSCoilDOvvGjl6YTTHG4j5MTeZKcV/LCx5M0f1hVT9c4p9nvmqJYxx7lxtwlaamuJBcuJlhj5RdyXe/izXlZ0JyGuAP15m3KzBNWZwvR4KVpMqWxsgyPE/oeQgKmkAnEwHCc8K7gcO3jva7frQRnbEH9VY5fnS+aylZLfXWMpQ2TVa0emlAwzHQy8pv7tn1ROwZpf0dFnqSaz5gEpzg+ObW9GBVqQ9Wz58j7qqWIQuUeSwfYEM+4lJf8F359Y8cT282ZMbowJKIiiqlKJM9VNQqD76MILIQeGRoMWfV2Sy+2m0AYE2HXJq1otf2JPkLRqh4j2pJL3M+AsvSzBmx2PraPCQdcScqZpiBU18e2AtEkP3Ez18PRr35VpqBCqX9BzaRyeQsRwHjYjdKLzQFYwfdSixTnT0Z2CommQ2SSBwHNnnKmYuvk4eaN+hxuZ7Rbd4YEF3MzLmfVtckzL2CluYT2b9UwB4KTIa7VzGTpEDlhTXWRnnfRKvfl9BIell280dIakFzTltqyOtoTYrO1ck9bj3hS7xqNXktROd+7Uqyh48K20Vm4vnMYRSdT10ceb4dWP1GEZd82sK3SCvFZeqjyzpqNE8+ZVZXf6DAI2yj7d6P4cPa1FoICpXRdPlwyCyCACoEu7avo73SrHp83mmg7O2DbkpaNUy1um/usoJNo7bp/m4RLPpKUPFhTqp30fG3hZ94tnkJMaNxunR/85fmNi0T1BgmD58xatWSPaxHFs1UHk80as2EDxG/+7YijTnnDP0k/DvtclQYDqG0mn6FxDT5olFilFdEk8ogUxMJylMiKhW5IDOM 2XUd1U0F BvzpfU7+ynU+hbRTCrcX3bQFbULKUqFAlqaK3YXD+uiwD2mArlMcuSpqaIqpHRTByuMTeSGOrRhktwCsFTj/O8Lbmd/AovoXhvyHbYSQjK73J0R1tUpJuwdaYyyK3+4OH3afdy1ugLesKww8KhYWGLAcUd1T00E8cFp/zizp9j+m9bfdh4VnMiUm/mHPmzHCZ5FpcE8SM2qtkrDikfiWynBuGaK9EDccadW3nJdT3M5YYpSGq3G5CwOgoW3W9HRWLVKnoF8sRJVH2Bs7+hj6KgR34qXmwFmFScyZ9/bdCFy1qH3WcJCgjjrVM/PJjPxIAIo0ZxIKKsDrxZse4gr0MbcAX/Kh1xNrASVILrXqm5uihb/2LTUiFzyxTstncNe19nXFc89XR8/SMWwi76QdCWd44KKAvyPJzkQJK43++MVgfwabu/DYSsaWDnkHnMGTQhjBr X-Bogosity: Ham, tests=bogofilter, spamicity=0.000084, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 2025-02-20 15:45, David Hildenbrand wrote: >>>> +    for (__iter.index = 0;                                 \ >>>> +        __page_ext && __iter.index < __pgcount;        \ >>>> +        __page_ext = page_ext_iter_next(&__iter),      \ >>>> +        __iter.index++) >>> >>> Hm, if we now have an index, why not turn iter.pfn -> iter.start_pfn, and only adjust the index in page_ext_iter_next? >>> >>> Then you can set the index to 0 in page_ext_iter_begin() and have here >>> >>> for (__page_ext = page_ext_iter_begin(&__iter, __page), >>>        __page_ext && __iter.index < __pgcount, >>>        __page_ext = page_ext_iter_next(&__iter);) >> >> I can do this if you feel strong about it, but I prefer explicitly over >> implicitly. I moved the index into the iter object just to avoid having >> to define it in the macro's body. Also, the way I did it allows for >> using page_ext_iter_begin()/page_ext_iter_next() own their if the need >> arises. > > Ah, I see what you mean. > > for (__page_ext = page_ext_iter_begin(&__iter, __page, __pgcount); >      __page_ext; >      __page_ext = page_ext_iter_next(&__iter)) > > Could do that I guess by moving the count in there as well and performing the check+increment in page_ext_iter_next. > > That looks very clean to me, but no strong opinion. Having the index in there just to make a macro happy is rather weird. OK, I'll give this a try. >>> A page_ext_iter_reset() could then simply reset the index=0 and >>> lookup the page_ext(start_pfn + index) == page_ext(start_pfn) >> >> Just note we don't have page_ext_iter_reset() today (and I guess it's >> not needed). > > Right, was writing this before reviewing the other patch. > >> >>>> + >>>> +/** >>>> + * for_each_page_ext_order(): iterate through page_ext objects >>>> + *                            for a given page order >>>> + * @__page: the page we're interested in >>>> + * @__order: page order to iterate through >>>> + * @__page_ext: struct page_ext pointer where the current page_ext >>>> + *              object is returned >>>> + * @__iter: struct page_ext_iter object (defined in the stack) >>>> + * >>>> + * IMPORTANT: must be called with RCU read lock taken. >>>> + */ >>>> +#define for_each_page_ext_order(__page, __order, __page_ext, __iter) \ >>>> +    for_each_page_ext(__page, (1UL << __order), __page_ext, __iter) >>>> + >>>>    #else /* !CONFIG_PAGE_EXTENSION */ >>>>    struct page_ext; >>>> diff --git a/mm/page_ext.c b/mm/page_ext.c >>>> index 641d93f6af4c1..508deb04d5ead 100644 >>>> --- a/mm/page_ext.c >>>> +++ b/mm/page_ext.c >>>> @@ -549,3 +549,44 @@ void page_ext_put(struct page_ext *page_ext) >>>>        rcu_read_unlock(); >>>>    } >>>> + >>>> +/** >>>> + * page_ext_iter_begin() - Prepare for iterating through page extensions. >>>> + * @iter: page extension iterator. >>>> + * @page: The page we're interested in. >>>> + * >>>> + * Must be called with RCU read lock taken. >>>> + * >>>> + * Return: NULL if no page_ext exists for this page. >>>> + */ >>>> +struct page_ext *page_ext_iter_begin(struct page_ext_iter *iter, struct page *page) >>>> +{ >>>> +    iter->pfn = page_to_pfn(page); >>>> +    iter->page_ext = lookup_page_ext(page); >>>> + >>>> +    return iter->page_ext; >>>> +} >>>> + >>>> +/** >>>> + * page_ext_iter_next() - Get next page extension >>>> + * @iter: page extension iterator. >>>> + * >>>> + * Must be called with RCU read lock taken. >>>> + * >>>> + * Return: NULL if no next page_ext exists. >>>> + */ >>>> +struct page_ext *page_ext_iter_next(struct page_ext_iter *iter) >>>> +{ >>>> +    if (WARN_ON_ONCE(!iter->page_ext)) >>>> +        return NULL; >>>> + >>>> +    iter->pfn++; >>>   > +> +    if (page_ext_iter_next_fast_possible(iter->pfn)) { >>>> +        iter->page_ext = page_ext_next(iter->page_ext); >>>> +    } else { >>>> +        iter->page_ext = lookup_page_ext(pfn_to_page(iter->pfn)); >>>> +    } >>>> + >>>> +    return iter->page_ext; >>>> +} >>> >>> We now always have a function call when calling into page_ext_iter_next(). Could we move that to the header and rather expose lookup_page_ext() ? >> >> I personally don't like over-using inline functions, also I don't think this >> code needs optimization since the current clients make the affected code paths >> slow anyways (and this also applies to the likely/unlikely use in page_owner >> and page_table_check, I'd drop all of them if you ask me). But again, I can >> change if this would prevent you from giving your ACK :) > > Well, 512^512 function calls for a 1 GiB page just to traverse the page ext? :) Page_owner may allocate memory, do hash lookup and what not from that code path. But you have a point that other clients (such as page_table_check) may benefit.