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 6112BC47DD9 for ; Fri, 23 Feb 2024 02:30:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DDC616B00BA; Thu, 22 Feb 2024 21:30:44 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D62C76B00BC; Thu, 22 Feb 2024 21:30:44 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BDD226B00BE; Thu, 22 Feb 2024 21:30:44 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id A78EC6B00BA for ; Thu, 22 Feb 2024 21:30:44 -0500 (EST) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 52B8C160F89 for ; Fri, 23 Feb 2024 02:30:44 +0000 (UTC) X-FDA: 81821490408.05.FC53F01 Received: from out-173.mta0.migadu.com (out-173.mta0.migadu.com [91.218.175.173]) by imf17.hostedemail.com (Postfix) with ESMTP id 68F5A4000B for ; Fri, 23 Feb 2024 02:30:42 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=RX5zaavS; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf17.hostedemail.com: domain of yajun.deng@linux.dev designates 91.218.175.173 as permitted sender) smtp.mailfrom=yajun.deng@linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1708655442; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to: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=Dn0DwPHukXKXbPRCk/008A2MQ+iJBle+LqZeMHn+wVQ=; b=UhnttfZXBIh3LoDqRroscY4Fh0E9b01yaKq+y8DBOBHZoMWOHUHwGe/xAEMUqlUlBRIIS1 F71sghFML3xrv2DUFyLXkPg+GtYeL3BuXdSGr/YiHtluAkyn/9l+k3hUqn53jRKKje3oHs G4n2wr/N0akyllePaezr/EzAMY2+8Is= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=RX5zaavS; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf17.hostedemail.com: domain of yajun.deng@linux.dev designates 91.218.175.173 as permitted sender) smtp.mailfrom=yajun.deng@linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1708655442; a=rsa-sha256; cv=none; b=1Jkq+RC9IsCRLQLMPPMJ9sXGCAO4/ZxkExDAHVUxmUoNMjCE16YYPO0ldQ2e+egLLO1IiD IlfF0aOTEoflR9Nl6cFHv2q+fu7gaChj81LbV52MoVYi8FkF5O7s8Vy159zHEUCD/EaTrD Lxv59BZPk9nvwM8pCzkz1vHPSqn6edU= Message-ID: <7cc1952d-958f-3867-d9eb-fd70d41b29f3@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1708655440; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Dn0DwPHukXKXbPRCk/008A2MQ+iJBle+LqZeMHn+wVQ=; b=RX5zaavSP806/U8b+yYYISEclwVh+Tr4o+XaPiXiO3bDMoeFU9jYmtlpeMvNIorn5lGraa pWNh/EPaXLh4fsR2LumXrp9VmSOfbtZ59tXQmdDqrWplHD58MOniyC/oN9NgcijlBOn5LK B1ylnjPMnHAlxl+97x0IeBD5eJvXdEI= Date: Fri, 23 Feb 2024 10:30:29 +0800 MIME-Version: 1.0 Subject: Re: [PATCH] mm/mmap: convert all mas except mas_detach to vma iterator Content-Language: en-US To: "Liam R. Howlett" , akpm@linux-foundation.org, vbabka@suse.cz, lstoakes@gmail.com, surenb@google.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20240218023155.2684469-1-yajun.deng@linux.dev> <9f76ce23-67b1-ccbe-a722-1db0e8f0a408@linux.dev> <20240220180603.dhnthx2nk5455opm@revolver> <0ff93990-b8b6-89ce-0174-4023599059e7@linux.dev> <20240221143103.bkhewpq2wirsoyqw@revolver> <20240222150703.u55kpmdlog5hrld3@revolver> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Yajun Deng In-Reply-To: <20240222150703.u55kpmdlog5hrld3@revolver> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT X-Rspam-User: X-Stat-Signature: kgypnn3pad9zkt6abh1ggcpt6r34bs97 X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 68F5A4000B X-HE-Tag: 1708655442-26645 X-HE-Meta: U2FsdGVkX1+oJVdXR0E6m2CFRys+yVw3QjZqkmFefxem3Dbr7EuG6B2uv9iti2xBuI+m2sYTvue3ym+Gb+QLBXgijU6WdWp7MEXqjGetvkDqy1CToDWPLVtoWvS9N8UKICElQGSQh9WsAgEA8YLqqUN+fB6kNHjGNOCZKIiq4a8gyWaUqH1ZtxGJ1jQiUjouRw00MrraQ61jsSHGGe7JKqjwfNz0ZsXEaLfSTw5zJA14+0wbqq4CRWLSigHWPJM3se4LqX5En7Cok7YHaA4GcAt9q2TyGwtrLy8o/YAMAe+pcuBcvRdSGZBgygVBUquG+WGjmX07WUnz8ekGca25BQUcxiapoTDAGks1cUDAK4SlyJg8cIkXu5l1o7QQpu6kLw+KN5loeiAGP0tgRgY0swHahmPBdA29qIDA+XX02oYFgQwOve3o9tpdP9o1PSS3+DCkCy94fz735busRFxySLquINhy0DZhmSjRyvS8iAgwr4FQnTN7pB/E2Amr8yU2Mqmv1+B6TF7Rsh49iiC2rHsJRHjOzygrhAiAh24s6kDyUg3hAQ3cwZuZcnEfeSX8T9KiDkOWWt0Y12CrqgdTqfoqi5dTcNOXzTt8MEeWCKS6U5bxhWqM1oKBjwWU/SGuvFmlsN/udFfJmeMfAFWncmWZ5sSCfSX/RuXpABHmhxTJTl1WOXQw2oSzlVCu1Mmaq4fay0YyyJUYUy3k2VBgxa4TkgstuBZgHqrKBDNM6H6ZRD1VB9H4pqNEfmcpC1QzuRtlTybpnJDfc2xPlw8CbLfyJl3n8teRtANQpcJXuUraMis8aEz55SDJ90rt/fhSvQCrQgK76GB1DoUM0IDFzFrp8Xz8GHJl8J8oIWrikBCC6dd7xwYourgjbLP0p2EEJxKWGe1dljjl6cYde15OUO/PEHcSrrPVYtMopZqe2GnCc7dCTNUZw/jRrihRvzdy3zMQaIjKMkjuT1w9Fjs n1kbWrCf W8vwAIlnbRXdohhBNrS4uVUJADcSq22YtsOcabCdgGrlZeliVwE5eR49jnLjVyTCIz5zAb3EIuh/DArONZ6ZvqR70Et8towxMkXCkc8kEvgW+W01f++s2sAQ4mGOuwmeL2T3o X-Bogosity: Ham, tests=bogofilter, spamicity=0.000146, 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 2024/2/22 23:07, Liam R. Howlett wrote: > * Yajun Deng [240222 03:56]: > ... > >>>>>>> @@ -1959,11 +1958,12 @@ static int expand_upwards(struct vm_area_struct *vma, unsigned long address) >>>>>>> struct vm_area_struct *next; >>>>>>> unsigned long gap_addr; >>>>>>> int error = 0; >>>>>>> - MA_STATE(mas, &mm->mm_mt, vma->vm_start, address); >>>>>>> + VMA_ITERATOR(vmi, mm, 0); >>>>>>> if (!(vma->vm_flags & VM_GROWSUP)) >>>>>>> return -EFAULT; >>>>>>> + vma_iter_config(&vmi, vma->vm_start, address); >>>>> This is confusing. I think you are doing this so that the vma iterator >>>>> is set up the same as the maple state, and not what is logically >>>>> necessary? >>>> Yes, VMA_ITERATOR can only pass one address. >>>> >>>>>>> /* Guard against exceeding limits of the address space. */ >>>>>>> address &= PAGE_MASK; >>>>>>> if (address >= (TASK_SIZE & PAGE_MASK)) >>>>>>> @@ -1985,15 +1985,15 @@ static int expand_upwards(struct vm_area_struct *vma, unsigned long address) >>>>>>> } >>>>>>> if (next) >>>>>>> - mas_prev_range(&mas, address); >>>>>>> + mas_prev_range(&vmi.mas, address); >>>>> This isn't really hiding the maple state. >>>> Okay,  I will create a new helper function for this in the mm/internal.h. >>>> >>>>>>> - __mas_set_range(&mas, vma->vm_start, address - 1); >>>>>>> - if (mas_preallocate(&mas, vma, GFP_KERNEL)) >>>>>>> + vma_iter_config(&vmi, vma->vm_start, address); >>>>> The above maple state changes is to get the maple state to point to the >>>>> correct area for the preallocation call below. This seems unnecessary >>>>> to me. >>>>> >>>>> We really should just set it up correctly. Unfortunately, with the VMA >>>>> iterator, that's not really possible on initialization. >>>>> >>>>> What we can do is use the vma->vm_start for the initialization, then use >>>>> vma_iter_config() here. That will not reset any state - but that's fine >>>>> because the preallocation is the first call that actually uses it >>>>> anyways. >>>>> >>>>> So we can initialize with vma->vm_start, don't call vma_iter_config >>>>> until here, and also drop the if (next) part. >>>>> >>>>> This is possible here because it's not optimised like the >>>>> expand_upwards() case, which uses the state to check prev and avoids an >>>>> extra walk. >>>>> >>>>> Please make sure to test with the ltp tests on the stack combining, etc >>>>> on a platform that expands down. >> >> It seems something wrong about this description. This change is in >> expand_upwards(), but not in >> >> expand_downwards(). So we should test it on a platform that expands up. > Oh, yes. Test on the platform that expands upwards would be best. > Sorry about the mix up. I didn't have a platform that expands up, so I can't test the expand_upwards(). >> And >> drop the if (next) part >> >> is unnecessary. Did I get that right? > Yes, I think the if (next) part is unnecessary because the maple > state/vma iterator has not actually moved - we use > find_vma_intersection() to locate next and not the iterator. This is > different than what we do in the expand_downwards. Yes. Since I can't test the expand_upwards(), I think it's safer to keep the if (next) part. > Note that, in the even that we reach the limit and cannot return a > usable address, these functions will call the counterpart and search in > the opposite direction. > >>>> Okay, I will test it. >>> Testing this can be tricky. Thanks for looking at it. >>> > ... > > > Thanks, > Liam >