From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0E859C4345F for ; Fri, 3 May 2024 22:29:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 284996B007B; Fri, 3 May 2024 18:29:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 234C96B0082; Fri, 3 May 2024 18:29:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0D5286B0083; Fri, 3 May 2024 18:29:34 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id DBBA46B007B for ; Fri, 3 May 2024 18:29:33 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 5BD49A1163 for ; Fri, 3 May 2024 22:29:33 +0000 (UTC) X-FDA: 82078527426.17.E415864 Received: from mail-yb1-f180.google.com (mail-yb1-f180.google.com [209.85.219.180]) by imf22.hostedemail.com (Postfix) with ESMTP id A1784C000C for ; Fri, 3 May 2024 22:29:31 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=pgPfVbsL; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf22.hostedemail.com: domain of surenb@google.com designates 209.85.219.180 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1714775371; h=from:from:sender: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:dkim-signature; bh=tlMjJ5CfnZ4xKYYAFVLOItyc2ZtPoj3gNMYwzfndvZg=; b=Q6bvCyhiuyNg817bHu0RZb0MHcvxnZw++vQ/gx48hmQoqgSjGklMLlBDEEk9YXbeuo08EX C7uy6qgk1Hp6ZvDvSMX1WI06gpTT1ZlMUiluBey+4Dg9MU2WCPRMmykQkel2o+8wf8A7Ub yiR0uTCGYwzZWXM/GAir6EUgmwlimW0= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=pgPfVbsL; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf22.hostedemail.com: domain of surenb@google.com designates 209.85.219.180 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1714775371; a=rsa-sha256; cv=none; b=1/shNvGKOcQZvt8enIOXkiTJLPxQftBBWSmamdopRZOWt/5H8hZuFiCO+HfvcelBMoeQKM UsB/38Seno6E4l1ap1Il0SYDdntKLSAXHKkBIluttdsatWFVyob5P5QGsXW3U/dtCwtoJe FiY+KvIEpDU5gVjg6S8wZu38eA5Za5Q= Received: by mail-yb1-f180.google.com with SMTP id 3f1490d57ef6-dc6dcd9124bso179990276.1 for ; Fri, 03 May 2024 15:29:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1714775371; x=1715380171; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=tlMjJ5CfnZ4xKYYAFVLOItyc2ZtPoj3gNMYwzfndvZg=; b=pgPfVbsLUBbOjSJE0jOzal9uOFPfhGuhKdRm2UX7CDhRoFkFqIc63sTZ+WFwTWmvY3 /EJu5WX6tbOPnvaEnzUbHHNAl0gpOFMxJbgxULjTU07ONbltqdHThTBObJIPlBDDIrt7 rs892EldfwyLOvJhUOu5hSKxTeZFDM5EL0bVD1e4s8tyhcXelDELlQp3a/BlwOeOqGkl Gp4WQsaJ7sxgzYiRkdpyp8F2Yb0oyrxrxqSBwczXYNXmQaFfUjN6eejF/dNwzW5OWTeV kKMdn3ZSGjs+jHINldnyrslYZ0sVePbkTY7YTEUQtiIZ77/Bi3WoTmI9/J6lu5Hns22/ 39vw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714775371; x=1715380171; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=tlMjJ5CfnZ4xKYYAFVLOItyc2ZtPoj3gNMYwzfndvZg=; b=VnqSzXe88uEzdUp5pYJHWP/qwSsyVnMZvIifzX++kLqGruQKYikbVM9xs1fuL+Hdvr 6ix/ihQb5OUHiliMidaXlxauAfjdQ2f//Y4vqJ5pckWJembW7B8DXa025W6kLJGS+qr4 7w/crk+IVfJsRppnabNXRYBXxeCyVuevKTwuZc7QsbOfpwuIKnnwSsUb7t58QICDJwGb pyYXkjVuo65JCtZByxP3LODdgn1VsGU1xox5PRHSYJ+BxhfMGWg5M+MEOz8S+0GGO3fe RYoK9WgrR0SGn2yUZSOCqjnn4ocmJcA7Wa1kVE+v8M/dAx9NpxtmHVbONf57YGp++JZT GeIw== X-Gm-Message-State: AOJu0YyyL4pbpd9rdRbnBOHrKHAiHAhp7liXRzjI3nStPdsaWEWfxRN+ /rvXtlXlYkFl85gUQJAhfd/hzhiEtTYG33dorIaqLkS8vY0uEjRlrphwHZ+XbaWhTUBFmhdAeSI l+wDf49RyIQDepACG0LUshAI0cbFCZCWUZ1tN X-Google-Smtp-Source: AGHT+IGnDqMJQjYoT2WywakY88/ZA4ZMnZSc0Ab2HFp6x7jblhlnEo3RFJYg2MQiyf1RDHM/TVdECZJCPiRFQTUDSIM= X-Received: by 2002:a5b:584:0:b0:de6:1245:c3d5 with SMTP id l4-20020a5b0584000000b00de61245c3d5mr4106051ybp.60.1714775370471; Fri, 03 May 2024 15:29:30 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Suren Baghdasaryan Date: Fri, 3 May 2024 15:29:16 -0700 Message-ID: Subject: Re: [RFC] Make find_tcp_vma() more efficient To: Matthew Wilcox Cc: linux-mm@kvack.org, netdev@vger.kernel.org, "Liam R. Howlett" , Arjun Roy Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: A1784C000C X-Stat-Signature: jpejm4qnqzst6dwymk5wr5d7td3n8xpm X-HE-Tag: 1714775371-624310 X-HE-Meta: U2FsdGVkX19Y3Mla3hic9fFqz9rMIL+y+AdOZL+Ncxjf129mEVnCrxvdepegOYo1wvEaXB0E5EtxlfsF8GyM/WycQOkrrZO8cIiXvN9ScQPHXs38eqxND2mpo4cp9Oxv69y4d7nLr1T3VO5rQlSTs5KDcvlNBZ5clclma/Ni9JUXCNx7CDXicawv7RLQ65tTAPXn0u6tcEtIlN94C6l5dkR17ClcvsDT0PREvXJ5f/5fzPl+azHCCCx2A93As3sO3n0QFPkA+of7veOeCWeK2RhMZTRh1hY1e5Uf7zeUXYEckVpxpHU8qvUErZWXdU1iqGPobxi2MyX2MEiVQbqloYfZ4dVRaZh4MdApE5fTlTjgV/F57Q1LkCUZDkjBc5+Dxhtzcm5qQSisUb9HAieKuIBSQKPylNMCKKmY9Masbd610kwnjcjkL0uwoRv714476IUjc+mPl18Co/9gpABZ/CMSRWMJaQDO0LpzV/tuY4esPV4EldDpPjl3Geh6h3NLxdg9MUtSAvnne5g5rNeX+d3MLj3EL70la0Y1T1rxTICkoxufX2bq8JS0+w+qpHu+PbnZyPmhfNpF98r3VogVD+ERVCJ7gjo4h4+g8vGUoyz1LCtFpYzOWPNUFeDlp4uqG3YrAfmzOVvIQedUvGK5DhuBJ9/P5+HgffH6V3IuC2oWJxIRhnv8i5y87LycmvVZCFMtg6zDuKhXVV9sKhm6D31BdoZXwerzzhfv6nHZGRjDLL50qxK+Pg1EEipqF1kkuSY5JYQHiiXiwusi5FYU7alKMj0gN9nmezKfjpUT/2GE8aRZP1zvlBIiu3mj+89I65+pTRjsWNmt6+qJAE6P442/V8QV5PKu9waIgVqrsa3NJcFGtfBg6FFR/sjLEBsXnozWGYEKWr1zk49sfGL5Lzo0r5bS12QHruZ+Fudj6jZhh7SNvquFGjzfp7QmuNx3Dl5oxNINbtGhS7BtSts FdaQiWwt ROB86/yPsi8x+KamrVh02zwKOvX0lVAGOmRaC3RMnz+k/G+8rZKOE87V7Opxxjo0c6+R+zZeiTNV6rZzpuBQOfLiQtq/5iYFpgSl4x7xOsp0B7Dvn3W/TJegg1hXbkfeD+YYHY1RXpBlujnceg59OmjnNUiUHIb8rfJVNfQM/sUrHzvLMYbmMaQ8AkgtTrEkYTaymqF/NTzoEtbAfYUiGRiVkw1qf8l9CThg8cAP5HgFi3Xofh+wNzthRXalYlnUSpvK5h+VyqHwFkXl+ztgsGY2ShFa5JEYBKR+UeyhmVNL/7RdfRw006dQtlS8/kP/C+dogoTniaVwyYK8= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000041, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Fri, Apr 26, 2024 at 10:15=E2=80=AFAM Matthew Wilcox wrote: > > Liam asked me if we could do away with the "bool *mmap_locked" > parameter, and the problem is that some architctures don't support > CONFIG_PER_VMA_LOCK yet. But we can abstract it ... something like this > maybe? > > (not particularly proposing this for inclusion; just wrote it and want > to get it out of my tree so I can get back to other projects. If anyone > wants it, they can test it and submit it for inclusion and stick my > S-o-B on it) I went through all uses of vma_end_read() to convince myself this is safe with CONFIG_PER_VMA_LOCK=3Dn and the change seems fine from correctness POV. However the fact that in this configuration lock_vma_under_mmap_lock()=3D=3DNOP and vma_end_read()=3D=3Dmmap_read_unloc= k() does not feel right to me. Current code is more explicit about which lock is held and I think it's easier to understand. A new interface like below might be a bit better but I'm not sure if it's worth it: #ifdef CONFIG_PER_VMA_LOCK static inline void mmap_to_vma_lock(struct vm_area_struct *vma) { down_read(&vma->vm_lock->lock); mmap_read_unlock(vma->vm_mm); } static inline void mmap_or_vma_unlock(struct vm_area_struct *vma) { vma_end_read(vma); } #else /* CONFIG_PER_VMA_LOCK */ static inline void mmap_to_vma_lock(struct vm_area_struct *vma) {} static inline void mmap_or_vma_unlock(struct vm_area_struct *vma) { mmap_read_unlock(vma->vm_mm); } #endif /* CONFIG_PER_VMA_LOCK */ > > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 9849dfda44d4..570763351508 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -779,11 +779,22 @@ static inline void assert_fault_locked(struct vm_fa= ult *vmf) > struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm, > unsigned long address); > > +static inline void lock_vma_under_mmap_lock(struct vm_area_struct *vma) > +{ > + down_read(&vma->vm_lock->lock); > + mmap_read_unlock(vma->vm_mm); > +} > + > #else /* CONFIG_PER_VMA_LOCK */ > > static inline bool vma_start_read(struct vm_area_struct *vma) > { return false; } > -static inline void vma_end_read(struct vm_area_struct *vma) {} > +static inline void vma_end_read(struct vm_area_struct *vma) > +{ > + mmap_read_unlock(vma->vm_mm); > +} > + > +static inline void lock_vma_under_mmap_lock(struct vm_area_struct *vma) = {} > static inline void vma_start_write(struct vm_area_struct *vma) {} > static inline void vma_assert_write_locked(struct vm_area_struct *vma) > { mmap_assert_write_locked(vma->vm_mm); } > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c > index f23b97777ea5..e763916e5185 100644 > --- a/net/ipv4/tcp.c > +++ b/net/ipv4/tcp.c > @@ -2051,27 +2051,25 @@ static void tcp_zc_finalize_rx_tstamp(struct sock= *sk, > } > > static struct vm_area_struct *find_tcp_vma(struct mm_struct *mm, > - unsigned long address, > - bool *mmap_locked) > + unsigned long address) > { > struct vm_area_struct *vma =3D lock_vma_under_rcu(mm, address); > > - if (vma) { > - if (vma->vm_ops !=3D &tcp_vm_ops) { > - vma_end_read(vma); > + if (!vma) { > + mmap_read_lock(mm); > + vma =3D vma_lookup(mm, address); > + if (vma) { > + lock_vma_under_mmap_lock(vma); > + } else { > + mmap_read_unlock(mm); > return NULL; > } > - *mmap_locked =3D false; > - return vma; > } > - > - mmap_read_lock(mm); > - vma =3D vma_lookup(mm, address); > - if (!vma || vma->vm_ops !=3D &tcp_vm_ops) { > - mmap_read_unlock(mm); > + if (vma->vm_ops !=3D &tcp_vm_ops) { > + vma_end_read(vma); > return NULL; > } > - *mmap_locked =3D true; > + > return vma; > } > > @@ -2092,7 +2090,6 @@ static int tcp_zerocopy_receive(struct sock *sk, > u32 seq =3D tp->copied_seq; > u32 total_bytes_to_map; > int inq =3D tcp_inq(sk); > - bool mmap_locked; > int ret; > > zc->copybuf_len =3D 0; > @@ -2117,7 +2114,7 @@ static int tcp_zerocopy_receive(struct sock *sk, > return 0; > } > > - vma =3D find_tcp_vma(current->mm, address, &mmap_locked); > + vma =3D find_tcp_vma(current->mm, address); > if (!vma) > return -EINVAL; > > @@ -2194,10 +2191,7 @@ static int tcp_zerocopy_receive(struct sock *sk, > zc, total_bytes_to_map= ); > } > out: > - if (mmap_locked) > - mmap_read_unlock(current->mm); > - else > - vma_end_read(vma); > + vma_end_read(vma); > /* Try to copy straggler data. */ > if (!ret) > copylen =3D tcp_zc_handle_leftover(zc, sk, skb, &seq, cop= ybuf_len, tss);