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 A3BB3C3DA4A for ; Thu, 8 Aug 2024 20:58:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 36B356B0089; Thu, 8 Aug 2024 16:58:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 31AAF6B008A; Thu, 8 Aug 2024 16:58:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 20AB16B008C; Thu, 8 Aug 2024 16:58:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 047766B0089 for ; Thu, 8 Aug 2024 16:58:27 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id A62D41A157D for ; Thu, 8 Aug 2024 20:58:27 +0000 (UTC) X-FDA: 82430291454.30.3ABEFC8 Received: from mail-ed1-f44.google.com (mail-ed1-f44.google.com [209.85.208.44]) by imf10.hostedemail.com (Postfix) with ESMTP id CA997C0025 for ; Thu, 8 Aug 2024 20:58:24 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=rhEx6lSI; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf10.hostedemail.com: domain of jannh@google.com designates 209.85.208.44 as permitted sender) smtp.mailfrom=jannh@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723150695; a=rsa-sha256; cv=none; b=ZuCpej8fiInQatHxizSguO8nl35vS8U2uSRcfq6jme6SqsLucZZ4Desb+8D5Ps8+jpi7/E uMuCaaRA5C7jpmYXJVIHbbf2z/iWemImkOD2jmRWACeDalHJc1suarDrmxndjzYdK+1JvI oT6kdew/TceDVTjvaZmYvBx0a4LtjqQ= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=rhEx6lSI; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf10.hostedemail.com: domain of jannh@google.com designates 209.85.208.44 as permitted sender) smtp.mailfrom=jannh@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723150695; 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=pegms3PnshsXX1UPyKLgluxFEHdbFiIeTyjYj44EsqA=; b=TBMf4I6SxC0oDmMDKTjjFs9SKyALIyv1VezrOQVKM09FHfc9fIGvy4vJw4/D7Kjgy1Eqm8 GhHtV8oFVudCZa+EqjMt7hfrEIB7eF3BiEwAZelgWORfekvxHsoXT5lZXEy5wM3E2h5gkB V+Vq+mZwRxPuvJESwwC0agM5ppyVVK4= Received: by mail-ed1-f44.google.com with SMTP id 4fb4d7f45d1cf-5a28b61b880so3830a12.1 for ; Thu, 08 Aug 2024 13:58:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1723150703; x=1723755503; 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=pegms3PnshsXX1UPyKLgluxFEHdbFiIeTyjYj44EsqA=; b=rhEx6lSIzBh8AxkuUz4WTb4Wv64AcnR1NEeJ9h7ST+osABn+Y6TrKIILxFgocHSHKz EM6geefU/Z7HkQdVey91SVZIfsSc9ikBT6albT2Esrn5XRDqdD5kn3qDF4Y8e4xcLyRU lxXIFpehHZBLMnH5U0462AgdjbaDEWFYAwaMLfr58tY0ZFBf4kQyz4WmmvWeiVEKyCol 8bGsPrVJPHbl7MT6e+QXJ9+ru7iF3oUBnYwOF5IZo2uLfWyvx1uwRc84xVP1yUaBIcv3 xqPOIec3kjg7zpHbt7rO5SrQzQU7+KM+l9Puw5huY88Kcf0lXTxiKb+mYCYaE36W9C9c T24Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723150703; x=1723755503; 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=pegms3PnshsXX1UPyKLgluxFEHdbFiIeTyjYj44EsqA=; b=rOGlkRFi+x0rPMTVZjXweuEnu3uFanCZeYNcdI0fZCezeEMGt/Tdjar9VNu4r4Vf6K QEjS9qESv870quuCl9M1xkkxKOfVvLT5LzFWe5mfuvPdYOuPGWzzfsqoFU1oAyEiOGHy L1AjurpFGYV9Whpxsj7NhN51isrINj7F9Gu2c6+IOKl/N2HRlNtSwhJiR4HnP8YUUxxo j2WH/2AtLglww9o+tpwMlFTGXNbUiMAiNgV/N8f54aaT7QQhbYRgbhvBtmssFSPRm0A2 2xbd3tmErkpSbGb9yYgHW23BjisCvrIUfXQTYw6lHn57XZpXJ0UGD96GwMP2aWbMMrbv Uquw== X-Forwarded-Encrypted: i=1; AJvYcCWfwPhfTsqUfuk6tQ4cVfWh5bi/kqqapvItFjPKJGydcPhVvdPUI+pCZ4RV4aVVp753aGaLMJjw5OtabhMBUMZccLw= X-Gm-Message-State: AOJu0YxDQO5MzD0Yk5Ledab47nkUV8jK3X5t4mDCOIZNVI7BrWTdvzca fHoX7t/UgJqZ1nhWYpLoMEjFyqLKX2D4VnajM1KxZZ2UUmmAEVrRlX8dR4vuMjLPF90tJ/is0eB IgGYp+ltcGNVuPyTJJbvYVb70SYCS9kLxOPEQ X-Google-Smtp-Source: AGHT+IH8aF37EKz07qcuHYbuMvchUS9oPhD+wlLA9h7t5l//hLbBFSnsHQZjKvgmKTW7NxWgePGWOkwaJt1Vy/kNbxk= X-Received: by 2002:a05:6402:2550:b0:58b:b1a0:4a2d with SMTP id 4fb4d7f45d1cf-5bc4b3fd4f9mr10721a12.1.1723150702346; Thu, 08 Aug 2024 13:58:22 -0700 (PDT) MIME-Version: 1.0 References: <20240807234029.456316-1-andrii@kernel.org> <20240807234029.456316-7-andrii@kernel.org> In-Reply-To: From: Jann Horn Date: Thu, 8 Aug 2024 22:57:43 +0200 Message-ID: Subject: Re: [PATCH v4 bpf-next 06/10] lib/buildid: implement sleepable build_id_parse() API To: Andrii Nakryiko Cc: Shakeel Butt , 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, 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: CA997C0025 X-Rspamd-Server: rspam01 X-Stat-Signature: 9khfmf9gaern7hhxxho65kgrjp1h6qra X-HE-Tag: 1723150704-547293 X-HE-Meta: U2FsdGVkX1+9ktdxiSYoFFJlmh7ehWfZayvCAzY4dBMyl/SS6Kw/um2sUeStMJ401wo2c0dx3XyEuEGDM8foF+BaJsaBmrRKqZNmG1B5g97TT1TgFI+GJRzOBYUs3yWCANeetEZ2W3MLiOv3X2eojTTYm/Btwkx9AjmB/wYFirqgtZ8/mUz/3riM+ttVIt0DoyCXlL1fQS7SdYk95sJtqsKQa9BpFPpURa/JS9gQ99yHjBgccfrZ/sYDClguu3cohkuT448bGqdWzAtvOjc/wKtpj52QGs8VnfB1bOaFdB7mmTjDabozxjxDht7ySPBU3udOxN5OGmwDLbTEcqr2FLvsfhYEFKzp+yi2hot4rV/uD++ZQo0pjMWyhygcQZ+XJhPFnEBW8xUx2saWqoQ5OU0aM4Dx+9zEggM5jf16GFvgZBYsxA5jgxrITtjFhb1PkBM6JBHVnULVt6z0zRcK7K5rG39g/Nf01w1fLrIyMcrs4gLmzpwYCkJmqkvE/fatCjhvkP6xE8b7bodythmZ6dxHxuBQsyQt8Pvr4XtT79uuYimX+TGXUsKjlIrZ3RIQNM+LxT2MUK4S62K59yJQjnVdey+5/+ltGASEeAfvyrk+BfZp1EsAg61wtdfqo6dBV8+lDXlhuorqqaLNjB2V7BIugA403nkNCV/kUWoqNcu96bavgGioAE4BLEOgS3Gx1wj2nHbP2s9tCCkfRxQI4MnOfvzm7g+m85P/Ny7kbbJ6zuuQbaYf2UDC7BRR0ToNQwAj5ZsV5DPQD78MczednDD1ti5/v0RxZpaAtZ+vbCKPYTfMtIWCaKpA/JxBrTnYhtTZUQHM8RBKHExN+k4BGdYz3fkZN2XLaV+VaaNjDc6GwvXHl1aQs/1kGsj7bKgTulcwqnktauF4yakPahwwS+1qT9mdc91iw956zX2rgScNnUn0DSkPXkTeOBp8rK7yGRaQOHGrHliNBU6XoaO mSG4S69Y HascJ5+QsSu5iCR8TNwMW0t7AmDc8jetqK+ZydmT9pKL5KCT5OBxRqhUONd3YKpPd11XAmyXg8niwRARYotdCo9DrZoLxZb9w1LNiV1MU4+1qildjb4IAix8wP2XIrDWUvoR/lbRYF6nXbECenDhmLAY59CRMbp+Rnp8xKizLgxLL3ovs2NKtgLOlg/PT+zjY8ba6m88ETei2IfUfEBeSvwZ3nFPjF41mbmJ7wU1iv/H+6JH4ihSemqzwoZawIDm5tSoWo8t9U3ETz4Xn/xMiLo4tusqhphqQ+vYEmU/sjtVc8ErKJL8TYc8JhOQKlzWDN3sBOPJN/mibojm9Fu42yr3+foqUs7YoBbhiiTAOMq98HNaIVDhIiuh5xrtjeSE0FO852S4oM/6gexMi2GPezv0rOgIt6nvPVQYCc5L72MsgBhAmwxY3jWSCIjAks/Vqeh7Th7A6LKFk4fdRZFYx7EkxmJcJ3RIbrsf1CkeGvBUU0cI= 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 10:16=E2=80=AFPM 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 folio= 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, u32= buf_sz, > > > - struct address_space *mapping) > > > + struct address_space *mapping, bool = 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 *dat= a, u64 data_sz) > > > @@ -63,6 +65,11 @@ static int freader_get_folio(struct freader *r, lo= ff_t file_off) > > > freader_put_folio(r); > > > > > > r->folio =3D filemap_get_folio(r->mapping, file_off >> PAGE_SHI= FT); > > > + > > > + /* 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 >> P= AGE_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 checked > > read_folio() for fuse and nfs and it seems like for at least these two > > filesystems the callback is accessing file->private_data. So, if the el= f > > 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? I think you have to think about it the other way around. The file is required, unless you know the filler function that will be used doesn't use the file. Which you don't know when you're coming from generic code, so generic code has to pass in a file. As far as I can tell, most of the callers of read_cache_folio() (via read_mapping_folio()) are inside filesystem implementations, not generic code, so they know what the filler function will do. You're generic code, so I think you have to pass in a file. > 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 NUL= L > just fine. > > 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... Why would you need to increment the file refcount? As far as I can tell, all your accesses to the file would happen under __build_id_parse(), which is borrowing the refcounted reference from vma->vm_file; the file can't go away as long as your caller is holding the mmap lock.