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 09E18C71136 for ; Fri, 13 Jun 2025 18:45:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8AA226B007B; Fri, 13 Jun 2025 14:45:41 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 883206B0089; Fri, 13 Jun 2025 14:45:41 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 798D76B008A; Fri, 13 Jun 2025 14:45:41 -0400 (EDT) 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 5B4C76B007B for ; Fri, 13 Jun 2025 14:45:41 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 1324FBE879 for ; Fri, 13 Jun 2025 18:45:41 +0000 (UTC) X-FDA: 83551256082.13.0DD2F4D Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf14.hostedemail.com (Postfix) with ESMTP id 9156E100005 for ; Fri, 13 Jun 2025 18:45:38 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=N1vnt6nj; dmarc=pass (policy=quarantine) header.from=redhat.com; spf=pass (imf14.hostedemail.com: domain of peterx@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=peterx@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1749840338; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=dleQCoAtSSBdBCiR900BFEh9O2XlUfclwDFjT5jtdZ0=; b=ckGFwn0YRoEk1mVl5MXIpOKMoZZfKEkCDmTmBau9O0pAxhNk0MuAaDDcnIPSnZK87JjDK9 IZKIfhKMezaytGmXXCUq8VvFhs7O7ddwWMJIO+L5BTDqpKYjEJJXcODg5F3pISevZ53YVt eOXE+R4aylDKvvZ+pwv/39ZSuSXxiOI= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1749840338; a=rsa-sha256; cv=none; b=54Wc2bXWQMCKl2EQQ36iMrx45Qo42juZgypDdfHevkjlfJ31Y36vfkfiLFyrrR4+JbyrBI sIK3zk5rOewGqA69L54XR0IMFn76p5vZV6lZFHLBsGQbZKWDkPpavohTpBEc69AvDJDCu4 Aqlg49Wb1beU47Fpwyrt2mUoWhJtxPI= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=N1vnt6nj; dmarc=pass (policy=quarantine) header.from=redhat.com; spf=pass (imf14.hostedemail.com: domain of peterx@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=peterx@redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1749840338; 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: in-reply-to:in-reply-to:references:references; bh=dleQCoAtSSBdBCiR900BFEh9O2XlUfclwDFjT5jtdZ0=; b=N1vnt6njyR/HSEAtUSxykPuDHQvzmvRLcDOhW6gKOYqms8ra8LNhOVUKdJJO8eRcjJS4MB E7FDfk7PJdE1Moia/olqvSW7pqlkRgKCHEowolJlHL58EhKR3OrXHphb93nYdzA4AJy1hB hvCa0MVWDiotxYKpEq0m2Dh9ccwQ5Rw= Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-9-6lnSTolUM2y-xoiVIjmhbg-1; Fri, 13 Jun 2025 14:45:36 -0400 X-MC-Unique: 6lnSTolUM2y-xoiVIjmhbg-1 X-Mimecast-MFC-AGG-ID: 6lnSTolUM2y-xoiVIjmhbg_1749840336 Received: by mail-qk1-f199.google.com with SMTP id af79cd13be357-7d097fd7b32so568275885a.2 for ; Fri, 13 Jun 2025 11:45:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1749840336; x=1750445136; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=dleQCoAtSSBdBCiR900BFEh9O2XlUfclwDFjT5jtdZ0=; b=UhbNAk7hV04QUO6dCog2ohScl1DEfd/Vlm5jcjySQ3P0tj8WHKuy6uREjT6jKjAyBG JX0C728+rLE4QcgqomDg5WhBG9Wzkc7NzGMRZ95WVFVawchBHGkyJKFMo3CaQEdwTzha /IFq6oFnfOO6NOhO2GA6zIuGBQYPizWAi/zoOW7taFZxi2xJxIXRPWXzT9hfqeMez7U0 IR2TnBfRpkbuvbWn17lYxhvfFOt1fBIQ5mppsNpf6rFb3TRbxcJ0z/nTwAPuPQdb33e8 luOsjiQPiL+EOpQZ5WO9om8YA+e5dfx8wyil3pMT0lmcKpHE8e2rh6H0RcROcoZDVkSq ghyA== X-Forwarded-Encrypted: i=1; AJvYcCU2N/ZR0ngHhRx033zFm2mg4BCL0zzyM+pPu2CLBfz99XusienpHzllro/fR0N9fsFKcsjQdZ6BKQ==@kvack.org X-Gm-Message-State: AOJu0YxQXYh1SkZ65upkcZejyHfRe/BxCZlH2yjwQbVh0pr4wnM2vkvD 7saqQc/JSZTCKHeNjdpjpn3vta3aLC95DktWJUInn+nm6Heqcg0rRLo5mFAtKwRCSReKsSeTD2t h07xNFhluxbqpEhywrKdaNbJaeVZD1d6NzsFOTi5QtQfwBZh7bYBVQKCeLyf4 X-Gm-Gg: ASbGncvtr83GSpejM29J02z7dlYH3MN3sUC7izYDJt1/yD8WrUWCsS4vtu4SIZD+QJp geEbyl9P9GwWupACmGzYAex/BP9DjY3vojwVt02AYuB+fscuvLHwoOhQr4BjLQeb0GsAeI4eyVy 6OIzLOcP9eudGd3Ma1qoL/7H5uoVuBkvWC5VVw4eJ47RpS6OufSlKDIxg/oAwQ5fxrWPustvkOy pMd06qaapcfdd36fXV8vZLcPWsQaOZp+re/tsY9BRtS1wkletsm2IkaX9Vco2eFSl4GzvPJrX5D LX61EolsiWQsbA== X-Received: by 2002:a05:620a:319b:b0:7d3:8566:e9ad with SMTP id af79cd13be357-7d3c6cda22amr73140585a.34.1749840335922; Fri, 13 Jun 2025 11:45:35 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGKzDCn4nojCIkAe3r6d+k+rYFyD6aXK6Jgcdcld1OPnByWZTDIhVjfVBp6BIIZpji/HLnesg== X-Received: by 2002:a05:620a:319b:b0:7d3:8566:e9ad with SMTP id af79cd13be357-7d3c6cda22amr73135785a.34.1749840335471; Fri, 13 Jun 2025 11:45:35 -0700 (PDT) Received: from x1.local ([85.131.185.92]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7d3b8dc92e8sm206411085a.5.2025.06.13.11.45.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Jun 2025 11:45:34 -0700 (PDT) Date: Fri, 13 Jun 2025 14:45:31 -0400 From: Peter Xu To: Lorenzo Stoakes Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, kvm@vger.kernel.org, Andrew Morton , Alex Williamson , Zi Yan , Jason Gunthorpe , Alex Mastro , David Hildenbrand , Nico Pache , Baolin Wang , "Liam R. Howlett" , Ryan Roberts , Dev Jain , Barry Song Subject: Re: [PATCH 3/5] mm: Rename __thp_get_unmapped_area to mm_get_unmapped_area_aligned Message-ID: References: <20250613134111.469884-1-peterx@redhat.com> <20250613134111.469884-4-peterx@redhat.com> <08193194-3217-4c43-923e-c72cdbbd82e7@lucifer.local> MIME-Version: 1.0 In-Reply-To: <08193194-3217-4c43-923e-c72cdbbd82e7@lucifer.local> X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: ws9SHksHsjZL6E9cVyEwjeVNNcu2sI5vx1QFBPDuAx0_1749840336 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 9156E100005 X-Stat-Signature: jqwgyrd7hykp6f7x74x75fecfbrmtdue X-Rspam-User: X-HE-Tag: 1749840338-716628 X-HE-Meta: U2FsdGVkX1+GhWm0PgRmtKj5WVtu3oWivXPwkgktHLvyLcpkSX7zThsyX7taQYE27y98P7mqkImnjl/6oVGC9tDX6DW43mmkuGcwifnh2ipkn6RLd8bttsO+Dbh0QzUSnfMvuPKjIMctaWfJ0zVBaFcuPKqkr/cY7Y/7cMDFCgxhYGmQdi3d8jdJU/pDYzHqIbJGPxBHQ/ioRWsMUsL78nPxTvN8MqGsQoXYdosO4FWWZDPrTcx4in2SesvQxaywI9cijkhDu/wM+sGBXD89dhOo4fHN0WMaVNzDAbKosoAe2uJdhNp8ybTxL1GuSxNDhRmeXMwKd3zR89vrQQrgjDCfxMj1jubbr54+wawLtSNEP3FMoH44TZRWK9zDCgdepx8wZN8FbLfEZCCCu/2davAIS9mwC039tAFHRQt9Cshq0PWKsnr63o5rP07AjHizCgYlTaEYUW50awtuhdo2ktGKIVOz5nimSaewmXUsptVdIK62shc9Lh3HgW16/7WAH6AiAeZthjUD7Y3R4Afh6EuGaN94Ept5fXcOPIJJrcVfpy933vE/sdqhrWdRnjCyjp8MaFMIyB9Xc692yz+dv2EAXiVtF4yzqX3arDuhzOvt7Xx5mp5x57LsPoquDb3uIWTImu0l8pOiMpJWT6QjAO2AAvb26yKMYzjFYzrIykf9RYmWEHrX3zG67YjVjFKsKgdJ1zDLkuG/fM0C9mqYMmFI9UMmYmJFUjdJrpDBQ/r3wLN3SZYMg+Ct5zeNKVfEZh34pPB1p0+nVikOTxywSZz0wE55Sq49SrF7H/uQmC6s/rXTsnvN3rq5Jog/qcEaF1g3wXWuWcZFe/z7o/oeRJpFmY2KbcwHBP0/3iHNTw9eY4rEwv2xD9gy9IBQyTrzvyNYp5xTadtPdd0UY6yXUH3j6+3ruucC+gGvkOz+t4KdgAmqE33m7Xxo8Nt8ei3MgrOE75uOMIaCNzilhE8 09AbJz21 AyvMoNcxWfsVFFd6J9NTxt6Kg/6jtaGsK0qXa4Y7cagUqvAL3dFUqERZYtIubwaXXbSnj2ARAV+GzkbO1VVwu/80vz1dErzYWsASIWjFvncPEjM9Xy8G19KLAOtFc4e7T3xkB/mpGhKItuxe7EXnMQrmLPs9RcU1XDyZ/28EnCxUd8Ppq0eB8PKSPru1t7m4itljSHNBpmYzkzwEEQMsGC2XCtRhrfxoTtgZa9zMD+kUImC5T90rzLuEmcIlqe7eRQC/GXNKxk/0oaZLbZeQRIyVasYQoU7bHCw/ePZoCExJsxuTiQMPD6YXgRgLPn5dtG/hRs5w2xXKnLlE8co7riEiIIOnW06rv+krNfi5PANx4gtFT9OmwOdR6vzH2z6WZDEJVDoMVwWoOhHj9TtGMqvLJx5DGey2BB7cNd9V2lJ6JVW4mgPA3Os7g2FTK2CyVdPCTA43XEW3ozg+R2haBOkZ4waLz00ueXVISoXjiR2O9Y/wjF/ocB9eY/x+Q5P31HU/5wY63GRqzRM08z0X1BO88z/2an8gczELqvuWQB4CjIKpqIAc0rK0eow== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, 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 Fri, Jun 13, 2025 at 04:36:57PM +0100, Lorenzo Stoakes wrote: > On Fri, Jun 13, 2025 at 09:41:09AM -0400, Peter Xu wrote: > > This function is pretty handy for any type of VMA to provide a size-aligned > > VMA address when mmap(). Rename the function and export it. > > This isn't a great commit message, 'to provide a size-aligned VMA address when > mmap()' is super unclear - do you mean 'to provide an unmapped address that is > also aligned to the specified size'? I sincerely don't know the difference, not a native speaker here.. Suggestions welcomed, I can update to whatever both of us agree on. > > I think you should also specify your motive, renaming and exporting something > because it seems handy isn't sufficient justifiation. > > Also why would we need to export this? What modules might want to use this? I'm > generally not a huge fan of exporting things unless we strictly have to. It's one of the major reasons why I sent this together with the VFIO patches. It'll be used in VFIO patches that is in the same series. I will mention it in the commit message when repost. > > > > > About the rename: > > > > - Dropping "THP" because it doesn't really have much to do with THP > > internally. > > Well the function seems specifically tailored to the THP use. I think you'll > need to further adjust this. Actually.. it is almost exactly what I need so far. I can justify it below. > > > > > - The suffix "_aligned" imply it is a helper to generate aligned virtual > > address based on what is specified (which can be not PMD_SIZE). > > Ack this is sensible! > > > > > Cc: Zi Yan > > Cc: Baolin Wang > > Cc: Lorenzo Stoakes > > Cc: "Liam R. Howlett" > > Cc: Ryan Roberts > > Cc: Dev Jain > > Cc: Barry Song > > Signed-off-by: Peter Xu > > --- > > include/linux/huge_mm.h | 14 +++++++++++++- > > mm/huge_memory.c | 6 ++++-- > > 2 files changed, 17 insertions(+), 3 deletions(-) > > > > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > > index 2f190c90192d..706488d92bb6 100644 > > --- a/include/linux/huge_mm.h > > +++ b/include/linux/huge_mm.h > > Why are we keeping everything in huge_mm.h, huge_memory.c if this is being made > generic? > > Surely this should be moved out into mm/mmap.c no? No objections, but I suggest a separate discussion and patch submission when the original function resides in huge_memory.c. Hope it's ok for you. > > > @@ -339,7 +339,10 @@ unsigned long thp_get_unmapped_area(struct file *filp, unsigned long addr, > > unsigned long thp_get_unmapped_area_vmflags(struct file *filp, unsigned long addr, > > unsigned long len, unsigned long pgoff, unsigned long flags, > > vm_flags_t vm_flags); > > - > > +unsigned long mm_get_unmapped_area_aligned(struct file *filp, > > + unsigned long addr, unsigned long len, > > + loff_t off, unsigned long flags, unsigned long size, > > + vm_flags_t vm_flags); > > I echo Jason's comments about a kdoc and explanation of what this function does. > > > bool can_split_folio(struct folio *folio, int caller_pins, int *pextra_pins); > > int split_huge_page_to_list_to_order(struct page *page, struct list_head *list, > > unsigned int new_order); > > @@ -543,6 +546,15 @@ thp_get_unmapped_area_vmflags(struct file *filp, unsigned long addr, > > return 0; > > } > > > > +static inline unsigned long > > +mm_get_unmapped_area_aligned(struct file *filp, > > + unsigned long addr, unsigned long len, > > + loff_t off, unsigned long flags, unsigned long size, > > + vm_flags_t vm_flags) > > +{ > > + return 0; > > +} > > + > > static inline bool > > can_split_folio(struct folio *folio, int caller_pins, int *pextra_pins) > > { > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > > index 4734de1dc0ae..52f13a70562f 100644 > > --- a/mm/huge_memory.c > > +++ b/mm/huge_memory.c > > @@ -1088,7 +1088,7 @@ static inline bool is_transparent_hugepage(const struct folio *folio) > > folio_test_large_rmappable(folio); > > } > > > > -static unsigned long __thp_get_unmapped_area(struct file *filp, > > +unsigned long mm_get_unmapped_area_aligned(struct file *filp, > > unsigned long addr, unsigned long len, > > loff_t off, unsigned long flags, unsigned long size, > > vm_flags_t vm_flags) > > @@ -1132,6 +1132,7 @@ static unsigned long __thp_get_unmapped_area(struct file *filp, > > ret += off_sub; > > return ret; > > } > > +EXPORT_SYMBOL_GPL(mm_get_unmapped_area_aligned); > > I'm not convinced about exporting this... shouldn't be export only if we > explicitly have a user? > > I'd rather we didn't unless we needed to. > > > > > unsigned long thp_get_unmapped_area_vmflags(struct file *filp, unsigned long addr, > > unsigned long len, unsigned long pgoff, unsigned long flags, > > @@ -1140,7 +1141,8 @@ unsigned long thp_get_unmapped_area_vmflags(struct file *filp, unsigned long add > > unsigned long ret; > > loff_t off = (loff_t)pgoff << PAGE_SHIFT; > > > > - ret = __thp_get_unmapped_area(filp, addr, len, off, flags, PMD_SIZE, vm_flags); > > + ret = mm_get_unmapped_area_aligned(filp, addr, len, off, flags, > > + PMD_SIZE, vm_flags); > > if (ret) > > return ret; > > > > -- > > 2.49.0 > > > > So, you don't touch the original function but there's stuff there I think we > need to think about if this is generalised. > > E.g.: > > if (!IS_ENABLED(CONFIG_64BIT) || in_compat_syscall()) > return 0; > > This still valid? Yes. I want this feature (for VFIO) to not be enabled on 32bits, and not enabled with compat syscals. > > /* > * The failure might be due to length padding. The caller will retry > * without the padding. > */ > if (IS_ERR_VALUE(ret)) > return 0; > > This is assuming things the (currently single) caller will do, that is no longer > an assumption you can make, especially if exported. It's part of core function we want from a generic helper. We want to know when the va allocation, after padded, would fail due to the padding. Then the caller can decide what to do next. It needs to fail here properly. > > Actually you maybe want to abstract the whole of thp_get_unmapped_area_vmflags() > no? As this has a fallback mode? > > /* > * Do not try to align to THP boundary if allocation at the address > * hint succeeds. > */ > if (ret == addr) > return addr; This is not a fallback. This is when user specified a hint address (no matter with / without MAP_FIXED), if that address works then we should reuse that address, ignoring the alignment requirement from the driver. This is exactly the behavior VFIO needs, and this should also be the suggested behavior for whatever new drivers that would like to start using this generic helper. > > What was that about this no longer being relevant to THP? :>) > > Are all of these 'return 0' cases expected by any sensible caller? It seems like > it's a way for thp_get_unmapped_area_vmflags() to recognise when to fall back to > non-aligned? Hope above justfies everything. It's my intention to reuse everything here. If you have any concern on any of the "return 0" cases in the function being exported, please shoot, we can discuss. Thanks, -- Peter Xu