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 90CD0C52D73 for ; Thu, 8 Aug 2024 21:21:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2B95F6B0088; Thu, 8 Aug 2024 17:21:41 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 242306B0095; Thu, 8 Aug 2024 17:21:41 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0E2F16B0098; Thu, 8 Aug 2024 17:21:41 -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 E3B9C6B0088 for ; Thu, 8 Aug 2024 17:21:40 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 90919140128 for ; Thu, 8 Aug 2024 21:21:40 +0000 (UTC) X-FDA: 82430349960.03.7E0C8C7 Received: from mail-ej1-f53.google.com (mail-ej1-f53.google.com [209.85.218.53]) by imf17.hostedemail.com (Postfix) with ESMTP id B246840027 for ; Thu, 8 Aug 2024 21:21:38 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=QgHjwgAq; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf17.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.218.53 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723152089; a=rsa-sha256; cv=none; b=OdwnOZFVAowz+rk7exRfViiyqS1OLTEV6J4h4LL75xqUalrpYkCFl/fnQFuAb8w8w4h+9g QO+tcdU7kLC0VPXFPv4YFyaNBtw5NoYmxmpFsWtaMFQE4xD2ypbGjtwPbL4r9KwclkeYgk BpOo5hbroclwc+ExATuUGV/8sb0oWMw= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=QgHjwgAq; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf17.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.218.53 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723152089; 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=0JclHXfYDnbuGnbRE8EV1RltyDub11sOgtZzsv2IjQE=; b=pBojv2aJb8UWFS0ky4Fm0xpT1neg2xZ1C4UozsIgyWD1Jw7XywWiGDfQG34yZPb+NCIn1n z72+gyQ4rUQPstQC3JEErNYqyd9mysXew+H89+QqY3O0Bz748jItCYywjI5C3t5E7H2Q+V 19kROX72oX+aQn1Zs2nhbpAlc5QopyM= Received: by mail-ej1-f53.google.com with SMTP id a640c23a62f3a-a7a8caef11fso171051166b.0 for ; Thu, 08 Aug 2024 14:21:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1723152097; x=1723756897; 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=0JclHXfYDnbuGnbRE8EV1RltyDub11sOgtZzsv2IjQE=; b=QgHjwgAqH4HPn/uTwMlIuAsDmssh/Q2lWZ29sHasmWwSpsqVXvBph+fUQFuJVtvxQS d9WGaqxV9+S9UqVkmUgI9P1XfhCY4YpwYp2zZpJVJmLRK9fzrQbCE8w9+5+q/bt+qEJi 5y9CLvEQn+9Rs3odNV/P+2af2aFj7CpTwbphf8fnmas7gmVxm/gV2wyM+A1nCl154G/e GF4YjcSVHc1yGxamejQrpQoiOmt5F+ifV9l+SsmdKtwN8pOeAF2xWo1YBRCGaULYSQew LCC0XULY+5h0YGgtwc7kKzvwowZ2OhuKyTcy70nKubrelmi+ir5kSJbL1qP3tzJ5dX2q J9lA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723152097; x=1723756897; 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=0JclHXfYDnbuGnbRE8EV1RltyDub11sOgtZzsv2IjQE=; b=sKLeVrXNWEnkqrRw9oyUE4yKkaMuemQ/qFT3O+PL1voPSdi+T5jpC3xeq7cI/ejZRk BW5WKAgfNNhcLtrTbZpo8kOUDOn/6T/vrT+ecz7uuEykmQKZgHWGXg4MfaShyTAoLBtq nQ4WHar80kXfGlFDU+dD4XW03A527KAeemf2/J5rJpC/9KMmql3GjlL0qrKHPNg2lKWw A6leg3Y4NpZbjPagLoQb1dzH0GFrotu0RtWrLfV//ayp8Ck6zqLxZ2nwJ8WgT8dG1Sle nstKZTxhSWEp15nxL1omz862YILywSyJ/wpNWlSYEGhygBDjkKYhMa1MPNNVC7pwbvjp tr2g== X-Forwarded-Encrypted: i=1; AJvYcCWnHCbOyOK3wmp5fhnENwJTReuzGHhWUHJ3zUl9/fQI2eM1XCdoA7c81YDdB1yg+AmW7kOy2uqeF8zkoCJ3O3znA24= X-Gm-Message-State: AOJu0YyDwSf+gmc8wUZ2fDn+g8vl5Bzrw66p/pyd9MCZ2m2w+ATviUTh 4WZMxUK8eKLsyG0d4qinfe8m+9aj1RexfLVVnZekt9TUGlsGHMcfZRa3NoqzEZ5dMHv7rwv/vFr T0TzjCSIARVGZ9I8L5Wn3T9KiSQU= X-Google-Smtp-Source: AGHT+IHAfWZuVZQYtiKRzjjCbfmEdzWeFZyhDDivJYrgxVAj7xBmUcYdzvDc6aLLE5erDZ3wIrvXL6Etc+8R/4JeF4o= X-Received: by 2002:a17:906:f59c:b0:a6f:59dc:4ece with SMTP id a640c23a62f3a-a8090c25982mr183186266b.2.1723152096588; Thu, 08 Aug 2024 14:21:36 -0700 (PDT) MIME-Version: 1.0 References: <20240807234029.456316-1-andrii@kernel.org> <20240807234029.456316-7-andrii@kernel.org> In-Reply-To: From: Andrii Nakryiko Date: Thu, 8 Aug 2024 14:21:20 -0700 Message-ID: Subject: Re: [PATCH v4 bpf-next 06/10] lib/buildid: implement sleepable build_id_parse() API To: Shakeel Butt Cc: Andrii Nakryiko , bpf@vger.kernel.org, linux-mm@kvack.org, akpm@linux-foundation.org, adobriyan@gmail.com, hannes@cmpxchg.org, ak@linux.intel.com, osandov@osandov.com, song@kernel.org, jannh@google.com, linux-fsdevel@vger.kernel.org, willy@infradead.org, Omar Sandoval Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Queue-Id: B246840027 X-Rspamd-Server: rspam01 X-Stat-Signature: uo77oof3ojz7y5g7wgkpth4t3zixjmsf X-HE-Tag: 1723152098-41977 X-HE-Meta: U2FsdGVkX18pGQ1bqEbxzUbvbLtMjCJuQFs+9TCoeXsMr7Dgsl7m0Bocc2p0DwkVwp9tQJatBg6FEu9jmUKQtNxuNL6z4YPkHo8OwkkUWOz5OtTlQUNe2VW2NiGs1mysaDlM+7Uj6HGBI7kQXQe/6Y9Rnz82dIxMGNqLJlV3bGeklEr/xDBAkrqO+JJOS1/kFpmu/QHJ2vlQIvA2mBxrSt93mrVEHGc1ZAyTimoe+4TbEd3wIEnojeOoAEce4kr1z2TRFflW4FFbdNrUgX6J/ssIqGY8thw1gqCTPi3LM6aeu1dDgoblHXlv5uKjau6m7cp1FDvZFDmU01NcAC2QsSIwuNa3xyAa6v15i9dfkMgMbUX3uchqYlXuJ5/3ahufgCSTsVDUbQdkOVGr91WkmK/Y+IZPANHJMOa2+fLlcaZPq79p5zEgw6a94vcUXiWPFo6N64OOcmQ/1BM5Lgm8F5Op/MRcDHc6nrDbbLvNc2DKVN3jp57BC1eVBKYx1EWtaB31+4pFndJyqHjg/Wz70vmI4ooDq0uY9NYgGvBN/7C9+jvdpQewx0VW/YQvNTa0HHK/TuEEaDBhZBbNf2N+Xb/iHe5MvE9iG2DhBYUajiROS99CAHVJcRYp3DJxYMjyWkoCaL3sAhxvFQFdrlXf7JlwqsILxPuHtzJ69WSaylZxJEfnmNy+eZ621fMqyOnE9uux0eweFkxyTA8AO9faiHuhYgq+GI8iOwiXAdiH5q3CGppGCLJQEfvEyIIcEbN0b8qkYzpICJ4BuWblR0g/sUC47fmwHq1iyTFRS0RWwFq6aKK3sHkpu6ARc7O0G9wuUTcKC2hzDVcCTjIwfOgUi2ZibdAt/tIM/AWi/2Zh6z+/1qNDoXjHknoKa/mSQUTwzsWPRbjZ15+uF6OKFxHxKNPTzXv9MXqBEH0zJ7vvMOXT14nzAA3TvbkD9/AUe08v03oP8245pvVOrTeRKk8 4DqsuYY5 kvC7FEBrgffxP+U5sXU5HSnTUwYl27N6skxLq0s69OtNc4labv4hxPXXXQ8cGfN9TPugCJWNQYSPsEBMm4gcOt+emgPPA0ZkJxK01t52abQsWA8jp9+buK4doX6sSQIBzTSHa6zNtoSxMnc5+21sVEi/OPjX8V54I1d03kzQXq0X4/xiCVFyJuin+gXEYe7/eIQ3xz40Ihk9YUCt6COp6b8eQ+VOYiH+wEqBNB34NQlqDKYGmHymOqSV4YR/CQMtWf0j+bsC1F+2k8Em5NZdNMz7H30J5daGOIK+jgHU5K8u2TKFSGePtxMyOarVADFy4+ivdn9ezbdLqKG+0p4jjM+O4I2VO/d7kdJSBipg3fc4Bcbq5Z5D+QTtDahHJjUv54urJCxDswHzG8bbo3mgdKI5QSagJpsX7BX9ZC02UPaG6ixUicRDxL9esBJrVi/Oc1j5OFATsFEsZ4bSQkDKWEoxTZGiO1IBjXQX3V3jQ5P2UUQG5F6vw5tmA7aDDAdRgsqXOQcQu31LAfEzOfcO5up1XXomQBJl5vuHtyYxRjY2ZE2Q= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, 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 Thu, Aug 8, 2024 at 2:02=E2=80=AFPM Shakeel Butt wrote: > > On Thu, Aug 08, 2024 at 01:15:52PM GMT, Andrii Nakryiko wrote: > > On Thu, Aug 8, 2024 at 11:40=E2=80=AFAM Shakeel Butt wrote: > > > > > > On Wed, Aug 07, 2024 at 04:40:25PM GMT, Andrii Nakryiko wrote: > > > > Extend freader with a flag specifying whether it's OK to cause page > > > > fault to fetch file data that is not already physically present in > > > > memory. With this, it's now easy to wait for data if the caller is > > > > running in sleepable (faultable) context. > > > > > > > > We utilize read_cache_folio() to bring the desired folio into page > > > > cache, after which the rest of the logic works just the same at fol= io level. > > > > > > > > Suggested-by: Omar Sandoval > > > > Cc: Shakeel Butt > > > > Cc: Johannes Weiner > > > > Signed-off-by: Andrii Nakryiko > > > > --- > > > > lib/buildid.c | 44 ++++++++++++++++++++++++++++---------------- > > > > 1 file changed, 28 insertions(+), 16 deletions(-) > > > > > > > > diff --git a/lib/buildid.c b/lib/buildid.c > > > > index 5e6f842f56f0..e1c01b23efd8 100644 > > > > --- a/lib/buildid.c > > > > +++ b/lib/buildid.c > > > > @@ -20,6 +20,7 @@ struct freader { > > > > struct folio *folio; > > > > void *addr; > > > > loff_t folio_off; > > > > + bool may_fault; > > > > }; > > > > struct { > > > > const char *data; > > > > @@ -29,12 +30,13 @@ struct freader { > > > > }; > > > > > > > > static void freader_init_from_file(struct freader *r, void *buf, u= 32 buf_sz, > > > > - struct address_space *mapping) > > > > + struct address_space *mapping, boo= l may_fault) > > > > { > > > > memset(r, 0, sizeof(*r)); > > > > r->buf =3D buf; > > > > r->buf_sz =3D buf_sz; > > > > r->mapping =3D mapping; > > > > + r->may_fault =3D may_fault; > > > > } > > > > > > > > static void freader_init_from_mem(struct freader *r, const char *d= ata, u64 data_sz) > > > > @@ -63,6 +65,11 @@ static int freader_get_folio(struct freader *r, = loff_t file_off) > > > > freader_put_folio(r); > > > > > > > > r->folio =3D filemap_get_folio(r->mapping, file_off >> PAGE_S= HIFT); > > > > + > > > > + /* if sleeping is allowed, wait for the page, if necessary */ > > > > + if (r->may_fault && (IS_ERR(r->folio) || !folio_test_uptodate= (r->folio))) > > > > + r->folio =3D read_cache_folio(r->mapping, file_off >>= PAGE_SHIFT, NULL, NULL); > > > > > > Willy's network fs comment is bugging me. If we pass NULL for filler, > > > the kernel will going to use fs's read_folio() callback. I have check= ed > > > read_folio() for fuse and nfs and it seems like for at least these tw= o > > > filesystems the callback is accessing file->private_data. So, if the = elf > > > file is on these filesystems, we might see null accesses. > > > > > > > Isn't that just a huge problem with the read_cache_folio() interface > > then? That file is optional, in general, but for some specific FS > > types it's not. How generic code is supposed to know this? > > > > Or maybe it's a bug with the nfs_read_folio() and fuse_read_folio() > > implementation that they can't handle NULL file argument? > > netfs_read_folio(), for example, seems to be working with file =3D=3D N= ULL > > just fine. > > If you go a bit down in netfs_alloc_request() there is the following > code: > > if (rreq->netfs_ops->init_request) { > ret =3D rreq->netfs_ops->init_request(rreq, file); > ... > ... > > I think this init_request is pointing to nfs_netfs_init_request which > calls nfs_file_open_context(file) and access filp->private_data. That's "nfs", which we know requires a file. For netfs implementations (cifs_init_request() and v9fs_init_request()), they both treat file as optional consistently. But regardless, that's just pointless code archeology, I'll just pass the file reference unconditionally. > > > > > Matthew, can you please advise what's the right approach here? I can, > > of course, always get file refcount, but most of the time it will be > > just an unnecessary overhead, so ideally I'd like to avoid that. But > > if I have to check each read_folio callback implementation to know > > whether it's required or not, then that's not great... > > I don't think we will need file refcnt. We have mmap lock in read mode > in this context because we are accessing vma and this vma has reference > to the file. So, this file can not go away under us here. Yep, good point, then it's not a problem, thanks! Will update.