Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Date:   Fri, 22 Sep 2023 09:44:08 -0700
From:   Mike Kravetz <mike.kravetz@oracle.com>
To:     Rik van Riel <riel@surriel.com>
Cc:     linux-kernel@vger.kernel.org, kernel-team@meta.com,
        linux-mm@kvack.org, akpm@linux-foundation.org,
        muchun.song@linux.dev, leit@meta.com
Subject: Re: [PATCH 1/2] hugetlbfs: extend hugetlb_vma_lock to private VMAs
Message-ID: <20230922164408.GA224968@monkey>
References: <20230920021811.3095089-1-riel@surriel.com>
 <20230920021811.3095089-2-riel@surriel.com>
 <20230921224201.GB21193@monkey>
 <c3309a3486df6ad2901d4d5c23e7bc6f4b8a9a24.camel@surriel.com>
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <c3309a3486df6ad2901d4d5c23e7bc6f4b8a9a24.camel@surriel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?xhwv6TFCZPvKmT/IQsGesMHdUkYQsIVCPQze221DfWIOhIOjZh5U4bEB9S?=
 =?iso-8859-1?Q?fNRbVX/dkg9ilbUatKYN8+MLQWs8/grsl4o+dTrOiYj056TMod5CGpWVT4?=
 =?iso-8859-1?Q?VcVMWuGovJgKccbFpokRM6vQ4cg7a/ZU7ECT9sd1GE0DOSI93Vd21c1RcF?=
 =?iso-8859-1?Q?MdvkA/KN5F8wsY/vAGqe/hWPYulAF5dlFNa9ZBEXFACZXYmUI+EY+vKTMT?=
 =?iso-8859-1?Q?Q+LwmwKbmRzLLCRKhmgv1zGm5c//zeTL2u+j1m4oAmOLxJY8AJhVq7Cqi4?=
 =?iso-8859-1?Q?HepVw0UZJ1YDCv7M38fTtpzE4SuyANwk0v4gLQ+NDCwM1XoHr6GsEQzS/k?=
 =?iso-8859-1?Q?hd57nyDjLd1b0E+PwbHOey2DBGW344m7qFCr0NmwM852iv9KQ5sypAW7Zc?=
 =?iso-8859-1?Q?Mf2tvOZlqHZipXHLmyT09fw2qYTNOK4/Y11cme7ppAJacBzQsEI0VHIdSB?=
 =?iso-8859-1?Q?L5Vdx+7RASVTTaITqACrj5PjphWZEpCBvwSzsf1mnSrUwo2my+WecFrEqm?=
 =?iso-8859-1?Q?9DLPE6Oh420SsxV5/B6CoksdwAt1di7MPbmjxehZQjMKtY6bBLHbZxIIq/?=
 =?iso-8859-1?Q?Qb5ZqrrUAkmQ+RBj+T5rc0QA3M4Z2sAWHwwOx3SqR84tGIb7+qHLLYcQRc?=
 =?iso-8859-1?Q?ypeokIbbvBr56h4l7LbaPMV6V6Uebq/1QEpq2QodG927YvzrWyqwhupVG/?=
 =?iso-8859-1?Q?MPkf/gUboOS8KCuPGSyjN1ucyJnNgYAFKhoSB0FlBTWn1ndV2Jy+YoPShh?=
 =?iso-8859-1?Q?1JlNV2pEbOrSRSSYZ59+V2sG7H+xCFG8b4H4Y/Oyhk/K24sSzBIULVUH12?=
 =?iso-8859-1?Q?dmLGE/ayafagHLYuSYf9TPaWN/veyQWPe3WK2GNe0ZMwMdgj3WfBqCsaNv?=
 =?iso-8859-1?Q?BpUZLr+uAYPpohferjnfgCbn/TvjJl3N11bSS5x9+jWdQWjKjdgEZkmtci?=
 =?iso-8859-1?Q?fiIf4XfRWWyrHdAUd2znIo9Ksxw/KArcU5nBezge+6srOUYYXoPBL96tOG?=
 =?iso-8859-1?Q?9PPWFF9zwcsQh47iQmGkeZF7tMIyxGLWAtgGa67C/FRvHmqbdv/DsgQinp?=
 =?iso-8859-1?Q?H57HCW6LprNXHmtFaxINPoJxqsxGk2TUKP9HbC2EDXfprohmLZ+t3BYW1g?=
 =?iso-8859-1?Q?bPvgpYt5HTKNbrbhpOjlFUyQ/LrXj9BEKOyfLBqbqcM/PZJvpEuuEVble9?=
 =?iso-8859-1?Q?Raxf6txXmX80j7JahbcRtu0onRuxZ7k1mfHrme69TManRs4v2qDM4RYxq6?=
 =?iso-8859-1?Q?LI7zdiH2INwv4a9GiKTUIBn6hOjI+WOVqkpz7BVI710uwiQbycEZpp9O1Q?=
 =?iso-8859-1?Q?QBCwqnoV0jiIai622ed71xF29/AKQ8ArlLHeJLy6tTE20xjW0/fSxZFk6R?=
 =?iso-8859-1?Q?Sfiu4g+Ti06YVB1CnAAqLYk6f7k5nxoGQ0XYXsAAFEMUqfRF3PtVOY/tJq?=
 =?iso-8859-1?Q?qRSLDxUQG6IfmXOzWmualrjWbZGSUa7BChRobmv0FeGODSuDDMGnJp8ayS?=
 =?iso-8859-1?Q?bfZby2qbb9wrvPPW/CyxO59O7w6Bqzsw6tSXVYyEH4E0kVQvDkDLY98F0I?=
 =?iso-8859-1?Q?gnkleIpd7fw2+YpMk+itl4OM/OghMpnKNUYBIlprd8c6kOBF+D/Uw6vvtY?=
 =?iso-8859-1?Q?Uk0RNGS8IqbkuYOyG40leCknx4efNK1y2+?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: MoZXDiUzMDGTwI2Qta82GGcCGkyrNvSp6EGnl3tbNH0klcRC9e0+DVa4b31Ve9bkRSBr9ODRY/x0dZgGVhUwfLtnzJhpEydwBzsjdNf1IWuvGo/AA7Q1Iro2Y9+l1PTSkULnave9LKUUnGduYWWgx0bwimi6JadBVrVuIwg3npNJZyNORCd7bD9ZrCc+r54+JzMh/sxrUoyMd9COJtLDlZrI2zl/a06vXijd9+EBk3WIFrFh/hyZ7tccUTUbtj4DXwQPfiMDsPIUpTFZUyQBFpeKx2SIxsyKo/q5ZzWrAHKfLLRScZ7VhgsUPzJWpPcRf6GbL4ioNiHM2VvYqM42D96BOCPalEAxE9bK2WTAYaTa2QpqAajtl8FiXx2pbDIgkhwQDGQG+/waq0dydralI0iTI5UPbkjtMZTFe2cscn4osOBFvzpaNStf1hYu6JzMhLCez/bKkO5MujUIfUXD4LGJBqNlHPiFUrUp6IonJl7pM6iFgqpPi1wJMDd/5RpSUS08v0GbxA6xw4IGb1C3v/SnAM80i5F0JBjFFX9EnUwU/b6KUxogYq7rmA9FifWFZrsvgbSwB1pGTRejG++DwhCrRFoHNzeuS56pN2MuNybBtc/jYt7F60yMmjwgmFBjnxDZNRmxgUoEJpYfbcllhc5owWN31+Ok+e3pAaQ6R/iaqT6hzh06xABFJJRvcfq//iRM3uTdqhJp5DuZglKBuQDI+e+TTQMB+JDJpdfURwVKYkrrolVIcFj0ds2x4Hox+9OfJ1YhPCSezOG8FoIGf12k76SAFxmZoBJl+j5+IZz4GOg1Wj1X8lbxoXRNNK7vhhOdq4VeurfNVswCfSGlVWm7fTkFD2KezKjyvKG8pbMH0v/mHftUwcUu7l/Ko+Vh+Vqd9m9eddreKt19ar70WfOYrmFe0Xwe63i1tBVBr84=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: eb064df0-8cc2-42f4-74d4-08dbbb8b25f6
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Sep 2023 16:44:11.6767
 (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: ZqBnY9R4kibX7Q3AeQh4+Wyy4RXdMIzWtsd7CBlJgWUUO/J9x934igRsJLpoKMYdQGUY2QwBLOQ+6lrzBtcj7Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW5PR10MB5876
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-09-22_15,2023-09-21_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 malwarescore=0 spamscore=0
 suspectscore=0 mlxlogscore=999 phishscore=0 mlxscore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2309180000
 definitions=main-2309220144
X-Proofpoint-GUID: hdug_rOdSBwTvQU1HXu-EvV-n5TLPY7c
X-Proofpoint-ORIG-GUID: hdug_rOdSBwTvQU1HXu-EvV-n5TLPY7c
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_MED,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	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]); Fri, 22 Sep 2023 09:44:55 -0700 (PDT)

On 09/22/23 10:37, Rik van Riel wrote:
> On Thu, 2023-09-21 at 15:42 -0700, Mike Kravetz wrote:
> > On 09/19/23 22:16, riel@surriel.com?wrote:
> > > From: Rik van Riel <riel@surriel.com>
> > > 
> > > Extend the locking scheme used to protect shared hugetlb mappings
> > > from truncate vs page fault races, in order to protect private
> > > hugetlb mappings (with resv_map) against MADV_DONTNEED.
> > > 
> > > Add a read-write semaphore to the resv_map data structure, and
> > > use that from the hugetlb_vma_(un)lock_* functions, in preparation
> > > for closing the race between MADV_DONTNEED and page faults.
> > > 
> > > Signed-off-by: Rik van Riel <riel@surriel.com>
> > > ---
> > > ?include/linux/hugetlb.h |? 6 ++++++
> > > ?mm/hugetlb.c??????????? | 36 ++++++++++++++++++++++++++++++++----
> > > ?2 files changed, 38 insertions(+), 4 deletions(-)
> > 
> > This looks straight forward.
> > 
> > However, I ran just this patch through libhugetlbfs test suite and it
> > hung on
> > misaligned_offset (2M: 32).
> > https://github.com/libhugetlbfs/libhugetlbfs/blob/master/tests/misaligned_offset.c
> 
> 
> Speaking of "looks straightforward", how do I compile the
> libhugetlbfs code?
> 
> The __morecore variable, which is pointed at either the
> THP or hugetlbfs morecore function, does not seem to be
> defined anywhere in the sources.
> 
> Do I need to run some magic script (didn't find it) to
> get a special header file set up before I can build
> libhugetlbfs?

libhugetlbfs is a mess!  Distros have dropped it.  However, I still find
the test cases useful.  I have a special VM with an old glibc just for
running the tests.

Sorry, can't give instructions for using tests on a recent glibc.

But, back to this patch ...
With the hints from the locking debug code, it came to me on my walk this
morning.  We need to also have __hugetlb_vma_unlock_write_free() work
for private vmas as called from __unmap_hugepage_range_final.  This
additional change (or something like it) is required in this patch.

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index f906c5fa4d09..8f3d5895fffc 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -372,6 +372,11 @@ static void __hugetlb_vma_unlock_write_free(struct vm_area_struct *vma)
 		struct hugetlb_vma_lock *vma_lock = vma->vm_private_data;
 
 		__hugetlb_vma_unlock_write_put(vma_lock);
+	} else if (__vma_private_lock(vma)) {
+		struct resv_map *resv_map = vma_resv_map(vma);
+
+		/* no free for anon vmas, but still need to unlock */
+		up_write(&resv_map->rw_sema);
 	}
 }
 

-- 
Mike Kravetz