From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3606626D4E5 for ; Thu, 5 Mar 2026 12:09:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772712586; cv=none; b=j5fjouowNTgWn0esJoNdFtig9e/VtoLQ2y+9s6YRC5TC5wgjuaA7dsvpwkCPr3UCd5OnwOAeYhs8YWmVJ+VzLDj3r7eBjgMKH7qInEVmtqRUZDwuVwMc0qrgQMdj9y61C/pARCPtCnldBRnD+Veh9D9lFSQlewWzqHi7x66EmeE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772712586; c=relaxed/simple; bh=DPZxp9d6aNr21sqGSxSxS6G6iuOVirqb2fJMZwF7CHs=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=ZN4lTQxp6NwpBxtFBt5NryyhZhghhJ4NS30FwFzDuGqNuNIkyl/JHlfQUlQ3HwrHDHdkgM3mxxuFtMS4kamhllp0ZjtQQw/3Ek89x4NmmmFKPVXNLhK6P0ngb3qKDeKFu5ySM3UwKSEm7BxTswLRmIL13qBsxjVvtApf5r+4HbU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=f3nNh68F; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=e10qgQFD; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="f3nNh68F"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="e10qgQFD" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1772712584; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=T5cwVERvcN024z3zP/cYTnGpIfAQ3OHPG7ozXhuqUS4=; b=f3nNh68FVMfUmCapngsoj3eWkh/bSyt6b5kVfaMNuEfHPI5/t4rzElZ14r7bOUIZz8Azvl G1UnnqvqIvOUoAKUkRWSpRKTDoHCECXxOUqpSX8vo+GdAYQxaJ14iONHuZE4a+Ejv9Etpn kwrmTjz4J0/1j/bik8LdID1uLVK7bw8= Received: from mail-lj1-f200.google.com (mail-lj1-f200.google.com [209.85.208.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-385-imUWaHF2NxG4HZltKohWsg-1; Thu, 05 Mar 2026 07:09:43 -0500 X-MC-Unique: imUWaHF2NxG4HZltKohWsg-1 X-Mimecast-MFC-AGG-ID: imUWaHF2NxG4HZltKohWsg_1772712582 Received: by mail-lj1-f200.google.com with SMTP id 38308e7fff4ca-389e78065b2so84928761fa.3 for ; Thu, 05 Mar 2026 04:09:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1772712581; x=1773317381; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=T5cwVERvcN024z3zP/cYTnGpIfAQ3OHPG7ozXhuqUS4=; b=e10qgQFD7APdYy3t9sdIRt19ILMZN6S2RohCJ7MSb2MeIMCfx6vsDINzmYtm4Gi9ix /C1YYQ7BKDMbiEAW+D59zCqmONyQ6JTC9wEIF0CtDMsWb4Yc+PgF4CcIPlHoWyr7bkbe RyxX496sMtfBwIGJjf4JNKaad/xkj6IwtQ9Ul+0Li+H21IqDRsKQwANdydBHtXjg/t6e cgIuvG6ytP29vnVmk4UrBlWpJ4madf8ieIQDuNhelbYuwtj5wN7yo1E8cPiQ8X16rbir iPVtF1fesywRI5RNAQiEqeqbwIS9Z+jQgFU8QXnEPahfPMjrShdI8eYIuAcNNBJJ/w+p fNTg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1772712581; x=1773317381; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=T5cwVERvcN024z3zP/cYTnGpIfAQ3OHPG7ozXhuqUS4=; b=NwVJ/jutuYqbMGINdL0DNinV106b1UFaY/vciW+wsdXrqTP6KYPfeaWmwndTrjYXY8 xfmK+phBP4IIzxUUsDkOdTuEWuX4u5c+RExQ4Z0pWPe3zwPcTJmuq9Mo1ZA/mN8L/CHM z0bNA87ISn1awnYy+q3XGxN2YPJY12HGVNOOLrV4H7P5wXd4Z9HgUtR/G2EogAkUSt6O +LJk14rez1+fhjSy1Yw/xymNGHz2405pYNutI+fCYPwZtKEN9ih2D17uymTsTxyXNSUK LWefWFZZzs4J3PGvxWqkeduM3XqYFGNB1cNIPYhzQfa8S3YVsXg6ogcoehMsB/hk2w/E am6w== X-Forwarded-Encrypted: i=1; AJvYcCVzl7ckldwsZDF0FyQEVENAIuDoyMQ8mlPH/b83kJepLw3hPJ6WF1yb7ooBSO89n+ITGThFbPzWLKvAqA4=@vger.kernel.org X-Gm-Message-State: AOJu0YwCY3NRKtLWNbN4ZBMgU0BFkLdzqUuXxUjvDjMWFy4r9hZ3qCWX GpKV0b3YJOuwnYPzVvLLsInl7e0N/iGKPgXuDEuoHYt6C7LwlLwx6OIRhx3k3C01rfEch0Uzm/x ucRMxyv18Pnsf/Uv3IQ4VUHHUVzF74p2wZrSv6ImJDMbjf+nOpwjv0VX0Pq4L8LwZ X-Gm-Gg: ATEYQzw6KrulPji1FKWOhqQFcKiVgG1ioI0GfJtjkKq0uanXtkCTR7E5fNmfsAKiA9w VDJs76v67HkZJ5DX/3oAVwdgkIJ8SOtNY03iC8o1dS+VjXym16ezcjh8EpyowMRLTXcxD8GaLNE iy4f8B54ed0tGRLDxgvGVKhLuYFTes5+7qtzmls+1AwybZMg2/lvxY7y5SUlL4FclZnCKH9OI59 laa/6E7jM+Vn7Ywenr+uiroRRTX4OdGEyIvjf9mTtgN0QLKOu91SSrTiBK6ozt89M2vEtfut5vv 5IgEdxGmUpO/oSNjCCPdp0I0RNaGyC9S+V7w73+9THFTVw2vKw07WlLJ7WCJS4oVce1iaKUeu/T 0PcNIcuZWFiZc3N9dB6htXbZtDAc9Y7wSBqc05Gkr+JhFHnc= X-Received: by 2002:a05:6512:ac1:b0:5a1:187d:fefb with SMTP id 2adb3069b0e04-5a12c28895cmr852552e87.19.1772712581483; Thu, 05 Mar 2026 04:09:41 -0800 (PST) X-Received: by 2002:a05:6512:ac1:b0:5a1:187d:fefb with SMTP id 2adb3069b0e04-5a12c28895cmr852525e87.19.1772712580678; Thu, 05 Mar 2026 04:09:40 -0800 (PST) Received: from [192.168.1.86] (85-23-51-1.bb.dnainternet.fi. [85.23.51.1]) by smtp.gmail.com with ESMTPSA id 2adb3069b0e04-5a12356ffdasm1945705e87.76.2026.03.05.04.09.39 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 05 Mar 2026 04:09:40 -0800 (PST) Message-ID: <80683d6d-ea38-4326-af5e-e4c173bb1930@redhat.com> Date: Thu, 5 Mar 2026 14:09:39 +0200 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] mm/migrate_device: fix folio refcount leak on folio_split_unmapped failure To: Usama Arif , Balbir Singh , Zi Yan , Kiryl Shutsemau , matthew.brost@intel.com, npache@redhat.com, david@kernel.org Cc: Usama Arif , Andrew Morton , linux-mm@kvack.org, joshua.hahnjy@gmail.com, hannes@cmpxchg.org, rakie.kim@sk.com, byungchul@sk.com, gourry@gourry.net, ying.huang@linux.alibaba.com, apopple@nvidia.com, riel@surriel.com, shakeel.butt@linux.dev, linux-kernel@vger.kernel.org, kernel-team@meta.com References: <20260304120132.3973445-1-usamaarif642@gmail.com> <5e59c077-9f06-4e45-86e1-ca696e6105b4@nvidia.com> <622eb392-8c04-473d-b42a-ecdc489799c4@linux.dev> <942f2df4-6fb5-415f-b7d4-87a83315890b@redhat.com> Content-Language: en-US From: =?UTF-8?Q?Mika_Penttil=C3=A4?= In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 3/5/26 13:44, Usama Arif wrote: > > On 05/03/2026 06:09, Mika Penttilä wrote: >> Hi! >> >> On 3/5/26 01:28, Usama Arif wrote: >> >>> On 04/03/2026 22:09, Balbir Singh wrote: >>>> On 3/5/26 08:54, Zi Yan wrote: >>>>> On 4 Mar 2026, at 16:48, Balbir Singh wrote: >>>>> >>>>>> On 3/5/26 02:17, Zi Yan wrote: >>>>>>> On 4 Mar 2026, at 7:01, Usama Arif wrote: >>>>>>> >>>>>>>> From: Usama Arif >>>>>>>> >>>>>>>> migrate_vma_split_unmapped_folio() takes an extra reference via >>>>>>>> folio_get() before calling folio_split_unmapped(). On success, the >>>>>>>> split consumes this reference: __folio_freeze_and_split_unmapped() >>>>>>>> expects the +1 in its folio_ref_freeze() check, and distributes it >>>>>>>> across the resulting sub-folios via folio_ref_unfreeze(...+1), which >>>>>>>> are later balanced by folio_put() calls in __migrate_device_finalize(). >>>>>>>> >>>>>>>> If folio_split_unmapped() fails (e.g., unexpected pinning returns >>>>>>>> -EAGAIN), the function returns without calling folio_put(). The extra >>>>>>>> reference is never released. >>>>>>>> >>>>>>>> Add the missing folio_put() on the error path. >>>>>>>> >>>>>>>> Fixes: 4265d67e405a4 ("mm/migrate_device: add THP splitting during migration") >>>>>>>> Closes: https://lore.kernel.org/all/CAA1CXcDyqPPwf_-W7B+PFQtL8HdoJGCEqVsVxq7DhOUB=L4PQA@mail.gmail.com/ >>>>>>>> Reported-by: Nico Pache >>>>>>>> Signed-off-by: Usama Arif >>>>>>>> --- >>>>>>>> mm/migrate_device.c | 4 +++- >>>>>>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c >>>>>>>> index 0a8b31939640f..351ecd9065d13 100644 >>>>>>>> --- a/mm/migrate_device.c >>>>>>>> +++ b/mm/migrate_device.c >>>>>>>> @@ -917,8 +917,10 @@ static int migrate_vma_split_unmapped_folio(struct migrate_vma *migrate, >>>>>>>> folio_get(folio); >>>>>>>> split_huge_pmd_address(migrate->vma, addr, true); >>>>>>>> ret = folio_split_unmapped(folio, 0); >>>>>>>> - if (ret) >>>>>>>> + if (ret) { >>>>>>>> + folio_put(folio); >>>>>>>> return ret; >>>>>>>> + } >>>>>>>> migrate->src[idx] &= ~MIGRATE_PFN_COMPOUND; >>>>>>>> flags = migrate->src[idx] & ((1UL << MIGRATE_PFN_SHIFT) - 1); >>>>>>>> pfn = migrate->src[idx] >> MIGRATE_PFN_SHIFT; >>>>>>>> -- >>>>>>>> 2.47.3 >>>>>>> Add Balbir, who wrote the code, to comment on this. >>>>>>> >>>>>> Thanks Zi! >>>>>> >>>>>> Just wondering if there is a reproducer for the issue and how the fix was tested? >>>>>> I expect migrate_vma_finalize() to be called for folios, even when split failed and >>>>>> drop the lock. >>>>> Does migrate_vma_finalize() do folio_put() for failed-to-split folios? >>>>> If so, how does it distinguish between split folios and failed-to-split folios? >>>>> By comparing source and destination folio orders? >>>>> >>>> We reset the MIGRATE_PFN_MIGRATE flag for failing to migrate pfns. We do a folio_put >>>> on the src in finalize, if it is split then on all the split folios as well. >>>> >>>>> What we see from migrate_vma_split_unmapped_folio() is that >>>>> it adds a refcount for all input folios, but only drops a refcount >>>>> for the split folio. Isn’t it cause failed-to-split folios to have >>>>> additional refcount? >>>>> >>> Hello! >>> >>> Thanks for reviewing everyone. So its very difficult to create a reproducer I think >>> the extra reference would need to appear after migrate_device_unmap() but before >>> folio_split_unmapped() in migrate_vma_pages()? That's hard to trigger reliably from >>> userspace. >>> >>> The fix came about when Nico indicated there might be an issue if split_huge_pmd_address >>> fails in my patch [1]. >>> >>> Below is my understanding of how refcounting is working over here step by step. I >>> might very well be wrong on this, and the refcounting is a bit all over the place >>> and I might miss a reference change somewhere so would really appreciate if someone >>> can confirm this! >>> >>> >>> 1. migrate_vma_collect_huge_pmd(): >>> a) folio_get(folio) -> +1 (collect reference) >>> 2. migrate_device_unmap(): >>> a) folio_isolate_lru() -> +1 (isolation reference) >>> b) folio_put() -> -1 (drops the collect reference) >>> >>> >>> Without this patch fix: >>> >>> 3. migrate_vma_split_unmapped_folio(): >>> a) folio_get(folio) -> +1 (split reference) >>> b) folio_split_unmapped() -> fails >>> c) Returns error — without folio_put() which is the fix >>> 4. Caller in migrate_vma_pages(): clears MIGRATE_PFN_MIGRATE | MIGRATE_PFN_COMPOUND >>> 5. __migrate_device_finalize(): sees !(src_pfns[i] & MIGRATE_PFN_MIGRATE), restores the folio: >>> a) remove_migration_ptes(src, src) — re-establishes user PTEs >>> b) folio_unlock(src) >>> c) folio_put(src) -> -1 (drops the isolation reference) >>> >>> The split reference in 3.a is never released and the folio has a permanently elevated refcount. >>> Unless I missed a folio_put somewhere for the refcount increase in folio_isolate_lru() (2.b)? >>> >>> Please let me know if this makes sense! >>> >>> [1] https://lore.kernel.org/all/CAA1CXcDyqPPwf_-W7B+PFQtL8HdoJGCEqVsVxq7DhOUB=L4PQA@mail.gmail.com/ >>> >>>> Thanks! Yes, the patch makes sense >>>> >>>> Acked-by: Balbir Singh >>>> >>>> Balbir >> I remember stumbling on this while ago also. The folio_get() in migrate_vma_split_unmapped_folio() >> is balanced with put_page() in __split_huge_pmd_locked() (freeze = true), can't fail for device pages. >> Folios at this point are unmapped but have 1 refcount from "collecting". >> After folio_split_unmapped() the refcount(s) is still 1. >> >> So it seems the code is good as is? A comment though would be good for the extra folio_get.. >> > hmm I dont think the put_page() in __split_huge_pmd_locked() is there to balance the folio_get() in > migrate_vma_split_unmapped_folio(). There are other points where split_huge_pmd_locked() is called > with freeze = true [1] and they don't get a reference before calling split_huge_pmd. > > I think the folio_put() in __split_huge_pmd_locked() freeze = true case is there as migration > entries are being installed? > > [1] https://elixir.bootlin.com/linux/v6.19.3/source/mm/rmap.c#L2334 > > Yes normally you want to drop the reference when installing migration entries but in this context you have already done the collecting for the THP folio and you want to balance with the folio_get() the put_page() to keep the refs unchanged. Is that right Balbir? --Mika