Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
Date:   Mon, 18 Sep 2023 13:59:55 -0400
From:   "Liam R. Howlett" <Liam.Howlett@Oracle.com>
To:     Peng Zhang <zhangpeng.00@bytedance.com>
Cc:     corbet@lwn.net, akpm@linux-foundation.org, willy@infradead.org,
        brauner@kernel.org, surenb@google.com, michael.christie@oracle.com,
        peterz@infradead.org, mathieu.desnoyers@efficios.com,
        npiggin@gmail.com, avagin@gmail.com, linux-mm@kvack.org,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v2 6/6] fork: Use __mt_dup() to duplicate maple tree in
 dup_mmap()
Message-ID: <20230918175955.w6vowfschshhy6cu@revolver>
Mail-Followup-To: "Liam R. Howlett" <Liam.Howlett@Oracle.com>,
        Peng Zhang <zhangpeng.00@bytedance.com>, corbet@lwn.net,
        akpm@linux-foundation.org, willy@infradead.org, brauner@kernel.org,
        surenb@google.com, michael.christie@oracle.com,
        peterz@infradead.org, mathieu.desnoyers@efficios.com,
        npiggin@gmail.com, avagin@gmail.com, linux-mm@kvack.org,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-fsdevel@vger.kernel.org
References: <20230830125654.21257-1-zhangpeng.00@bytedance.com>
 <20230830125654.21257-7-zhangpeng.00@bytedance.com>
 <20230907201414.dagnqxfnu7f7qzxd@revolver>
 <2868f2d5-1abe-7af6-4196-ee53cfae76a9@bytedance.com>
 <377b1e27-5752-ee04-b568-69b697852228@bytedance.com>
 <20230915200040.ebrri6zy3uccndx2@revolver>
 <7f0db16b-609c-7504-bd5b-52a76e0200b3@bytedance.com>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <7f0db16b-609c-7504-bd5b-52a76e0200b3@bytedance.com>
User-Agent: NeoMutt/20220429
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?NFU1eUxjTExYMkw2eHY2bDZ1THd6YjFwdVpSQmxvdjN4OUIyWWNSMTZnYUFw?=
 =?utf-8?B?T05yaUFUTnZRTDgxOXVaY2FxOCtpVkZKaExmblA0OWgzZzR6cXJVWmFMZ2dv?=
 =?utf-8?B?SlZqZFp0SGs1QnMxcHMwTXk2QWYvaFN4UTNob25meFZkcktHUitsZlQ1Z0ll?=
 =?utf-8?B?R291YTZRSjNrem5aeGdBYm5DNjBESVhYRndOT2pnc1JKdHltNlgzeE5tYzNs?=
 =?utf-8?B?bjhmZGlLamViTHhaNmgxbC9xR1l3c2NUMWQ2azVlQXptQ0FkR2RkNFFQMEs3?=
 =?utf-8?B?NnUyT1l5OXF5cXFtUWtMLzJKTk82RGVVSm9oeHNNTWs5alYrMjdXdnhIeUJu?=
 =?utf-8?B?QnpCWHhTb3l1QzhaUTczMGl2UlVrcWtIRVAvSWRvczJ0YUQwUFRRSjlhT3ly?=
 =?utf-8?B?TWxkVThhdmZOUzhYaXA2eXoySEl6dVVCZ0xHcUd6SHFpNGI2SDc2TStqNEpG?=
 =?utf-8?B?TmVYRTVCbFp4cElmbGlibGdrUTgvWHhqSlVzS2tYakxpc3puOVUraG0vYU5v?=
 =?utf-8?B?Ui8yUVpvL25TN1YzYXJsUzYxREtFWHcrajIvT1Z3NlBWampjNXBhem5hVko3?=
 =?utf-8?B?RGx3RCt3S250UUtBRXR1NC9iaEVGcUlwQnpYSUd0dGh2UDZJN0hFZlJsVUg2?=
 =?utf-8?B?VngzSmtnenVLM255ZE9QcVpDK2ZYL2JwbFk0Y0hjSnZZQWx2dm9UaU1OZHN6?=
 =?utf-8?B?bHdyYlRFVU1BTi9MQ1JmMmpzN25NMnBReWdKOW1pMTQ1c2hMS0tEQVd6bmdm?=
 =?utf-8?B?NGtFUTNabmtMa1RweW9HZEdVTFlGZ0NNS0dZNWRuSkhOay9mK0FsZnRia2x5?=
 =?utf-8?B?U1NPMUhiMnArbVpqYVIxUUNEZGpkMmZWUDNrRVYvdElvdU11RDZmOUxnQ0Va?=
 =?utf-8?B?N3kxcUVNaHdmUmwzTWMwTkZyODcra0xsVk9sOHloNEsyaEhnT3NQN0I5eW5y?=
 =?utf-8?B?aDZta1daMU1nRkU4cnE2UjZmd1NGTERqL2Fxd09XUGZVQTZUcndjU0FlR1I5?=
 =?utf-8?B?UlUvWGljNHJsNURaNllsRzZjSGRZMExHZkRxOFN5ekFhY29PUUlkY0lieXk0?=
 =?utf-8?B?d1ZNblNKTkQycFVhUkdxSENjSDBXeUp4ZHhpQkNTdkJpcll2UlZNUGw4c2xR?=
 =?utf-8?B?c1NiZU1qWjhPZFp6UzVpSXF6cGQzc2g5MC9Xc2NIWTIvejR3OVNKck1PdTlX?=
 =?utf-8?B?Y1VQOXZ1bUJEckxQa3BVQ2pCYXFHOXZ5cGxqTmpOcU03MytlaWxjK0ErNkdp?=
 =?utf-8?B?VzRSV1FEM0VFYnFxTXVYZ0E4OGVFa3k0aWJoa0hBSm9ua1o3a200a0t0L01W?=
 =?utf-8?B?dEJIOGh0OUpuem81N3FDdnprbjR4ZGdiYU51TGtCdEZ4QVhLYmtsaE44QjNU?=
 =?utf-8?B?Q25qYTVTRElkMHVDUDZvRmp2ZGdKNE5ubDlSUWM3QlRJR1ZVcXcwdFAySXk2?=
 =?utf-8?B?M0pINkJ0L3Rvc1hJSkVPSzZaK0wzTUdob0VrUTJZa0V1YnFWMFNVUEJ2TWpu?=
 =?utf-8?B?TGNWS2Q4ZTlTWTNXbWpENUp0djcxM0pYQXpXeVZ4MG9HZFJiS0x6Uks3cm9m?=
 =?utf-8?B?eFVMdU05dk5HN0xValZkRFcrSExDWTN0bFJTTHRTM2sxN3pNRjUxdHE2TTlT?=
 =?utf-8?B?NUxaUGFFNmk3SHF6QmRadVBDK3lzQVg4TjNZZWh5MHZYdzFRbmtiRFl2Rlhl?=
 =?utf-8?B?Q0Q1WEltdlRkeG5PNU5od01LdE1pZFltSjJ0RFNiY3FwSXBMM0hwSzB1MXlU?=
 =?utf-8?B?V25UN2VsQ09ReElIQnVLMUIwWVJvZ05xenhQWVBwM09qNDNXWEJRZ1ZIVDRa?=
 =?utf-8?B?cnlFUzNhU2pReDUwU2ZrUjhhblA0Q1pOc3JnVHZLVExTeVlmYnQyVUFCVWZ3?=
 =?utf-8?B?UXYraGEwUEJZTmkvclpENks4bFZJUWVFUzcxVlJOaU8yOEltMG5iS3dUcTE3?=
 =?utf-8?B?bjkrNnVBRUpHYzdUbVNpWDBmWGhMTy9hWkNVTWU3clBKNWtkV1NQelFOM0Fr?=
 =?utf-8?B?ZUU3R3BRRm10a0w2dUhWaHNqbmNuS0FlRmoyTFBmSGFTSVdnQjJ2M3l4VXZW?=
 =?utf-8?B?SEs3bENrSnR0b20xZUpFNHNTbldrQkl4UEI5ZmZhVjNVR3BzTmRtZzFoQnJE?=
 =?utf-8?Q?pG0/RQmgZFSD5O4bt2PC5rAc/?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?WnI0YzFTYkJaQmNYNTVzYUdkMFBQTzIxSDFuZTJ1Vks1aW5IWkkzQzU5YkdK?=
 =?utf-8?B?RkFWSDZwNlFOVTZvOWtMUWxvdUtGSU9iWnVWMVRMMEhSOFg1c2V6NCtjcFZv?=
 =?utf-8?B?TXhwUVRmeXkwZ0dyWWZGUTJ2SElRT0QySzZEWmxNaTdDWjhMYWtBUHFvTWJv?=
 =?utf-8?B?aUhxWHFPUDhOMVBkYVZDTk1zSFpUdXkrbnRINWJVaFF0eU9abCtNeDMzZXo4?=
 =?utf-8?B?QVJIZ0lnOXRnR203N3NpNjhZTnp2UjYydkkzYmFzcm9jUTZDRUlabDVKZFZU?=
 =?utf-8?B?cC9GZCtBQjBYdm53WXJHNmVKRkZlRXE3R2FmNFczUjdVTEZuSTZPWE9wRnJa?=
 =?utf-8?B?ODgxenlqMCt4TTJaUjR5dG4xRVVTL00wdXY5NXNoSlMwTWJkYnlHeFdaMlJq?=
 =?utf-8?B?VmJjS2NKdUtlUnVxM0RxSU5EN3QyVURudURiMGFFd1o0V1VjYnRNd2xPM2xl?=
 =?utf-8?B?Rk1nNURJS2RIOE9nQ0dJa1JiS2lyWHM4a1M4L0Z5TjNieUxDYjdwNkFWdGlO?=
 =?utf-8?B?cFd0aEU0UG9YWHBrVE1MUGJucjVOUkI1R2pwbDZUNnR2SXB3RVpveE9lZjZV?=
 =?utf-8?B?dnJzdkdNYWd4SWZsK3BDOGxHcDFLdlVISU80M0xaczlkZEtxTkNGWWE3b3RP?=
 =?utf-8?B?VEtqQ3pEVTlld1ZTRXE5N0pnUHZWcWYvMmx4Rk9QMDlyTzlhZmZQcmgrYVNh?=
 =?utf-8?B?S3UzRzFObzB1OHJYS0V0bVdQd01JT3BjKzd6Y1dWTk5ndTh1SjhZdDdNeWc2?=
 =?utf-8?B?ZVh1Um1xRE50eVcxVnU0R3VmWWIzMGNzNy9LNDEzK1gzWURXTC9JdkFlUk9h?=
 =?utf-8?B?M092aEZBYXRWZDZ3cFcxTUFuMkVqMXVML2MzelI3T2NLVWlMYWRZMXZzbm9S?=
 =?utf-8?B?cUR2c3VBdTVuUHQ3T3orcUNKbytCRDlRMUM5MmFKdjNnZXRyMHNQNHNjTzl3?=
 =?utf-8?B?akdkUUpDdklHdG9yQ0pOemV3eVhoRkhWZG5xTWlqUHc1aXRFYXRIcFJkaC8r?=
 =?utf-8?B?MHJVa3BCeWdVL2JiZG84c1RSVnY4UWhWcDN6dDZ5UmtMWXZPelVpMXBHOXpt?=
 =?utf-8?B?VGFham1mL05LOFpRNk5lUlQvajZOZ1o5RkQ4Vm9NU21UdFNGbXNsSkhRSzAv?=
 =?utf-8?B?dkNUQ0NVd05ISCs0c1FsQ2NreUVhZ0hiVjdiOWVORkVYUCtXRXdSbHFmdFBH?=
 =?utf-8?B?eUNnd3U3bzdRVlN1c1g1ellObDJxZnA1Uzh3Rk9yYkUyMlBKcDV3RHlXMVVZ?=
 =?utf-8?B?aHFEVGUxOUgxdTQ1TTNiclNKU1cvTlArM3JId1JTaG5YMWFWeWRaSDVONEkx?=
 =?utf-8?B?RUE2M1hjUnBiS2RGS1dCb0g4bEJ4djlSMkIwc1pYRkVMOHZFclZHeHhZZkJv?=
 =?utf-8?B?WlNOeVhjaGFTbW1heU8wUktyM3lEOGlwc1ZTemlFWm9RMFl3dmFmQmoxbG5a?=
 =?utf-8?Q?Q4g/bPEO?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5b071dfd-5302-4e13-aacf-08dbb87112ab
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB3022.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Sep 2023 17:59:58.9055
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: nZnkfKu5R/oTFDe3UWVRu5U4lZ5S3hlWGi5mSjQmDzp+Hg2mkHWbJscw0c/xc2dL2OvJtShjNZP35JH8vMOYrQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB4455
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.267,Aquarius:18.0.980,Hydra:6.0.601,FMLib:17.11.176.26
 definitions=2023-09-18_08,2023-09-18_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 adultscore=0 bulkscore=0
 suspectscore=0 phishscore=0 malwarescore=0 mlxscore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2308100000
 definitions=main-2309180159
X-Proofpoint-ORIG-GUID: cfVC2knk8YOAFMFFf3GFEy6ZBJtOw3Jf
X-Proofpoint-GUID: cfVC2knk8YOAFMFFf3GFEy6ZBJtOw3Jf
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (howler.vger.email [0.0.0.0]); Mon, 18 Sep 2023 11:01:05 -0700 (PDT)

* Peng Zhang <zhangpeng.00@bytedance.com> [230918 09:15]:
>=20
>=20
> =E5=9C=A8 2023/9/16 04:00, Liam R. Howlett =E5=86=99=E9=81=93:
> > * Peng Zhang <zhangpeng.00@bytedance.com> [230915 06:57]:
> > >=20
> > >=20
> >=20
> > ...
> >=20
> > > > > > +=C2=A0=C2=A0=C2=A0 if (unlikely(retval))
> > > > > >  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 goto ou=
t;
> > > > > >  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 mt_clear_in_rcu(vmi.mas.tree);
> > > > > > -=C2=A0=C2=A0=C2=A0 for_each_vma(old_vmi, mpnt) {
> > > > > > +=C2=A0=C2=A0=C2=A0 for_each_vma(vmi, mpnt) {
> > > > > >  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct =
file *file;
> > > > > >  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 vma_sta=
rt_write(mpnt);
> > > > > >  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (mpn=
t->vm_flags & VM_DONTCOPY) {
> > > > > >  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0 vm_stat_account(mm, mpnt->vm_flags, -vma_pages(mpnt));
> > > > > > +
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0 /*
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0 * Since the new tree is exactly the same as the old one,
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0 * we need to remove the unneeded VMAs.
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0 */
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0 mas_store(&vmi.mas, NULL);
> > > > > > +
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0 /*
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0 * Even removing an entry may require memory allocation,
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0 * and if removal fails, we use XA_ZERO_ENTRY to mark
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0 * from which VMA it failed. The case of encountering
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0 * XA_ZERO_ENTRY will be handled in exit_mmap().
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0 */
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0 if (unlikely(mas_is_err(&vmi.mas))) {
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 retval =3D xa_err(vmi.mas.node);
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 mas_reset(&vmi.mas);
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (mas_find(&vmi.mas, ULONG_MAX))
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 mas_store(&vmi.mas, =
XA_ZERO_ENTRY);
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 goto loop_out;
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0 }
> > > > > > +
> > > > >=20
> > > > > Storing NULL may need extra space as you noted, so we need to be =
careful
> > > > > what happens if we don't have that space.=C2=A0 We should have a =
testcase to
> > > > > test this scenario.
> > > > >=20
> > > > > mas_store_gfp() should be used with GFP_KERNEL.=C2=A0 The VMAs us=
e GFP_KERNEL
> > > > > in this function, see vm_area_dup().
> > > > >=20
> > > > > Don't use the exit_mmap() path to undo a failed fork.=C2=A0 You'v=
e added
> > > > > checks and complications to the exit path for all tasks in the ve=
ry
> > > > > unlikely event that we run out of memory when we hit a very unlik=
ely
> > > > > VM_DONTCOPY flag.
> > > > >=20
> > > > > I see the issue with having a portion of the tree with new VMAs t=
hat are
> > > > > accounted and a portion of the tree that has old VMAs that should=
 not be
> > > > > looked at.=C2=A0 It was clever to use the XA_ZERO_ENTRY as a stop=
 point, but
> > > > > we cannot add that complication to the exit path and then there i=
s the
> > > > > OOM race to worry about (maybe, I am not sure since this MM isn't
> > > > > active yet).
> > > > I encountered some errors after implementing the scheme you mention=
ed
> > > > below.
> >=20
> > What were the errors?  Maybe I missed something or there is another way=
.
> I found the cause of the problem and fixed it, tested the error path and
> it seems to be working fine now.
>=20
> The reason is that "free_pgd_range(tlb, addr, vma->vm_end,floor, next?
> next->vm_start: ceiling);" in free_pgtables() does not free all page
> tables due to the existence of the last false VMA. I've fixed it.
> Thanks.

Sounds good.

Please Cc the maple tree mailing (maple-tree@lists.infradead.org) list
on v3 - we are looking forward to seeing it.

Thanks,
Liam