Received-SPF: pass (google.com: domain of linux-kernel+bounces-75010-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 21 Feb 2024 10:38:27 -0500
From: "Liam R. Howlett" <Liam.Howlett@Oracle.com>
To: Yajun Deng <yajun.deng@linux.dev>
Cc: akpm@linux-foundation.org, vbabka@suse.cz, lstoakes@gmail.com,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] mm/mmap: return early if it can't merge in vma_merge()
Message-ID: <20240221153827.wkmjnnwsf6lyxatc@revolver>
Mail-Followup-To: "Liam R. Howlett" <Liam.Howlett@Oracle.com>,
	Yajun Deng <yajun.deng@linux.dev>, akpm@linux-foundation.org,
	vbabka@suse.cz, lstoakes@gmail.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
References: <20240221091453.1785076-1-yajun.deng@linux.dev>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240221091453.1785076-1-yajun.deng@linux.dev>
User-Agent: NeoMutt/20220429
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?Iv0E3SuttJvHrQ0FL+BKcOzvAphQQEZOjW0lkzsBB0ogD85hSI8vsZJMZJ85?=
 =?us-ascii?Q?4FjersiY9+CAoEt0ZX4N30OYMgohSSAGdpR5w+sFEJMEZRNCSEvA9ih944gu?=
 =?us-ascii?Q?7/hNzsEd4b6vIpzDI85Kw0zNwpEsILyFqro56yYrqduLWyNxCtBNUugNZKcS?=
 =?us-ascii?Q?+kks1itDQYCfua1ZHyk/GOGTlROZEXzmvEkgTU6J1mB8abJkblP6Kz6UvH2B?=
 =?us-ascii?Q?qS/1KzL7z54Uo8lhdABXlLC7KJ1Zh81TUMdpoK3AtZlcEihH5KLWyciujWvc?=
 =?us-ascii?Q?5YOGnL2itbfRtCgvxa39dHELIgZonDt7dSxcX03Mm7FRiqoS0GFokj3g2eEO?=
 =?us-ascii?Q?/gUvMKY5VdPLrmyX8stGkU2sVY2GkR+XeDt0+Q2quPIa707YGiMGdZrPvmB5?=
 =?us-ascii?Q?y4642+yqqOMsnN+OdOvd5uKJ2+53Be51HxZ3Jm1Wxi19PKRidCLzu0g2UraB?=
 =?us-ascii?Q?QzgO1V9/DewqnkCACt6fhB1kY4ak+iOcDWGa+jtd21e+jPiR3Q6y/xti36Fq?=
 =?us-ascii?Q?ZmJZB8/fmSJcI8WjWhHEeLHt2KXHmDjMKXYHjVkgeSZGVWIw1OJHdTwCz92w?=
 =?us-ascii?Q?3eNbaP78BOFipA/uqwdw/j1Se+lyg1b9mOrmSx4VuUf3msyUXOLtrE62/m6G?=
 =?us-ascii?Q?S16scdA9y0sWo8h8s7gNrV4EyJK6W6zGmYU6f0Ba1hs2rPpdsl6ZBLMalKrT?=
 =?us-ascii?Q?KpgiiQoXk3dWtJugqbYnYjT/AZkO1G5NQnPleOWDjTdMnmT0Z52+O2CExtYb?=
 =?us-ascii?Q?V4Wsoa5iSUQzjNeYz8QudFmFJKsN+XhW5orSPmAa/4BLralrc3HJLz2JkhLU?=
 =?us-ascii?Q?t2ASCCmgPG3hPMNcdixfwP8FO1DFtG0/oNl5FIrdFBu7BZmfgyWyiYMMUjs7?=
 =?us-ascii?Q?yQApJl3Bb8UkH3OUYNeNepUG6AQaYqj9L0i1rgudtGsdBuaZCuTz8GNOypuU?=
 =?us-ascii?Q?mBi3L7KicGdal0MnfuLeBg0Wuq8y+120XaZZ0FIs11ZMUnERGLOyhdVF8t4n?=
 =?us-ascii?Q?hbgwBm9l7tcsv1Vxj6joQnyRZlccqp73Fu8JnU4xzsVkLnEtAYJEp+K11jA7?=
 =?us-ascii?Q?kRs5+yENFyg9a0E8CUUDT+oQCedeXbV9o1QtRJJ/0837VcpFrTTIBIh4vPoP?=
 =?us-ascii?Q?mNP/kwe4csqI5yjX0r2xFA6BD4FX4eUDRzSYwvvQ723SDxAGORm/Kug5m/SU?=
 =?us-ascii?Q?Fq9SFR2U7Sa0LgDPjmJBDixdM0U4SHsCkB8jTP8+wTURw4wGWBodG07OGKfz?=
 =?us-ascii?Q?0k4StI/32TNR1dMBRLGst0fYF0N/QNflZ8ngp71H+10Xmqc7ch1hqtHRkhSa?=
 =?us-ascii?Q?CWXnLRDDdIoxgLmkusLMoTzfpQGnuxfPDaoBZYOcrNlLnZYEOunn862zJlqT?=
 =?us-ascii?Q?SXqNSoBGfkfa6g7DwpM9cV5SwJ+tIJv81zFn7azALgoEBvxur0pZUWbN8hmI?=
 =?us-ascii?Q?Yiy4jVB3R7y8tuCJ3zuHIGuNagnnQq8DGYOvNw/LnQ2QlXQKyoaG6q9pFVqF?=
 =?us-ascii?Q?yqBzXPtKeU9W+lGzTlsYlCsuo7GUuklk/sqzyWJHF3euNUF+sqx/kkp+6kpx?=
 =?us-ascii?Q?rkLjeAGIOHdq8TIU3BLPYpDEXCjy2si24+ACZ2pBAXXi0waFM8R46J6ysPKa?=
 =?us-ascii?Q?cg=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	hiofgXPYeNTF8HHIiZ/1Pa8na8ifkw2wlaQVwKN3wGLzNON7ekfE6ubHpS00yyboRepAPkTCZt5mxdqEyg5hGrWxpzyYZrm+z4lBxhgYHlluCjNlIexOAj5s+F8pZOfE6fMn2Wa6juB5y02aiPacuIB7WbuJi/PLY2uoLPlUeyECZuLQGdkcikxuopZJc8mOlzxefwkeXpLT/gV1zN7Or9yau2pNBvdlI1X+tTjyQAP9J7yjeNq0h+otesCx8S1darDBDa3K42E0SNKGNgQJ42zN38QMgglm0yKb5W4JIVOKOTrVTVXGx5UzFLF6mWPyfsEgi5866d7O84fsgf3i/NQKgSU+TqqgVBriJihAqxNq4nSlo7GI4vw3k4eUYW8RGKAH4FxhZkYdtFtAeTI/VzbS7IQ/3cH4DmFzHfO4TuAOQjEvxGwzrp8MiE6q2ctceyAN99cfnbMBSyFUt2TWyxi+Wj21fCzm9enaLE0ibpOj8eRTLIymZ+E4KXBKWWB0Hp7C48i+J2gV4jpMlj/pmrTJ7yks+zvke9yZ4hZYxM6twPiqr3oSHQBrYNUQW8iWk4XQeyeYGPfPbZ2JJIh6jScfbV22wfyKJDLmnOlhHxU=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a4db4ec3-c973-4fc4-ee30-08dc32f32707
X-MS-Exchange-CrossTenant-AuthSource: DS0PR10MB7933.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Feb 2024 15:38:29.4238
 (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: vuiIMfWjGB+C+GMqVYwgAmKoi0pWERtP63F2tvtMgvGk1lavVc7Z1Fb5rjKrPpofanPC7XN6u/3DZaKP7ll8dw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB5708
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-02-21_02,2024-02-21_02,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 suspectscore=0
 mlxscore=0 spamscore=0 mlxlogscore=632 phishscore=0 bulkscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2311290000 definitions=main-2402210120
X-Proofpoint-GUID: gmiMlZNa8aOF7b2h0_fr8ALl32XFVy4h
X-Proofpoint-ORIG-GUID: gmiMlZNa8aOF7b2h0_fr8ALl32XFVy4h

* Yajun Deng <yajun.deng@linux.dev> [240221 04:15]:
> In most cases, the range of the area is valid. But in do_mprotect_pkey(),
> the minimum value of end and vma->vm_end is passed to mprotect_fixup().
> This will lead to the end is less than the end of prev.
> 
> In this case, the curr will be NULL, but the next will be equal to the
> prev. So it will attempt to merge before, the vm_pgoff check will cause
> this case to fail.
> 
> To avoid the process described above and reduce unnecessary operations.
> Add a check to immediately return NULL if the end is less than the end of
> prev.

If it's only one caller, could we stop that caller instead of checking
an almost never case for all callers?  Would this better fit in
vma_modify()?  Although that's not just for this caller at this point.
Maybe there isn't a good place?

Or are there other reasons this may happen and is better done in this
function?

Often, this is called the "punch a hole" scenario; where an operation
creates two entries from the old data and either leaves an empty space
or fills the space with a new VMA.

> 
> Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
> ---
> v2: remove the case label.
> v1: https://lore.kernel.org/all/20240218085028.3294332-1-yajun.deng@linux.dev/
> ---
>  mm/mmap.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 0fccd23f056e..7668854d2246 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -890,6 +890,9 @@ static struct vm_area_struct
>  	if (vm_flags & VM_SPECIAL)
>  		return NULL;
>  
> +	if (prev && end < prev->vm_end)
> +		return NULL;
> +
>  	/* Does the input range span an existing VMA? (cases 5 - 8) */
>  	curr = find_vma_intersection(mm, prev ? prev->vm_end : 0, end);
>  
> -- 
> 2.25.1
>