From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.223.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 78C5012DD95 for ; Wed, 29 Jan 2025 14:13:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=195.135.223.131 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738159991; cv=none; b=hd65Y7SaRc5NYyIY5U15szrHNppJtNFzGE//omYhJs+/7DtZIdwVNGAXrHxQRui6niIYtdtciiJ9E23znpWOhFO1JP9qI4AH0FVjl8Dm8RSPKWJD76ThuaaO93Q0cbOxRVxty9xI1lOU0ZeIu46ea/Mic7GfzJCjqHLab8aNNr0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738159991; c=relaxed/simple; bh=y1HxBeESB7QpnQGI18K3/rmdlqfDYx3kbiLLaqPSzEQ=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=bNddsL1IEeRlt6SOChM4GuPSSx4A7ykRp+3J4dd6SXEtaqOIIiYHDqN84URDNUfV0YV4Su5v6WlDbKZlIpPTsSRWk58eBIN0WvQh8dAzTs42pUTum2+TpC2KehfDuqmSV7uZWXAdO4FQLQOn5AGanCx1KaUlx4TWA3tjvxtBYg0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=suse.cz; spf=pass smtp.mailfrom=suse.cz; dkim=pass (1024-bit key) header.d=suse.cz header.i=@suse.cz header.b=VPvu91RF; dkim=permerror (0-bit key) header.d=suse.cz header.i=@suse.cz header.b=gTQr5kgq; dkim=pass (1024-bit key) header.d=suse.cz header.i=@suse.cz header.b=XA8G4s0o; dkim=permerror (0-bit key) header.d=suse.cz header.i=@suse.cz header.b=V+XXKVTF; arc=none smtp.client-ip=195.135.223.131 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.cz Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=suse.cz header.i=@suse.cz header.b="VPvu91RF"; dkim=permerror (0-bit key) header.d=suse.cz header.i=@suse.cz header.b="gTQr5kgq"; dkim=pass (1024-bit key) header.d=suse.cz header.i=@suse.cz header.b="XA8G4s0o"; dkim=permerror (0-bit key) header.d=suse.cz header.i=@suse.cz header.b="V+XXKVTF" Received: from imap1.dmz-prg2.suse.org (unknown [10.150.64.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 9CB321F365; Wed, 29 Jan 2025 14:13:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1738159987; h=from:from:reply-to: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:autocrypt:autocrypt; bh=27ElVmP+O0blWhPjSnlInaGynaqJHWGY6ZV2gMpGT74=; b=VPvu91RFQ63Wy0tBwGGZS6g6jkWgZDzb9MH92VhWGgaKIa+irt5Hhb2ghrzOgRY81Zm/wJ VXo3xPrvb8Q4EhhheKXEbLgs6ah5WdJmVBX+/5ikjOA/2hq+1EeViocTEUOd9W4tvF97BQ ViKq7hMcartRsaGyc8+7rOLIySTaEEA= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1738159987; h=from:from:reply-to: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:autocrypt:autocrypt; bh=27ElVmP+O0blWhPjSnlInaGynaqJHWGY6ZV2gMpGT74=; b=gTQr5kgqUKam43r1YkdOBI2B0GJYiMQFLZ3juV0RGZ1k3jgcQ21B7e1RO9BGy69NjtfLQ5 gFMpGb2lYBgcOrAA== Authentication-Results: smtp-out2.suse.de; none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1738159986; h=from:from:reply-to: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:autocrypt:autocrypt; bh=27ElVmP+O0blWhPjSnlInaGynaqJHWGY6ZV2gMpGT74=; b=XA8G4s0oqO+cIJcOBZdMY3dUGDnaIZw4RcMl8B3ntJY5j3IVUzzB5wo74YZDXdemFPb3qF p+mbeRHtVBULvNMH1AU4yLGgamypZFpL97vz9fYWZfEZFgooCG+t7sFiDlfWzpYu4bQpCa uQq0ALUnZMZ2oHJQ+fIVb7S4k2dVBag= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1738159986; h=from:from:reply-to: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:autocrypt:autocrypt; bh=27ElVmP+O0blWhPjSnlInaGynaqJHWGY6ZV2gMpGT74=; b=V+XXKVTFW8zf59OEAXEfMx8YsUbs/hUsjFi6Y6FK3QC/8ZptQ4L7bH/FAaLLhHua8I+y4b wT0FbVowd9f9jpDQ== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 8B13F137DB; Wed, 29 Jan 2025 14:13:06 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id 4uaaIXI3mmctRAAAD6G6ig (envelope-from ); Wed, 29 Jan 2025 14:13:06 +0000 Message-ID: <2e6fac60-3b73-4e7a-a824-21cbe2c326e6@suse.cz> Date: Wed, 29 Jan 2025 15:13:06 +0100 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 3/5] mm: eliminate adj_start parameter from commit_merge() Content-Language: en-US To: Lorenzo Stoakes , Andrew Morton Cc: "Liam R . Howlett" , Jann Horn , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <335ff27920d96c2a45b9724725cd3be2caf987b1.1737929364.git.lorenzo.stoakes@oracle.com> From: Vlastimil Babka Autocrypt: addr=vbabka@suse.cz; keydata= xsFNBFZdmxYBEADsw/SiUSjB0dM+vSh95UkgcHjzEVBlby/Fg+g42O7LAEkCYXi/vvq31JTB KxRWDHX0R2tgpFDXHnzZcQywawu8eSq0LxzxFNYMvtB7sV1pxYwej2qx9B75qW2plBs+7+YB 87tMFA+u+L4Z5xAzIimfLD5EKC56kJ1CsXlM8S/LHcmdD9Ctkn3trYDNnat0eoAcfPIP2OZ+ 9oe9IF/R28zmh0ifLXyJQQz5ofdj4bPf8ecEW0rhcqHfTD8k4yK0xxt3xW+6Exqp9n9bydiy tcSAw/TahjW6yrA+6JhSBv1v2tIm+itQc073zjSX8OFL51qQVzRFr7H2UQG33lw2QrvHRXqD Ot7ViKam7v0Ho9wEWiQOOZlHItOOXFphWb2yq3nzrKe45oWoSgkxKb97MVsQ+q2SYjJRBBH4 8qKhphADYxkIP6yut/eaj9ImvRUZZRi0DTc8xfnvHGTjKbJzC2xpFcY0DQbZzuwsIZ8OPJCc LM4S7mT25NE5kUTG/TKQCk922vRdGVMoLA7dIQrgXnRXtyT61sg8PG4wcfOnuWf8577aXP1x 6mzw3/jh3F+oSBHb/GcLC7mvWreJifUL2gEdssGfXhGWBo6zLS3qhgtwjay0Jl+kza1lo+Cv BB2T79D4WGdDuVa4eOrQ02TxqGN7G0Biz5ZLRSFzQSQwLn8fbwARAQABzSBWbGFzdGltaWwg QmFia2EgPHZiYWJrYUBzdXNlLmN6PsLBlAQTAQoAPgIbAwULCQgHAwUVCgkICwUWAgMBAAIe AQIXgBYhBKlA1DSZLC6OmRA9UCJPp+fMgqZkBQJkBREIBQkRadznAAoJECJPp+fMgqZkNxIQ ALZRqwdUGzqL2aeSavbum/VF/+td+nZfuH0xeWiO2w8mG0+nPd5j9ujYeHcUP1edE7uQrjOC Gs9sm8+W1xYnbClMJTsXiAV88D2btFUdU1mCXURAL9wWZ8Jsmz5ZH2V6AUszvNezsS/VIT87 AmTtj31TLDGwdxaZTSYLwAOOOtyqafOEq+gJB30RxTRE3h3G1zpO7OM9K6ysLdAlwAGYWgJJ V4JqGsQ/lyEtxxFpUCjb5Pztp7cQxhlkil0oBYHkudiG8j1U3DG8iC6rnB4yJaLphKx57NuQ PIY0Bccg+r9gIQ4XeSK2PQhdXdy3UWBr913ZQ9AI2usid3s5vabo4iBvpJNFLgUmxFnr73SJ KsRh/2OBsg1XXF/wRQGBO9vRuJUAbnaIVcmGOUogdBVS9Sun/Sy4GNA++KtFZK95U7J417/J Hub2xV6Ehc7UGW6fIvIQmzJ3zaTEfuriU1P8ayfddrAgZb25JnOW7L1zdYL8rXiezOyYZ8Fm ZyXjzWdO0RpxcUEp6GsJr11Bc4F3aae9OZtwtLL/jxc7y6pUugB00PodgnQ6CMcfR/HjXlae h2VS3zl9+tQWHu6s1R58t5BuMS2FNA58wU/IazImc/ZQA+slDBfhRDGYlExjg19UXWe/gMcl De3P1kxYPgZdGE2eZpRLIbt+rYnqQKy8UxlszsBNBFsZNTUBCACfQfpSsWJZyi+SHoRdVyX5 J6rI7okc4+b571a7RXD5UhS9dlVRVVAtrU9ANSLqPTQKGVxHrqD39XSw8hxK61pw8p90pg4G /N3iuWEvyt+t0SxDDkClnGsDyRhlUyEWYFEoBrrCizbmahOUwqkJbNMfzj5Y7n7OIJOxNRkB IBOjPdF26dMP69BwePQao1M8Acrrex9sAHYjQGyVmReRjVEtv9iG4DoTsnIR3amKVk6si4Ea X/mrapJqSCcBUVYUFH8M7bsm4CSxier5ofy8jTEa/CfvkqpKThTMCQPNZKY7hke5qEq1CBk2 wxhX48ZrJEFf1v3NuV3OimgsF2odzieNABEBAAHCwXwEGAEKACYCGwwWIQSpQNQ0mSwujpkQ PVAiT6fnzIKmZAUCZAUSmwUJDK5EZgAKCRAiT6fnzIKmZOJGEACOKABgo9wJXsbWhGWYO7mD 8R8mUyJHqbvaz+yTLnvRwfe/VwafFfDMx5GYVYzMY9TWpA8psFTKTUIIQmx2scYsRBUwm5VI EurRWKqENcDRjyo+ol59j0FViYysjQQeobXBDDE31t5SBg++veI6tXfpco/UiKEsDswL1WAr tEAZaruo7254TyH+gydURl2wJuzo/aZ7Y7PpqaODbYv727Dvm5eX64HCyyAH0s6sOCyGF5/p eIhrOn24oBf67KtdAN3H9JoFNUVTYJc1VJU3R1JtVdgwEdr+NEciEfYl0O19VpLE/PZxP4wX PWnhf5WjdoNI1Xec+RcJ5p/pSel0jnvBX8L2cmniYnmI883NhtGZsEWj++wyKiS4NranDFlA HdDM3b4lUth1pTtABKQ1YuTvehj7EfoWD3bv9kuGZGPrAeFNiHPdOT7DaXKeHpW9homgtBxj 8aX/UkSvEGJKUEbFL9cVa5tzyialGkSiZJNkWgeHe+jEcfRT6pJZOJidSCdzvJpbdJmm+eED w9XOLH1IIWh7RURU7G1iOfEfmImFeC3cbbS73LQEFGe1urxvIH5K/7vX+FkNcr9ujwWuPE9b 1C2o4i/yZPLXIVy387EjA6GZMqvQUFuSTs/GeBcv0NjIQi8867H3uLjz+mQy63fAitsDwLmR EP+ylKVEKb0Q2A== In-Reply-To: <335ff27920d96c2a45b9724725cd3be2caf987b1.1737929364.git.lorenzo.stoakes@oracle.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Score: -4.30 X-Spamd-Result: default: False [-4.30 / 50.00]; BAYES_HAM(-3.00)[100.00%]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_SHORT(-0.20)[-0.999]; MIME_GOOD(-0.10)[text/plain]; MID_RHS_MATCH_FROM(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; MIME_TRACE(0.00)[0:+]; ARC_NA(0.00)[]; TO_DN_SOME(0.00)[]; RCVD_TLS_ALL(0.00)[]; DKIM_SIGNED(0.00)[suse.cz:s=susede2_rsa,suse.cz:s=susede2_ed25519]; FUZZY_BLOCKED(0.00)[rspamd.com]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_FIVE(0.00)[6]; FROM_EQ_ENVFROM(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; RCVD_COUNT_TWO(0.00)[2]; DBL_BLOCKED_OPENRESOLVER(0.00)[oracle.com:email] X-Spam-Flag: NO X-Spam-Level: On 1/27/25 16:50, Lorenzo Stoakes wrote: > Introduce internal __VMG_FLAG_ADJUST_MIDDLE_START and > __VMG_FLAG_ADJUST_NEXT_START merge flags, enabling us to indicate to > commit_merge() that we are performing a merge which either spans only part > of vmg->middle, or part of vmg->next respectively. > > In the former instance, we change the start of vmg->middle to match the > attributes of vmg->prev, without spanning all of vmg->middle. > > This implies that vmg->prev->vm_end and vmg->middle->vm_start are both > increased to form the new merged VMA (vmg->prev) and the new subsequent VMA > (vmg->middle). > > In the latter case, we change the end of vmg->middle to match the > attributes of vmg->next, without spanning all of vmg->next. > > This implies that vmg->middle->vm_end and vmg->next->vm_start are both > decreased to form the new merged VMA (vmg->next) and the new prior VMA > (vmg->middle). > > Since we now have a stable set of prev, middle, next VMAs threaded through > vmg and with these flags set know what is happening, we can perform > the calculation in commit_merge() instead. > > This allows us to drop the confusing adj_start parameter and instead pass > semantic information to commit_merge(). > > In the latter case the -(middle->vm_end - start) calculation becomes > -(middle->vm-end - vmg->end), however this is correct as vmg->end is set to > the start parameter. > > This is because in this case (rather confusingly), we manipulate > vmg->middle, but ultimately return vmg->next, whose range will be correctly > specified. At this point vmg->start, end is the new range for the prior VMA > rather than the merged one. > > This patch has no change in functional behaviour. > > Signed-off-by: Lorenzo Stoakes Reviewed-by: Vlastimil Babka > --- > mm/vma.c | 53 ++++++++++++++++++++++++++++++++--------------------- > mm/vma.h | 14 ++++++++++++-- > 2 files changed, 44 insertions(+), 23 deletions(-) > > diff --git a/mm/vma.c b/mm/vma.c > index 955c5ebd5739..e78d65de734b 100644 > --- a/mm/vma.c > +++ b/mm/vma.c > @@ -633,29 +633,45 @@ void validate_mm(struct mm_struct *mm) > * > * On success, returns the merged VMA. Otherwise returns NULL. > */ > -static struct vm_area_struct *commit_merge(struct vma_merge_struct *vmg, > - long adj_start) > +static struct vm_area_struct *commit_merge(struct vma_merge_struct *vmg) > { > - struct vma_prepare vp; > struct vm_area_struct *remove = NULL; > struct vm_area_struct *remove2 = NULL; > + unsigned long flags = vmg->merge_flags; > + struct vma_prepare vp; > struct vm_area_struct *adjust = NULL; > + long adj_start; > + bool merge_target; > + > /* > - * In all cases but that of merge right, shrink next, we write > - * vmg->target to the maple tree and return this as the merged VMA. > + * If modifying an existing VMA and we don't remove vmg->middle, then we > + * shrink the adjacent VMA. > */ > - bool merge_target = adj_start >= 0; > + if (flags & __VMG_FLAG_ADJUST_MIDDLE_START) { > + adjust = vmg->middle; > + /* The POSITIVE value by which we offset vmg->middle->vm_start. */ > + adj_start = vmg->end - vmg->middle->vm_start; > + merge_target = true; > + } else if (flags & __VMG_FLAG_ADJUST_NEXT_START) { > + adjust = vmg->next; > + /* The NEGATIVE value by which we offset vmg->next->vm_start. */ > + adj_start = -(vmg->middle->vm_end - vmg->end); Wouldn't it make more sense to use vmg->next->vm_start here, which AFAIU is the same value as vmg->middle->vm_end, but perhaps a bit more obvious considering the flag and comment says we're adjusting vmg->next->vm_start. (somewhat less important if this code is temporary) > + /* > + * In all cases but this - merge right, shrink next - we write > + * vmg->target to the maple tree and return this as the merged VMA. > + */ > + merge_target = false; That comment reads like it would make sense to init merge_target as true and only here change it to false. > + } else { > + adjust = NULL; > + adj_start = 0; > + merge_target = true; > + } I guess all these could be initial assignments and we don't need the else branch at all. > - if (vmg->merge_flags & __VMG_FLAG_REMOVE_MIDDLE) > + if (flags & __VMG_FLAG_REMOVE_MIDDLE) > remove = vmg->middle; > if (vmg->merge_flags & __VMG_FLAG_REMOVE_NEXT) > remove2 = vmg->next; > > - if (adj_start > 0) > - adjust = vmg->middle; > - else if (adj_start < 0) > - adjust = vmg->next; > - > init_multi_vma_prep(&vp, vmg->target, adjust, remove, remove2); > > VM_WARN_ON(vp.anon_vma && adjust && adjust->anon_vma && > @@ -739,7 +755,6 @@ static __must_check struct vm_area_struct *vma_merge_existing_range( > bool left_side = middle && start == middle->vm_start; > bool right_side = middle && end == middle->vm_end; > int err = 0; > - long adj_start = 0; > bool merge_will_delete_middle, merge_will_delete_next; > bool merge_left, merge_right, merge_both; > > @@ -860,11 +875,8 @@ static __must_check struct vm_area_struct *vma_merge_existing_range( > vmg->start = prev->vm_start; > vmg->pgoff = prev->vm_pgoff; > > - /* > - * We both expand prev and shrink middle. > - */ > if (!merge_will_delete_middle) > - adj_start = vmg->end - middle->vm_start; > + vmg->merge_flags |= __VMG_FLAG_ADJUST_MIDDLE_START; > > err = dup_anon_vma(prev, middle, &anon_dup); > } else { /* merge_right */ > @@ -893,12 +905,11 @@ static __must_check struct vm_area_struct *vma_merge_existing_range( > * IMPORTANT: This is the ONLY case where the final > * merged VMA is NOT vmg->target, but rather vmg->next. > */ > + vmg->merge_flags |= __VMG_FLAG_ADJUST_NEXT_START; > vmg->target = middle; > vmg->start = middle->vm_start; > vmg->end = start; > vmg->pgoff = middle->vm_pgoff; > - > - adj_start = -(middle->vm_end - start); > } > > err = dup_anon_vma(next, middle, &anon_dup); > @@ -912,7 +923,7 @@ static __must_check struct vm_area_struct *vma_merge_existing_range( > if (merge_will_delete_next) > vmg->merge_flags |= __VMG_FLAG_REMOVE_NEXT; > > - res = commit_merge(vmg, adj_start); > + res = commit_merge(vmg); > if (!res) { > if (anon_dup) > unlink_anon_vmas(anon_dup); > @@ -1087,7 +1098,7 @@ int vma_expand(struct vma_merge_struct *vmg) > if (remove_next) > vmg->merge_flags |= __VMG_FLAG_REMOVE_NEXT; > > - if (!commit_merge(vmg, 0)) > + if (!commit_merge(vmg)) > goto nomem; > > return 0; > diff --git a/mm/vma.h b/mm/vma.h > index ffbfefb9a83d..ddf567359880 100644 > --- a/mm/vma.h > +++ b/mm/vma.h > @@ -67,16 +67,26 @@ enum vma_merge_flags { > * at the gap. > */ > VMG_FLAG_JUST_EXPAND = 1 << 0, > + /* > + * Internal flag indicating the merge increases vmg->middle->vm_start > + * (and thereby, vmg->prev->vm_end). > + */ > + __VMG_FLAG_ADJUST_MIDDLE_START = 1 << 1, > + /* > + * Internal flag indicating the merge decreases vmg->next->vm_start > + * (and thereby, vmg->middle->vm_end). > + */ > + __VMG_FLAG_ADJUST_NEXT_START = 1 << 2, > /* > * Internal flag used during the merge operation to indicate we will > * remove vmg->middle. > */ > - __VMG_FLAG_REMOVE_MIDDLE = 1 << 1, > + __VMG_FLAG_REMOVE_MIDDLE = 1 << 3, > /* > * Internal flag used during the merge operationr to indicate we will > * remove vmg->next. > */ > - __VMG_FLAG_REMOVE_NEXT = 1 << 2, > + __VMG_FLAG_REMOVE_NEXT = 1 << 4, > }; > > /*