Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Date:   Wed, 11 Oct 2023 10:59:50 -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,
        mjguzik@gmail.com, mathieu.desnoyers@efficios.com,
        npiggin@gmail.com, peterz@infradead.org, oliver.sang@intel.com,
        mst@redhat.com, maple-tree@lists.infradead.org, linux-mm@kvack.org,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v4 10/10] fork: Use __mt_dup() to duplicate maple tree in
 dup_mmap()
Message-ID: <20231011145950.6ypjrfgkngukbjyr@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, mjguzik@gmail.com,
        mathieu.desnoyers@efficios.com, npiggin@gmail.com,
        peterz@infradead.org, oliver.sang@intel.com, mst@redhat.com,
        maple-tree@lists.infradead.org, linux-mm@kvack.org,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-fsdevel@vger.kernel.org
References: <20231009090320.64565-1-zhangpeng.00@bytedance.com>
 <20231009090320.64565-11-zhangpeng.00@bytedance.com>
 <20231011012849.3awzg5sfdk3sqmvo@revolver>
 <9eb93423-a2ee-4b9c-be8c-108915eb7e0f@bytedance.com>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <9eb93423-a2ee-4b9c-be8c-108915eb7e0f@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?V0Y5ZWJuNGdueDhBMHByaU9wWHc3b2JKWERsNW5uaUxYdVRnQ2REZGY1eEJh?=
 =?utf-8?B?bUNpeWttZGpxOWN2OXhUcjFhVUZuMWQ4cXk4N1U5dWsvSURzWjVvNTE5ZG1J?=
 =?utf-8?B?OE9XRFdLa0JYY3dBSUFOOEZSNkpiMFlkUm1tMzV0Vkg4QXB0NDVXclAwTm8w?=
 =?utf-8?B?V1grWGNVSmRnOWRsendBazAvZGduMHQvVGpmdGgwSDlqeTFyT2ZVbzJHOFRU?=
 =?utf-8?B?QXVXTTZLdzB5NEJYK0hWOUtYcUZkS2RTY1VKa0dqZFhIaXhUNm5TcitMRWVR?=
 =?utf-8?B?ZU1ESWVJTDd4RUFvNG4ra3BhNDVhK2dXRHlqVnp1czZzVUhxWXZiREpuNXBw?=
 =?utf-8?B?d1RpblBSby84YnA0N3BISGhhSnRSdXppc2YrQXUzcEx1MVl2K21hQ0llQjVS?=
 =?utf-8?B?bUMvYmlibXFjUUJXcWtpVVN4WDJJMmxVd2hucVR6aDNTV052aGVRSnpyZDM0?=
 =?utf-8?B?WEszRmFmN2tvWlBoaldxemZ5RHNTWThYNmNYUUloeGJCRmJkalNnZGpXQWJu?=
 =?utf-8?B?dHduMklTTDRDczNsS0lOOVY3SldyWmQwOHRvUi81Y0F1TTRxc0RNd2I1WGJn?=
 =?utf-8?B?YnBYTTFtTUo1YWNET2pNNHQwOVRIL09IZWlNczlLV2EzNVNEbEduMVNxTStJ?=
 =?utf-8?B?TlFLVGdDTitKdTVEZjQ2djN3WXpSeVdUb1N3ZGUwVUd1ZmpzTXErY2pWNXZv?=
 =?utf-8?B?d1dOQkJFbEZJUmNwaThFYXFhamZkekpZdTRGRHc2NkJ1aW9JRVVldXVkY3hV?=
 =?utf-8?B?NzI2aHpYQmdIQmpDVSt4UnB1L1NFZGcyT29lRjFzWnQ1c056Q0pLSzQ3T2lC?=
 =?utf-8?B?N3Q4TStIbDVkcmN2bHZFYmZMc3pSK0Y0RENHWnp0NTFsc29Zc2RDbGdvdmpu?=
 =?utf-8?B?Y21xOUVSOENFWVkrbm1NWlRSbHFnL2wxNE9Bb09Wd1NibHYzQlozVnRoUUtH?=
 =?utf-8?B?SlJtdko4ZGUwazdRcDJQTDg5VHdSSzZaSHBhWWozZ3RtVzhWRCtpczFHeVZ1?=
 =?utf-8?B?ZlFXemtjUFFiVW01RjREN3hjMWY0cURmcGxMWXJUVWFnK3A1dEN5cUdPZEJ4?=
 =?utf-8?B?VWZ2K3lQQ0FTM0l6Tk52S0NsdUpuOVRtWWo2TGtpWnV1Smc4UmsyMFllZGls?=
 =?utf-8?B?cTJUdEFiZzBuZGZHQmlrdm1zbEFZbExCa2I5eVFyenRYdXVUaFZ2QnN2OWVB?=
 =?utf-8?B?Zjd0a3NMMm42ZDc3ekdsbldjQVBYVUhaZXlkNmFSbGo1aGVMN1JOQkRqNlVi?=
 =?utf-8?B?RXJHQW9RU0V6djJmeDRMMmhVejFIWlp5TThWRThPb241blBDRzVwUlBRTDFh?=
 =?utf-8?B?N1BzQlVkR2o2dDFMT1hIZ1NtYUFRWjRwcWQ3LzdYNWdUNnRlM0tCNERpMjhk?=
 =?utf-8?B?TTF4Unh4YVVtckhsSHJ1M0lpVFdiZ0szODFLNWNycjZhMjB3WDFQQWN6NWVI?=
 =?utf-8?B?cllJaUR0bUpjbmo2eEU2ZFpIT0RJVE1JcVpTUUhHZUZXWUpKNXI3aC9ZL1or?=
 =?utf-8?B?T09mYXBaajNvd25NYlE4Tm1IMEhQVFhKemVXMUhWMmVZTU9ma0FpR2U1dTFk?=
 =?utf-8?B?QUszZ0IvME1vZ1N5Y1dqNEUrWXZUZ2xGM1ZuVG1vS3YwVXRSTnlGYzl3SVhs?=
 =?utf-8?B?TFkwT0R6SkgzSTBpcVFEcGhRL2kvMGVEcXJUcW9scm9wdHROSTFFakw4MUtC?=
 =?utf-8?B?ZFZmMUg4Tzh3Sk9nekJjMWJtSitRNHhnUUJCL1psR0hWRUxldmVLZTdENVM2?=
 =?utf-8?B?S0FmRytPQVcyYUFJR0k0OWY2RHI0bDZ0aitKUGpSR3Q0NyswUXAzYklIWHd4?=
 =?utf-8?B?cERYUFRGN0NxWlpTODBVakliQXlsN2JreWpFUmoxd0VSeHJWeERYVnRIc1p6?=
 =?utf-8?B?YUFENk9sV2VUbjQxS042TlVlOHlHai9TbGRsS0JEZ2FCak8zMmdEb1ozc1dR?=
 =?utf-8?B?Q3pIK0hCM1hjT0x6MmtEMTVNVnUrdUFUditxOHVEeHFlWWdjODRwQjBJVmlo?=
 =?utf-8?B?NFFEWnpUdWQ3eXJMVHJvZlBjaTV2M3pHdVJQQUthU3J0c1h0SnZUMjV1WlJP?=
 =?utf-8?B?SmNwWlhYNnQwUlFMbS9INTlqZ1NwaW9LZzNkNFBqUFZSam9WMzlGSjByTzRI?=
 =?utf-8?Q?9/HHzEV9qMiKTJaJz/tKAIxLJ?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?dEtlQTRYa0dxK2ZRYVg3bzRjUkgzMGNoNjh3akdUeGVVeHNLYjhhQzBBdlF5?=
 =?utf-8?B?NVlSdHFPcWlOdFZta0FiMzlkemZvaCs4ZmNONERsRXoxWU1PQjFvS1UzcEsw?=
 =?utf-8?B?bG53ajJKbGlEMEtPcnVEYmZXVHo0WFQrR2I1LzNZNnVoNE9WNUtLakhPWnc4?=
 =?utf-8?B?QmFycTh4dWQ4VTdCbFppWXpqaW1oMUlzd09rbWhoL0N3ZmNpK0RMUHQ0S3Yx?=
 =?utf-8?B?Ky9LZXlHWVcxQTZtaUlOWDZrc3FjY3RJNzFSTjFTMjNiNlVoZkFTNGVXdzRT?=
 =?utf-8?B?S0NuRUNWdnN6Y1hxOXI5Ti9VK1dQVlE4ZkpNUmsxemg3S1FIdjdqanRGQloy?=
 =?utf-8?B?RVJRN0dDT0xRN1BhQXpEYUFldkpmTGV2bkxpMTczYlVZYnd6M3g3YXAxYWZS?=
 =?utf-8?B?ZlM3cFFhUU9nakNwY1VzYU5pQk9zRzNmWnlIMGFUaERkOTJ5OERQbW4wK3BO?=
 =?utf-8?B?RVI0NDNiUFpTL1VjYW0yQnJXWWNtUXpxUkRiYkZFQzd2dEVjUlBSMm1reE9G?=
 =?utf-8?B?LzJnK285UlVYODlEQW1MUnBFMEgydzU2a29uS29JdlBnSEdrcE5SMHhzajdU?=
 =?utf-8?B?YU5KWW45SC9VenVvVjYxTHN1MmtBRXNWWHFuZldIU3JYQVkvRVBuR09keHAz?=
 =?utf-8?B?N3EvY05ZUTRINkNmYTBWeVVhSWQxbmltdENiNFh5cWZZdkdTQTk1TDJKcFZE?=
 =?utf-8?B?NUYxWjEzSTlkR2tlRWpGSCtjTnpOUmg0R3ZkYmhsYjhYMTZOSVdTZGJlK2Y2?=
 =?utf-8?B?UU9CNmNKMWJsbTV2cE95dVdVRTBGKzBZWUoxcmsvUFJxZEdlL3pOSkkrdlh5?=
 =?utf-8?B?UHNRVGhIWGJPU0FZd1Rvejk2dklOL0IveGtpS1hjQ01RdmNiQUZnU1NIQU85?=
 =?utf-8?B?aDluY0F0Q3hWa25nRUZxNC9TTWU0QStzS25IU2N3SHBFU0Mra0Qyb3Jxak4w?=
 =?utf-8?B?SXMyTWt6RStaSElFZ05aendheU9FQkhPaHVZeGtiZHdKU2ZyS29GWWkvK29V?=
 =?utf-8?B?UlJlajBqc2xieHZsa2E0YUs3NUFJZEFiKzlRK2FsMi9ncGVIdUtFaU4yL2M2?=
 =?utf-8?B?ODRTV1psSGtOUWlHelczRnUxS0FRSmhwamc5L09VeTBHcEhtdFV1aUMwZWFa?=
 =?utf-8?B?dk1wMW9wMUhZT1hTZSthaHU1eGZ4S245bEUvNlkycmhtUExCS0ZsTDZQUTRB?=
 =?utf-8?B?TW1tdjZYNWhXUk5Dd3F0YVZrbWRMdWwvM3NRbStuS2lndHJpR1dYM0lpWWdy?=
 =?utf-8?B?LzkxSW54ZlFZbllOaXBGRTYwU3o2dUNpNW8wb1RpRTRUL3pnd21jKzlkSDdu?=
 =?utf-8?B?aS82U2R4eFhRaFVTcUo4TjNOR0pOOG5TQzY0Q05qZjl6bmVoUnRQMzFRNkVq?=
 =?utf-8?B?N1lVUDBzQkt4NHdLZU9DdmxBcmcyV3lkaHZ3Y3g1bWQxcmc1MDRhOUlWWXlp?=
 =?utf-8?B?a010QU9QRGwwU3Z1WmRHOUltdlRJWUVNU2ZuYVJDbWE0TkFVRysvdXByV2w0?=
 =?utf-8?B?b0hnVTB2ZDQ1akVJbzlua1Roa3J3V0t0bG9wQzdxYWllOGdqbUxTd0NMS0Ew?=
 =?utf-8?Q?BPgdi6Skj2EC+8ryJ0xvi4Efc=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7b225e23-2215-453a-13e9-08dbca6aba40
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB3022.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Oct 2023 14:59:54.4905
 (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: M3102G0+7IrslwoVFbWI5k3cK+oLyy9ZGAo4O2wVfoIOjK0tLFc1hR+Uw8fcZfyCTgzIk01cp9htT8DWbF/ogg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR10MB5259
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.267,Aquarius:18.0.980,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-10-11_09,2023-10-11_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 suspectscore=0
 phishscore=0 adultscore=0 bulkscore=0 malwarescore=0 mlxscore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2309180000 definitions=main-2310110132
X-Proofpoint-GUID: uEthcw_PXY-sNNAxPhJNNHKgpSPsZ15a
X-Proofpoint-ORIG-GUID: uEthcw_PXY-sNNAxPhJNNHKgpSPsZ15a
X-Spam-Status: No, score=2.7 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email
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 (pete.vger.email [0.0.0.0]); Wed, 11 Oct 2023 08:01:10 -0700 (PDT)
X-Spam-Level: **

* Peng Zhang <zhangpeng.00@bytedance.com> [231011 03:00]:
>=20
>=20
> =E5=9C=A8 2023/10/11 09:28, Liam R. Howlett =E5=86=99=E9=81=93:
...
> >=20
> > > +	unmap_region(mm, &vmi.mas, vma, NULL, NULL, 0, tree_end, tree_end, =
true);
> > > +
> >=20
> > I really don't like having to modify unmap_region() and free_pgtables()
> > for a rare error case.  Looking into the issue, you are correct in the
> > rounding that is happening in free_pgd_range() and this alignment to
> > avoid "unnecessary work" is causing us issues.  However, if we open cod=
e
> > it a lot like what exit_mmap() does, we can avoid changing these
> > functions:
> >=20
> > +       lru_add_drain();
> > +       tlb_gather_mmu(&tlb, mm);
> > +       update_hiwater_rss(mm);
> > +       unmap_vmas(&tlb, &vmi.mas, vma, 0, tree_end, tree_end, true);
> > +       vma_iter_set(&vmi, vma->vm_end);
> > +       free_pgtables(&tlb, &vmi.mas, vma, FIRST_USER_ADDRESS, vma_end-=
>vm_start,
> > +                     true);
> > +       free_pgd_range(&tlb, vma->vm_start, vma_end->vm_start,
> > +                      FIRST_USER_ADDRESS, USER_PGTABLES_CEILING);
> I think both approaches are valid. If you feel that this method is better=
,
> I can make the necessary changes accordingly. However, take a look at the
> following code:
>=20
> if (is_vm_hugetlb_page(vma)) {
> 	hugetlb_free_pgd_range(tlb, addr, vma->vm_end,
> 		floor, next ? next->vm_start : ceiling);
> }
>=20
> In free_pgtables(), there is also a possibility of using
> hugetlb_free_pgd_range() to free the page tables. By adding an
> additional call to free_pgd_range() instead of hugetlb_free_pgd_range(),
> I'm not sure if it would cause any potential issues.

Okay.  It is safe for the general case, but I've no idea about powerpc
and other variants.  After looking at the ppc stuff, I don't think it's
safe (for our sanity) to proceed with my plan.

I think we go back to your v2 attempt at this and store XA_ZERO, then
modify unmap_vmas(), free_pgtables(), and the (already done in v2) exit
path loop.  Then we just let the normal failure path be taken in
exit_mmap().  Sorry for going back on this, but there's no tidy way to
proceed.


From your v2 [1]:
+			if (unlikely(mas_is_err(&vmi.mas))) {
+				retval =3D xa_err(vmi.mas.node);
+				mas_reset(&vmi.mas);
+				if (mas_find(&vmi.mas, ULONG_MAX))
+					mas_store(&vmi.mas, XA_ZERO_ENTRY);
+				goto loop_out;
+			}

You can do this instead:
+			if (unlikely(mas_is_err(&vmi.mas))) {
+				retval =3D xa_err(vmi.mas.node);
+				mas_set_range(&vim.mas, mntp->vm_start,
mntp->vm_end -1);
+				mas_store(&vmi.mas, XA_ZERO_ENTRY);
+				goto loop_out;
+			}

We'll have to be careful that the first VMA isn't XA_ZERO in the two
functions as well, but I think it will be better than having 7 arguments
to the free_pgtables() with the last two being the same for all but one
case, and/or our own clean up for exit.  Even with a wrapping function,
this is too messy.

[1]. https://lore.kernel.org/lkml/20230830125654.21257-7-zhangpeng.00@byted=
ance.com/

Thanks,
Liam