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 BD132C52D73 for ; Thu, 8 Aug 2024 21:02:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 395606B0089; Thu, 8 Aug 2024 17:02:41 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 31EAF6B008A; Thu, 8 Aug 2024 17:02:41 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1BF5A6B008C; Thu, 8 Aug 2024 17:02:41 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id EF6A76B0089 for ; Thu, 8 Aug 2024 17:02:40 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 9A727140BBC for ; Thu, 8 Aug 2024 21:02:40 +0000 (UTC) X-FDA: 82430302080.08.0280A69 Received: from out-173.mta0.migadu.com (out-173.mta0.migadu.com [91.218.175.173]) by imf16.hostedemail.com (Postfix) with ESMTP id 37E54180026 for ; Thu, 8 Aug 2024 21:02:37 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=tFUB62D+; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf16.hostedemail.com: domain of shakeel.butt@linux.dev designates 91.218.175.173 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723150910; a=rsa-sha256; cv=none; b=7eRS1GVtUVsSq2kJUhRjeKICdSNsj8+TgXCREYGQMKiRgBTHO6opep1fTlCaqyZn8cw/pB FxNobKrSTuhIsx/42Q2JExHtljts1Mt/fzR0ZDTxP7Jz7gSi34VWSILZ3ERg0YIjoaUmcc OzvFPtSFIHzr9N+IYHrbLds1btuyJC4= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=tFUB62D+; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf16.hostedemail.com: domain of shakeel.butt@linux.dev designates 91.218.175.173 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723150910; 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=VbAPo9z8CAdum7Nxf7shmVZRkLoHtXDBnE81/6BCoLU=; b=YQDpSXRKgVj3ZrKM5kzRoXEIs7APYf1neG/0icAQ1CrYIrz1Zv+2Ib6G/oapVBvbJdDDuw qoD35nnpRyR5hOdgnLQd6qYL9PToPOq5/WbDXdjoIQ2ao6qLcGAdSmqnRPoTtkcCZ6J5C3 Hs04fX6lddObw5qjeYImnh+abGHhPYk= Date: Thu, 8 Aug 2024 14:02:28 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1723150955; h=from:from: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; bh=VbAPo9z8CAdum7Nxf7shmVZRkLoHtXDBnE81/6BCoLU=; b=tFUB62D+e4lq8dxNzUOPrno/cMFWL+pHnwIsSkKnGRZp4TS82cUe2MP+t8uMxSK44rpFco eWJxwwA+nZQuoEYGyuLByl2JtYnFPuFn/XrZe4FcoKPK5W2qPp56L6WdEIfsucLYecO85p J5Q5miMXAQ33kdvorfHR3VIK6HuiPEU= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Shakeel Butt To: Andrii Nakryiko 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 Subject: Re: [PATCH v4 bpf-next 06/10] lib/buildid: implement sleepable build_id_parse() API Message-ID: References: <20240807234029.456316-1-andrii@kernel.org> <20240807234029.456316-7-andrii@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Migadu-Flow: FLOW_OUT X-Rspamd-Queue-Id: 37E54180026 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: nizja7d3id5pb8ypoxiephda5fjda51t X-HE-Tag: 1723150957-412611 X-HE-Meta: U2FsdGVkX1+dyeiLwUZm5Tf14aLLPeYMgHgKHUD9BLSBCPlM4opkGZfl/KNnHOQUopxqg06omoh3pLpwpA2ocRagdkYhFG2vRfcJb84lp01PsBFU3AoZCo+3ou0RQenYsBwu6v7AJtnIy1Mv58wjUZ7iR7v6CDIFsJkLVWtDiRI/O/Qm9eG6lrfAq5+Czk/nkZNTccPCPJO028mXZUQuOW4zOSAjFE/TEyOEC5ATYmeaUpsrmrINtAQINxpO8qB/449Fm4n9JD1XBTeEQFr7eA40hKclj46riWxWcRimyTFC/8bFIWtXHIDheth3psqr3XwFBdSVKLguusZxPuAB5/yjWTNuO//ItLuyAs5Y+5KbzwdEGsrGkdR0wqc2I4Lken5ilHebgeSkOQr459V1ZM8Nb/OLduP/uL0m5pt/qBjyHgdEOHccxyxnypyeYoEsfONfX7gUl+YuT0pRM5AoEleA1jZlqmfVDqACvesSm3a/liOnN8hprtpwAxei21WLiexsiFSXMsO42hDz7zX2LpizFyhAJVzx3Ee/Njh4DfLk3BDM251J1aFR+1yY9N72Syf6euA43+vSr9wf0QPjAH/mzmLabTwBrz8OJHRQ9V+jeNa8PNlJa3W39AosNB3/KBtY+hXTYOsfNpho8nYB2tOQTqbe6Mp4/UqB6hTMmEfmRVHeft0HS+Pe1JEu7qzS88lFbWG5+EaD1vSxtbqrKs2zr/Q9AEQ4ja78jOhGWdltg/eHx3DpORP82KMU/5K5RqAtB0ZHb3lfs+e5FE3Us8jPaMD540F9HK3MEoQNnyop4OIeHPB394azd2KBb/5kY09ZcypCgGf45oQQvGgidbwqAkdEpeZGpaTxsAV5qzXuG+Sv7K1x7zqeSJNXVpmzQUBq/u3qkhGsjlcoNzVnnzUG1r4HoT34/8RNHCQOAKc8rbfdWdZvoTB0slKEYns1aOx8VwBJhJnyTjgyQIA Nj9x+9oG Dhp7UGMWDiko8JIkWUxrv3bz+nRyPZM+Ll8KW4WaqheSDEIDOb/PD05GpGG+5Q9q3f2eIqueIX6lHyWEsi3HWKQOJ9PIdkMWe23E5Qx28ArrMB8kzdyUfXGgCdCIrSevUIHGBLEeivUFS3uetgKkY1OP3RQ6ke+/IR04CtTHt0mBFetVGdrM7bWVhZnkHGIPCnEHQPI5yqJr9voFK6+MW8KZF5MullDmYZ6Q9tpsBPxrUhRjCyw5aQrUg9aVNuO1rT+qiB5sH+VFGZPldlzGD7ZM+zu2PVKjnM7uY7Pjy40LhpqibrURtaycsOOYMB4/1PtyVu+UMcniMuoXaE78eznC3btJ/lSD1C0ST1prp16eFVCqhZBpGDNyQ183WT0MDoR0cDbUylzFPfBT2DfIgW2hE9ytMAniBdxBfZu/o+4fDzsT8pfPwUh034hMmIp1K8e20 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 08, 2024 at 01:15:52PM GMT, Andrii Nakryiko wrote: > On Thu, Aug 8, 2024 at 11:40 AM 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 = buf; > > > r->buf_sz = buf_sz; > > > r->mapping = mapping; > > > + r->may_fault = may_fault; > > > } > > > > > > static void freader_init_from_mem(struct freader *r, const char *data, 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 = filemap_get_folio(r->mapping, file_off >> PAGE_SHIFT); > > > + > > > + /* 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 = 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 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 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 == NULL > just fine. If you go a bit down in netfs_alloc_request() there is the following code: if (rreq->netfs_ops->init_request) { ret = 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. > > 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.