From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 1EA252F8E81 for ; Mon, 11 May 2026 16:21:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778516480; cv=none; b=H/7c1O3mVb3U2zzB2f2IoBVCmBXOqzkgmZHaMJ+xDU8Oxyyh31fIvyX/aVm3oxp/0Yvj8uRmRW4In/uAhirDq9SlXJHZAGAS+ozqpHUZE4Go2Xyj+Y4kdOK58lBO6walfcQ1668Ah/xI4gv8LYvsUtDUAzntApW1uKbgJ63ZvZw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778516480; c=relaxed/simple; bh=ek5yZ3rdPQXD+YgBaHFA9FsfHhCxUgeyqnfga+3gvmU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=GaDCWdOksTRBnPLp+cWmOlsj+LdVJ3iJUY3v/7W+56gRIb/y4NhihsUS5s4pKcyjBVuXpcPIbTRR5Nyi/gPq+WDEk1U5btuOhoJyv4WVOoqJV5pUZ44IwsyWdHXLpZo0SskpZcw3zgSiyRMUOssx80NqY7TYJTwWkY2b+8+wOcQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=fqyEPxGf; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="fqyEPxGf" Received: by smtp.kernel.org (Postfix) with ESMTPSA id C3C3AC2BCB0; Mon, 11 May 2026 16:21:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1778516479; bh=ek5yZ3rdPQXD+YgBaHFA9FsfHhCxUgeyqnfga+3gvmU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=fqyEPxGfgb7qeT1w+ddbKI7pMfjx8+f5I9DLHxyteXx9gRDOwpoj0q+jzKy+8Bie/ rVG68ZqWMXY6jyxRwrvVQNeUdhjp/mL4EenVuE3vrK0yWQVTVMn0Jc735GqIi/6RFB B4sxeDnDfdvzij4ddl89gLvWSeMW5oa7X3Tn2m70huFYt571XEYbeLcCG06kxJarpE MGRzDWMUWUlqv2IR/KSQfHwTWzRQ7AIqtaQ3DhJ6GeHOcZwlc1SyN+rJcDA38OuyGY meOA8xICIj+fVnc/LBecBoMAVFcF7sBFntPY55qlFrvkkwK5xc6SQy2k9wtMeYwHcX 104nYh3gNciaQ== Date: Mon, 11 May 2026 17:21:14 +0100 From: Lorenzo Stoakes To: "David Hildenbrand (Arm)" Cc: Wandun , linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org, ziy@nvidia.com, baolin.wang@linux.alibaba.com, liam@infradead.org, npache@redhat.com, ryan.roberts@arm.com, dev.jain@arm.com, baohua@kernel.org, lance.yang@linux.dev Subject: Re: [PATCH v2] mm/khugepaged: avoid underflow in madvise_collapse for sub-PMD MADV_COLLAPSE Message-ID: References: <20260511065701.799006-1-chenwandun@lixiang.com> <4e3d0c1b-af33-470d-acb0-1e6540ba312a@gmail.com> <0a860f74-b2fc-4ea7-9f13-1879c2c8c168@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <0a860f74-b2fc-4ea7-9f13-1879c2c8c168@kernel.org> On Mon, May 11, 2026 at 10:01:39AM +0200, David Hildenbrand (Arm) wrote: > On 5/11/26 09:35, Wandun wrote: > > > > > > On 5/11/26 15:17, David Hildenbrand (Arm) wrote: > >> On 5/11/26 08:57, Wandun Chen wrote: > >>> From: Chen Wandun > >>> > >>> madvise_collapse() computes the THP-aligned window: > >>> > >>>      hstart = ALIGN(start, HPAGE_PMD_SIZE);     /* round up */ > >>>      hend = ALIGN_DOWN(end, HPAGE_PMD_SIZE);    /* round down */ > >>> > >>> The following case will cause hstart > hend, and result in underflow > >>> in the return statement, avoid it by returning -EINVAL early when > >>> hstart > hend. > >>> > >>>      madvise(PMD-aligned + PAGE_SIZE, PAGE_SIZE, MADV_COLLAPSE); > >> Ok, so providing a PMD-aligned address as start will result in 0 and a > >> non-aligned address will result in -EINVAL. > >> > >> Didn't Lorenzo agree that just returning 0 in both cases would be clearer? But I > >> might have misunderstood it. > > Lorenzo suggested retuern -EINVAL for both case at the beginning, > > Later, Lorenzo add an correction, suggested should return 0 for > > compatibilty reasons for hstart == hend case. > > (If I haven't missed any information) > > Let's wait for Lorenzo's confirmation. :) thanks. See below but TL;DR I convinced myself that actually, I agree with David... I hadn't really examined the madvise() <-> madvise_collapse() logic closely enough but yeah. Return 0 for both cases. > > I think the important part is that we cannot have a situation where start < end > (given that madvise() consumes a length). Because, there we really should have > returned -EINVAL. > > For start <= end, if there is nothing suitable to collapse, I'd say we'd just > consistently return 0. Right so madvise_vma_behavior() should be called with range->[start, end] tied to the VMA (under VMA lock we assert this also). So what we're really talking about is hstart, hend. Really we should NEVER have aligned the addresses for the user, that was the real mistake here. But that ship has sailed... Since we do: hstart = ALIGN(start, HPAGE_PMD_SIZE); hend = ALIGN_DOWN(end, HPAGE_PMD_SIZE); That means e.g. start = + 1 end = start + Results in hstart > hend, so the user must have given incorrect input. hstart == hend would be e.g.: start = + 1 end = start + HPAGE_PMD_SIZE Which is still an invalid input. I honestly wish we just required that the user provided PMD aligned ranges and we didn't align for them, it's stupid that we do. So the real question is, what constitutes an actual invalid input here? We've made a mess and now we have to decide how we interpret it... :) I still feel that hstart > hend is an error. Since we are aligning things the stupid way we do, we are treating start, end as _bounds_. So we are saying 'turn everything in the bounded range [start, end) into huge pages'. So we use ALIGN() on start so nothing BEFORE start gets converted. And we use ALIGN_DOWN() on end so nothing AFTER end gets converted. But if you do: (first case above) PMD aligned PMD aligned | <-----------------> | You're kinda doing something stupid obviously, because that range cannot span any huge pages, you don't even cross a boundary, and importantly - your range _isn't even large enough to include a single page_. With: PMD aligned PMD aligned | <------------------------|-> You are crossing a boundary but not enough to get a page. But you might well have a large enough range to span a single page... But OTOH... PMD aligned PMD aligned | <---|-> Would get you the same as is equally silly. Yeah ok this is a long way round of coming to the same conclusion as David, godamnit, and I so wanted to disagree here :P Since both get you nothing, and the input was valid _to madvise()_ let's just return 0 for both cases. > > -- > Cheers, > > David Cheers, Lorenzo