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 B8BC9C48BEB for ; Wed, 21 Feb 2024 20:44:02 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3A1776B0088; Wed, 21 Feb 2024 15:44:02 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 350C36B0092; Wed, 21 Feb 2024 15:44:02 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2182E6B0093; Wed, 21 Feb 2024 15:44:02 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 12CC06B0088 for ; Wed, 21 Feb 2024 15:44:02 -0500 (EST) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id D5C621C0C23 for ; Wed, 21 Feb 2024 20:44:01 +0000 (UTC) X-FDA: 81816987882.19.9D03362 Received: from mail-wm1-f51.google.com (mail-wm1-f51.google.com [209.85.128.51]) by imf04.hostedemail.com (Postfix) with ESMTP id 0D6AC40017 for ; Wed, 21 Feb 2024 20:43:55 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="Lo/LN2B3"; spf=pass (imf04.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.51 as permitted sender) smtp.mailfrom=lstoakes@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1708548236; a=rsa-sha256; cv=none; b=HbLT/SZfYbc/slRl+1270cTMOOKqwCWsa6MdSAVd+OUR01QL0ypOEiXfYhwurl7R5mzsqc 1QCBEvnuU7XkKPvaTJRCoatP56HcHGT7J6mAO20w8dlO9kTfo3u+AXobfD+srOXZoNpXFq sGqE6MR5gZLl4kbF5wweOqtQLDaG178= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="Lo/LN2B3"; spf=pass (imf04.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.51 as permitted sender) smtp.mailfrom=lstoakes@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1708548236; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=RQKBdsBXJqQPwRNGAXBAfHtnvpBdZt9I1p+3iXhYFuE=; b=R17ycduvbRtmtzCMYQYsra9RH58Zd3/Q3W9iiw1Cvup0P6+sDVuZEpDY7Gh5RFUrw13ZzG fe02fKJ1No5g1z0Vzl0KXt7ht87K1tlagnh0nqRTDpLdwiude7Crgm31yd2mkrTxcPeaTE Vss78MweqP8PTUXLFVtFdPlG6Hs9LIU= Received: by mail-wm1-f51.google.com with SMTP id 5b1f17b1804b1-4127946aa11so5203675e9.1 for ; Wed, 21 Feb 2024 12:43:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1708548234; x=1709153034; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=RQKBdsBXJqQPwRNGAXBAfHtnvpBdZt9I1p+3iXhYFuE=; b=Lo/LN2B3/HG4ks6kGh+ByhTjP9o6kjVQ+h3/ZCuWQRl3bBgICR5Kkjf8RkjxvPmAof m1ize+YXB8qJ5OfHPAFdwG4/poIdAwmStag2JM3xxm+AUlnwQJ7XCaXvR8GAWJ5vi576 a93CUaedS46QxkPf+VTu1nzK6dXbTxTS6oKs+Lh8DN1LHrW9vSg6xgsGoh/UIOSvF/J6 24YMNPG1/7lIk4hSHrGe6pf3XLLTKZbFDKQrsXNxUZsx2iyfv7DhsQmtajewgnGMKIfL pH3/nwtG5n64OGJszY61iKpYsHyTQ0lBqLTqjh9+7nFgWGPq+5wORkQcdYDSHmKQ8/Gt mv+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708548234; x=1709153034; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=RQKBdsBXJqQPwRNGAXBAfHtnvpBdZt9I1p+3iXhYFuE=; b=FZk9TllevZsz3dqBkuJ8qxXjSPNx5Yp0fVaKopICboQnVHCjEQSgo8ggCQe7wZMTdI YSokdy9PIyE5kFqedIphc5YYgGJjT2DWDio7UmlHralp3RBjZL6/jvn/NeJdLfVefyxZ zN55vfZ3uwo1q5DMM04sY2Ghniz7tc/q95VLDm1htKJI2HX2wUmBcL775Q5v55+LrzDd 1srmoYmir6gbCagBml+WQWq5TlQIKuEitlrY7UJF++UzwKWaAJh8D2zrrjJdcmrdKgCA 48XJQ2WVmxf31DLOxPTcMDhtBcld0KAkj3HYv7bLIxiNmB+PuJOzd5FM+EaCWVDZYxOA kNQA== X-Forwarded-Encrypted: i=1; AJvYcCU/u6CZTGRw8Jv3m1hKgQmTKe1xYhOLY9LhcPJHnzBORhm/6AZ4AXqE9GOJZMf0aZEeXLDFGrD0Gfl/mAkjhoKSk+I= X-Gm-Message-State: AOJu0YxzYdKn0eFO5KDgGXWr29XfTXRw5oQJTdzDigBp0ioh8SW+OyjR 1XWyTt6vplxPh2c+1p6Wtc/n6hRBfTPihFTxb/O7ZnukFzFpoZyZ X-Google-Smtp-Source: AGHT+IGgtUvjV/jJGCiYwhCbpRW1x+SE9MZ7JJC29jUKrNye7C5xmVAOxyBEWykJhlJrHuiU8UEJBw== X-Received: by 2002:a05:600c:6cf:b0:412:d8d:1d98 with SMTP id b15-20020a05600c06cf00b004120d8d1d98mr15226515wmn.37.1708548234197; Wed, 21 Feb 2024 12:43:54 -0800 (PST) Received: from localhost (host86-164-109-77.range86-164.btcentralplus.com. [86.164.109.77]) by smtp.gmail.com with ESMTPSA id bp24-20020a5d5a98000000b0033cdf4bea19sm19940088wrb.9.2024.02.21.12.43.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Feb 2024 12:43:53 -0800 (PST) Date: Wed, 21 Feb 2024 20:41:39 +0000 From: Lorenzo Stoakes To: "Liam R. Howlett" , Yajun Deng , akpm@linux-foundation.org, vbabka@suse.cz, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] mm/mmap: return early if it can't merge in vma_merge() Message-ID: References: <20240221091453.1785076-1-yajun.deng@linux.dev> <20240221153827.wkmjnnwsf6lyxatc@revolver> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240221153827.wkmjnnwsf6lyxatc@revolver> X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 0D6AC40017 X-Stat-Signature: s89f1h7tnksjhpz7ghqtfz67ichmsu4o X-Rspam-User: X-HE-Tag: 1708548235-315272 X-HE-Meta: U2FsdGVkX1+eDPAV6qPVwoYNZtCwDnVSiWjjQOjhOhDZdkG7NOx+52q5VH8Y9l8tqi50bQo9tThaC/XseOMY5p0t3rH9cfr6K2gSfcCbTR+JBGHdH/GDWDoOQdRDUHjI3vrOIsvHCb8y5VqQo+H0eo0aQmAs1ZgjEjEFRfvVAobC/Cb0nkf+r7IS+yvPAepYSs/gau0YifUQpFhlpXL17EL331hQnbq65V2ZBZSfOsWtQDkVCffDvS9QyagLn9SQe8me8lgiqCpj7U8qov8UcZrQO9hCChadsDW16aZfxG6SYpVnwEGYSV2SqPHj6+INYln9OtLb4bho8mQGoQrsDR9gvmnIhM4dL9hTSeRR/fye8P11waT96iqeGlZV/E6AD/7ZKmpkPBaD53Y2towTSsHQCoX1Q1Dnk7sbwc3OkhdcgV8WnCy5Tk2lyGkFcPF8zFbk+gzCguD6Rm7VjdTJF/ljDPKjcZ7hloPOBm/YeiSLRIUI0ncbk75y4sRGgKGtSMIAp/U6M7L/H7XonhbJfu9je3C2e6601+y5Y+wIlKX3yTgUV+2d+eFWsYsLrrLvJmLnSt53knY597qL44wtB3H6Rvekb11b6tHzxCR9vhnY3ySwDbyi9wsxsVRMwUl3r8ea5EPA399okn3Yd5FSglv5FMeruYdhH1owjojjL5TaIzC2IrX1LV+sRdoZdkyII7pjxbS3Yizwlh7GHxaZnGJBldlf82OfIEA8qxilpzVqQs8JkIH0DTbB1bgnbAQO6rgw3iN0xcp/O9dqICJqVrsLMwQ6acQC3Icv1aBKJJ/41b4Ss7uuNPha48InTHATlKmH1IQhUic8HGRYbPrSklz2BZVbkNuXGVYw6iT+75Lyn3x/PUp9uRbkcEIMYSk2fzLRiO2bSz7NNkVUdk7Pm0sEFFOtaBmyE0vEYZKsQuzwR12+7wjwc0bfRgEufZn2lV7ri+B7P+ku2Rbl9JD /XACXsJj kvkgHPB8ydwBxMZIj8P64PBtLCiM4zXJYeJOrFckb75MO3XjcMdwVPCoZzxAJz3icktrTNnuBe2n0oxc= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000043, 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 Wed, Feb 21, 2024 at 10:38:27AM -0500, Liam R. Howlett wrote: > * Yajun Deng [240221 04:15]: > > In most cases, the range of the area is valid. But in do_mprotect_pkey(), > > the minimum value of end and vma->vm_end is passed to mprotect_fixup(). > > This will lead to the end is less than the end of prev. > > > > In this case, the curr will be NULL, but the next will be equal to the > > prev. So it will attempt to merge before, the vm_pgoff check will cause > > this case to fail. > > > > To avoid the process described above and reduce unnecessary operations. > > Add a check to immediately return NULL if the end is less than the end of > > prev. > > If it's only one caller, could we stop that caller instead of checking > an almost never case for all callers? Would this better fit in > vma_modify()? Although that's not just for this caller at this point. > Maybe there isn't a good place? I definitely agree with Liam that this should not be in vma_merge(), as it's not going to be relevant to _most_ callers. I am not sure vma_modify() is much better, this would be the only early exit check in that function and makes what is very simple and straightforward now more confusing. And I think this is the crux of it - it's confusing that we special case this one particular non-merge scenario, but no others (all of which we then deem ok to be caught by the usual rules). I think it's simpler (and more efficient) to just keep things the way they are. > > Or are there other reasons this may happen and is better done in this > function? > > Often, this is called the "punch a hole" scenario; where an operation > creates two entries from the old data and either leaves an empty space > or fills the space with a new VMA. > > > > > Signed-off-by: Yajun Deng > > --- > > v2: remove the case label. > > v1: https://lore.kernel.org/all/20240218085028.3294332-1-yajun.deng@linux.dev/ > > --- > > mm/mmap.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/mm/mmap.c b/mm/mmap.c > > index 0fccd23f056e..7668854d2246 100644 > > --- a/mm/mmap.c > > +++ b/mm/mmap.c > > @@ -890,6 +890,9 @@ static struct vm_area_struct > > if (vm_flags & VM_SPECIAL) > > return NULL; > > > > + if (prev && end < prev->vm_end) > > + return NULL; > > + > > /* Does the input range span an existing VMA? (cases 5 - 8) */ > > curr = find_vma_intersection(mm, prev ? prev->vm_end : 0, end); > > > > -- > > 2.25.1 > > So overall I don't think this check makes much sense anywhere. I think a better solution would be to prevent it happening _at source_ if you can find a logical way of doing so. I do plan to do some cleanup passes over this stuff once again so maybe I can figure something out that better handles non-merge scenarios. This is a great find though overall even if a patch doesn't make sense Yajun, thanks for this, it's really made me think about this case (+ others like it) :)