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 ACEFDD60D03 for ; Tue, 19 Nov 2024 00:59:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 299FE6B0089; Mon, 18 Nov 2024 19:59:32 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 222906B008A; Mon, 18 Nov 2024 19:59:32 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0C3DA6B008C; Mon, 18 Nov 2024 19:59:32 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id DF25A6B0089 for ; Mon, 18 Nov 2024 19:59:31 -0500 (EST) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 5EB22160149 for ; Tue, 19 Nov 2024 00:59:31 +0000 (UTC) X-FDA: 82801035576.04.2A0FE5A Received: from mail-lf1-f41.google.com (mail-lf1-f41.google.com [209.85.167.41]) by imf06.hostedemail.com (Postfix) with ESMTP id 22ABA18000E for ; Tue, 19 Nov 2024 00:58:53 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Yom7xkuu; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf06.hostedemail.com: domain of richard.weiyang@gmail.com designates 209.85.167.41 as permitted sender) smtp.mailfrom=richard.weiyang@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1731977903; a=rsa-sha256; cv=none; b=sDkJ1lDvwpgrxI/3zAVtvjT8ZMuKIxDrHpCO8SauJp+qWVPP7zk+KfN9LSIxiZ2QfjfjEB DOaxyPK1+ENbqc0XBFe/IyDE4lRVf2SuxL+PqCTyWqu7YdEEgzT6sf2j7L/lEyZhYQN6Up IKriRgMg3aW9zFvnExZfJViW1N5w9as= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Yom7xkuu; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf06.hostedemail.com: domain of richard.weiyang@gmail.com designates 209.85.167.41 as permitted sender) smtp.mailfrom=richard.weiyang@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1731977903; h=from:from:sender:reply-to: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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=gs/4+8NbzZ4oMfdU1adTNNLBgfddTYjhuqhFVJxzScI=; b=sd0+EanJcxsL5JeebhF4R0JIlgijjjVx0sXXX4uqAi+/5zzylyL8tHfDa/+QouZZdm+/my 9qo7Jyjx2O9J/fbxZ9DNMrmhzE478o3xL5SThnCcxPZ7FjacovGj3iHEF/u4CuwHtKki9O vhY/Lk81XbLRpqutT8IX/sGtK6+GXfY= Received: by mail-lf1-f41.google.com with SMTP id 2adb3069b0e04-53b13ea6b78so4170208e87.2 for ; Mon, 18 Nov 2024 16:59:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731977967; x=1732582767; darn=kvack.org; h=user-agent:in-reply-to:content-disposition:mime-version:references :reply-to:message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=gs/4+8NbzZ4oMfdU1adTNNLBgfddTYjhuqhFVJxzScI=; b=Yom7xkuuzapq9QYCyBHQXtLLvi39VDywQi2yiuSjeC5ou7hqfSfNw8HiaX0Z2uAnSj bzUH9xpOu3VV+ybL1jcnWq7hWXQJuIQG+KPkAgRzy/dH/GaWLfjeJa/CUGvBEScIG+Pg tqLrpaNAN2zwXHC48qCTXSnlaG6YQiQwIh0y4oUBVDH0ZVNZWUPUGeMg9M9C61/6YW5n YrK/UDs+Dc4Kcv3UlEIiOiK8vB5uf0fvhZGiq6WCvcELEThJbHqj17fCw08cA4iwlQ9Q yCbmvLqysIF7b3RA5hMBoCzY0u4qpUg3tSnWFzMR5N19uhGob158nSR1PmT7hsMnfgsX eNyg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731977967; x=1732582767; h=user-agent:in-reply-to:content-disposition:mime-version:references :reply-to:message-id:subject:cc:to:from:date:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=gs/4+8NbzZ4oMfdU1adTNNLBgfddTYjhuqhFVJxzScI=; b=cTxOKtzfueZ48Vayk/364s9E5Xrdlf6dh9mDjqyiMuiSlRL3XhdofSGGSTvXTOWpF0 tYAEQxGVTqdcUw8OJss+LM04JXtvU5mu2YIlCjAGkWJSZQQ7t1aolnkkKGbYS1kl9P1t byUVJJpMx1H2uRtL7mzCGJiwqppkQMzvuQLVSzFEY7O6Hp6d6qI6pi9MF+K3RjQALecN QxOcOEnrQqegMAMQtUcL1AfinhO4NXOVL10hobN41HiU+5K9WDh6f6pzdwQ6Q+9qVhwt XLoIC5EXoDooQl9Vzsw1M3bbyV6DO0pbJsLEi/cEy7Z2TtCHpU5NtdBwWiRKRGnAt8dW 2hAA== X-Forwarded-Encrypted: i=1; AJvYcCUuGEynqzdFiGos5d0UbEEJNvpzOCGLDhjoiQsoBzBe4+6/K8Ra0gwSN3GpjVpWrHYdw5bO1QNSyQ==@kvack.org X-Gm-Message-State: AOJu0YwYhJgEAuSRh1UpLyuAdn2W44H4/FonZyvvWk9kOAmEtN+j+AtN qz1AYbY0xHoevs55RXJLJp5v4JF9SdqAOW/54AL/jfpggUK7jsSt X-Google-Smtp-Source: AGHT+IFg2iJ/awLg50ND0gmHiU/EsRQUV35MidArFdSNFmavmsZffwOBZ4fvQpmajuu8MxlmzlGi8g== X-Received: by 2002:a05:6512:2820:b0:53b:154c:f75d with SMTP id 2adb3069b0e04-53dab2a9e83mr8291667e87.31.1731977967122; Mon, 18 Nov 2024 16:59:27 -0800 (PST) Received: from localhost ([185.92.221.13]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-aa20df26d42sm591451366b.33.2024.11.18.16.59.24 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Mon, 18 Nov 2024 16:59:25 -0800 (PST) Date: Tue, 19 Nov 2024 00:59:23 +0000 From: Wei Yang To: Lorenzo Stoakes Cc: Wei Yang , akpm@linux-foundation.org, Liam.Howlett@oracle.com, vbabka@suse.cz, jannh@google.com, linux-mm@kvack.org Subject: Re: [PATCH] mm/vma: check retry_merge only for new vma case Message-ID: <20241119005923.jgaunkxm6od54gil@master> Reply-To: Wei Yang References: <20241118021823.17386-1-richard.weiyang@gmail.com> <20241118123439.4zi7ebdelwg6e6wh@master> <7e550e5d-88f7-4407-91e5-2b4ae177b05e@lucifer.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7e550e5d-88f7-4407-91e5-2b4ae177b05e@lucifer.local> User-Agent: NeoMutt/20170113 (1.7.2) X-Rspam-User: X-Rspamd-Queue-Id: 22ABA18000E X-Rspamd-Server: rspam01 X-Stat-Signature: utgzn56damijqd1xufawoemf579mk37f X-HE-Tag: 1731977933-233399 X-HE-Meta: U2FsdGVkX18/WwyA7FbfMpWDXrDe9iA08/ZpSktrXIK85gnwuDnGU3/3gjd3D98C2wX2W3VHzRDXDj5C3fQNNB1Kkin0dXOEV69f6ekObrkSQFlz/iofBPQqRNqHZsa8KV5CN60c2p77eeI4XfLbH/DmNvtvH3JUcYU2bM+DCNV9L01LfzwcmkTSk6Kh097q9KlIPC7cQjoY330pcGXW6m6Lz7nJ32yQu8r6ultdhS9Gn7T5WD7vW7XUNl584fNoOhf0obOJbSpZEJzo2GqD3+d3Bbuvht/Ei6sgny89Sz61lSNZ4hzyY5Kc/kl6c0Cek4KG/tJRE1t3YDHXU+GpwkNa/Kd9ysot6qVTtK77rcEyhvyyLopp9T9Nqk+l3XqV0ZQ8w3nRwkHew4uAcl6c5YNphxS14cCAvzHCM5pxKPCtAzcW3nCdxaIqaLObAp3OrUJ/8iv1gmqQa0yZsboxdKbRmdhKHSm6I3kHV6qF/7X+cM71C2g7B5nYrDNvo2nI7xx8I9ER0qVPcB+SJSfYXkq5sNxT0f53Lsou+p6pmdtCpZHfCTejeKM4Dq2UIbb9XA9xzCQct4WuSEAmZdMo71IQs0im8cq/56LJyv9h8Uidy1nQSyvV8RSCtQw9bDrsFi8xS48C4ps0jGOE9sG9NdoooNNuWQ5m0jjAa4vHRjwWBW0uGxTmKDFEEIIbI2e9dX+iljfAm5pT56Y0s02GTa1WAoG3ky65BjSyA3hw6CWWFL1cijO8R/CwXQ94WiNtvpdFoUDuFRlz4mKk4nZEq3j3HRCLr7INrgP/Wu6r7lU3fc+yGIY77rHAw7B9yyEen1M3JUNPxvIQwzUi29m1kXD9iN9afeVfrjZUbk2NC4MIMJtGDXrRA9empHGrRCW4tOzNGu3ExLzRkR5VKxj/7uF3au/KE2JIMLqSoPDKsDhmRKDk0IL0WiXksWAuL8FArs7qYSWcVY8l9usVa8i oXxThbSG XiJE5Rw9UqSxOWX3Viq5EtqnAXe8tcnJxLOZtFwMLVlEy53ly70ecGMLTzCQ2OnFm9gItEhMjOzBtZmNN2ZiVaONxO1V7/ND4hxvFgHRBuBSFymhze91gRZAldnd2Hu9TY2chZuXfGeYtpgjU92+vQ3/c2AHKmhWY7KyL8htuLlCLyTpZKgO+tSMPDoNkj2P/raHQj9+CHDlfG8H+nxq9FVwxZZ7sVL48P0B2Eq9/QDlIYOtECL9X+o2i+kL8zSk0aXV5KQ9GkGWCRlfDYEeErwP1ioxm/FQ5ajeVGfp7WiBPeizbouNHTjXkPViK7lMVr8Sp+gMR+scG8jqTOwCQ+1aShQyH1Jy9ThR/07A8revxl8ZhX1Sxtvr8gxpa3iNDN6wNcc3nW6tdg1h9aXDyn57i7dQS6C2Eusr5QkBBEdm1jli2Ae9DxGgCvgs132ZakamaapJF3q6UagWTi48l9uARMkXy2ws6McOGdzBT3Ct8x1YlhCK5MVQSIjpNa56BSW3XbbHpvfd5sPio0lrANJhqDWFFNBcbXlTBbRqoYkOA1H7+Ue0V5WrNiNqPhuEVHeJPQyz1RKuEjphooQqLWg+G/q24a3dMrTIJEsQiRxqM7Tv/hHWG08b9FMuJ7HdR9Ji8FVP63LG9mKPLa3Y92Wehvh4wwaBE2HcDatHl/pc9Rc/y5mKImyzg8nlnDVaQJ68W X-Bogosity: Ham, tests=bogofilter, spamicity=0.085188, 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 Mon, Nov 18, 2024 at 12:52:48PM +0000, Lorenzo Stoakes wrote: >On Mon, Nov 18, 2024 at 12:34:39PM +0000, Wei Yang wrote: >> On Mon, Nov 18, 2024 at 10:04:44AM +0000, Lorenzo Stoakes wrote: >> >On Mon, Nov 18, 2024 at 02:18:23AM +0000, Wei Yang wrote: >> >> Current code logic looks like this: >> >> >> >> __mmap_region() >> >> vma = vma_merge_new_range(&vmg) >> >> if (!vma) >> >> __mmap_new_vma(&map, &vma) >> >> __mmap_new_file_vma(map, vma) >> >> map->retry_merge = xxx --- (1) >> >> if (map.retry_merge) >> >> vma_merge_existing_range(vmg, &map, vma) >> >> >> >> Location (1) is the only place where map.retry_merge is set, this means >> >> it is not necessary to check it if already merged with adjacent vma. >> >> >> >> Let's move the check and following operation into new vma case. >> >> >> >> Signed-off-by: Wei Yang >> > >> >Nack. >> > >> >As I said to you in another patch, the aim isn't to find the mathematically >> >smallest size of patch. The aim is to keep things readable and nesting >> >things like this works against that aim. >> > >> >> IMHO, by nesting it, readers will notice this case only happens for new vma >> case. So we don't need to retry if vma_merge_new_range() succeed. > >By this logic we should not split up the functions any more, because the >reader won't know that it's only if you create a new VMA AND it's >file-backed AND that succeeded AND etc. etc. etc. > >So by your logic, we should undo every refactoring here since we need the >reader to understand precisely the conditions under which this can occur >right? > >Or where do we stop? Because now it's 'misleading' to imply it might always >happen in this situation without stipulating that there are other >conditions? > >Do you see how this breaks down? > >We encapsulate the circumstances under which a retry_merge occurs, using >the map.retry_merge variable. That's it. > >It's simple, it documents itself, and a reader wishing to know when this >happens can very easily find out. > >The purpose of this code is - again - not to be as small as possible or to >avoid all possible branches (unless perf numbers guide us to do so) - it's >to be readable. > >In the original refactoring I can see: > >- prepare for mmap >- try to merge >- if merge failed, create new >- maybe do a deferred merge >- mmap complete, do 'after mmap' tasks. Thanks for your explanation. This looks neat. So it hide the reason of deferred merge from user, we would think both two cases would lead to a deferred merge. This is what we want to have, right? > >In your proposed code I see: > >- prepare for mmap >- try to merge >- if merge failed, create new > - if merge failed, created new, and we required a deferred merge, do a deferred merge Maybe here is - maybe do a deferred merge >- mmap complete, do 'after mmap' tasks. > >It's about humans reading this and being able to understand what is going >on. > -- Wei Yang Help you, Help me