From owner-dev-commits-src-main@freebsd.org Wed Apr 14 09:35:52 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 4C3D25D0E05; Wed, 14 Apr 2021 09:35:52 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-lj1-x231.google.com (mail-lj1-x231.google.com [IPv6:2a00:1450:4864:20::231]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4FKy5M2jPNz4mgW; Wed, 14 Apr 2021 09:35:51 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-lj1-x231.google.com with SMTP id a25so9265871ljm.11; Wed, 14 Apr 2021 02:35:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=7QWtvOpdToDxK30UTF0nOasU55mTECq6rSwaPnt09sc=; b=Ke94yYTbbEJlOsOG+84aXd18JVhhuTS88j5XFSUFA3lwFEtzRL92xxgDGqU/HTxGNS oyVFSNHfwX7Ut5dWPhcSHBnhBH4FQ7Tiqfcd5O5N49fPUHkx7leNpG8m29Hahxyay9rd qW1PiJzXIopvm7nM9c7Oo3FwLPZ8Y2dnY+1hyesI/4oWo9vtimtsufYueF1kJ8hKuUIH acV7goEJ+DeLIjXRpFrYbHXBEhyrr9fTIJdaUI33nFHSs2sIRzR2DLva7B/ocZYLxHIb g2hevjQDd3FYEB3IzhyPngrFZmjfvcw/TveWk4Hwg4dunx0F21CUw6t0KyV7XtKn7jIK MO1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=7QWtvOpdToDxK30UTF0nOasU55mTECq6rSwaPnt09sc=; b=Dw5PT0XdRty7OYIyYBqxNSNoKhEZ3GJjmABdBoPb6E8uYJ+btWBCg4/kg8Wgg62fb4 Fz1VhPWVAIIr68N2hxoV13afM0bw47O94wjY+Y8GwxQw8Gqd2KzWAPbJmUzJDDnxkO4T Cx/F3J/7GKMKp0UmBzKTO5EuSx8U0Y+75Z9ywyGcSe9ME7OoznR6H6eatI5JjjqVVbTE vs4jaorBhkngpJDZ12hIpk7WDFmSaGnBWJ58H9v7ImxhQ8ScoATrEvJ104kSxV4vtRLt V7sDAr/Dfj1cD21WOJ5fTd3dZF676GGieOLk9gtMlU7rPDbI3Z5GxzLKkZuZMhdY7Mel ZPWw== X-Gm-Message-State: AOAM532fKF+j5SyiYj1opGkDjKimQwbLtOn9GU2SJQszMFyoMM9K599E RPbqa32HF431nQWFe+EMxFzVY3vybEiIxO5gTdpR7Aq+ X-Google-Smtp-Source: ABdhPJxskxTvOfgHD3Zt8KGRJcp6sEFHJIteiP4nebO4M8S9LRdOMM3cECGxSA2AXmkzAMYelDqK6NptYHoux4azcDE= X-Received: by 2002:a2e:7807:: with SMTP id t7mr23651566ljc.313.1618392949481; Wed, 14 Apr 2021 02:35:49 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a2e:1405:0:0:0:0:0 with HTTP; Wed, 14 Apr 2021 02:35:48 -0700 (PDT) In-Reply-To: <6F4745E7-CC46-4060-99A1-90C2A691EBC7@freebsd.org> References: <202104110643.13B6h91E076304@gitrepo.freebsd.org> <6F4745E7-CC46-4060-99A1-90C2A691EBC7@freebsd.org> From: Mateusz Guzik Date: Wed, 14 Apr 2021 11:35:48 +0200 Message-ID: Subject: Re: git: 97ed4babb516 - main - zfs: avoid memory allocation in arc_prune_async To: Jessica Clarke Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 4FKy5M2jPNz4mgW X-Spamd-Bar: --- Authentication-Results: mx1.freebsd.org; dkim=pass header.d=gmail.com header.s=20161025 header.b=Ke94yYTb; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (mx1.freebsd.org: domain of mjguzik@gmail.com designates 2a00:1450:4864:20::231 as permitted sender) smtp.mailfrom=mjguzik@gmail.com X-Spamd-Result: default: False [-3.98 / 15.00]; TO_DN_EQ_ADDR_SOME(0.00)[]; TO_DN_SOME(0.00)[]; R_SPF_ALLOW(-0.20)[+ip6:2a00:1450:4000::/36:c]; FREEMAIL_FROM(0.00)[gmail.com]; DKIM_TRACE(0.00)[gmail.com:+]; DMARC_POLICY_ALLOW(-0.50)[gmail.com,none]; NEURAL_HAM_SHORT(-0.98)[-0.983]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RBL_DBL_DONT_QUERY_IPS(0.00)[2a00:1450:4864:20::231:from]; FREEMAIL_ENVFROM(0.00)[gmail.com]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US]; DWL_DNSWL_NONE(0.00)[gmail.com:dkim]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; R_DKIM_ALLOW(-0.20)[gmail.com:s=20161025]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[4]; TO_MATCH_ENVRCPT_ALL(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000]; MIME_GOOD(-0.10)[text/plain]; SPAMHAUS_ZRD(0.00)[2a00:1450:4864:20::231:from:127.0.2.255]; RCVD_IN_DNSWL_NONE(0.00)[2a00:1450:4864:20::231:from]; RCVD_COUNT_TWO(0.00)[2]; RCVD_TLS_ALL(0.00)[]; MAILMAN_DEST(0.00)[dev-commits-src-all,dev-commits-src-main] X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Apr 2021 09:35:52 -0000 On 4/11/21, Jessica Clarke wrote: > On 11 Apr 2021, at 07:43, Mateusz Guzik wrote: >> >> The branch main has been updated by mjg: >> >> URL: >> https://cgit.FreeBSD.org/src/commit/?id=3D97ed4babb51636d8a4b11bc7b207c3= 219ffcd0e3 >> >> commit 97ed4babb51636d8a4b11bc7b207c3219ffcd0e3 >> Author: Mateusz Guzik >> AuthorDate: 2021-04-11 05:15:41 +0000 >> Commit: Mateusz Guzik >> CommitDate: 2021-04-11 05:19:56 +0000 >> >> zfs: avoid memory allocation in arc_prune_async >> --- >> sys/contrib/openzfs/module/os/freebsd/zfs/arc_os.c | 16 ++++++++++------ >> 1 file changed, 10 insertions(+), 6 deletions(-) >> >> diff --git a/sys/contrib/openzfs/module/os/freebsd/zfs/arc_os.c >> b/sys/contrib/openzfs/module/os/freebsd/zfs/arc_os.c >> index 201dbc423336..e73efd810e53 100644 >> --- a/sys/contrib/openzfs/module/os/freebsd/zfs/arc_os.c >> +++ b/sys/contrib/openzfs/module/os/freebsd/zfs/arc_os.c >> @@ -158,10 +158,13 @@ arc_default_max(uint64_t min, uint64_t allmem) >> static void >> arc_prune_task(void *arg) >> { >> - int64_t nr_scan =3D *(int64_t *)arg; >> +#ifdef __LP64__ >> + int64_t nr_scan =3D (int64_t)arg; >> +#else >> + int64_t nr_scan =3D (int32_t)arg; >> +#endif > > int64_t nr_scan =3D (intptr_t)arg;? > >> arc_reduce_target_size(ptob(nr_scan)); >> - free(arg, M_TEMP); >> #if __FreeBSD_version >=3D 1300139 >> sx_xlock(&arc_vnlru_lock); >> vnlru_free_vfsops(nr_scan, &zfs_vfsops, arc_vnlru_marker); >> @@ -185,13 +188,14 @@ arc_prune_task(void *arg) >> void >> arc_prune_async(int64_t adjust) >> { >> - >> int64_t *adjustptr; >> >> - if ((adjustptr =3D malloc(sizeof (int64_t), M_TEMP, M_NOWAIT)) =3D=3D = NULL) >> - return; >> +#ifndef __LP64__ >> + if (adjust > __LONG_MAX) >> + adjust =3D __LONG_MAX; >> +#endif > > The code is correct without the ifdef, is this just to suppress a > tautological > condition warning? If so, be precise in your condition and use __INT64_MA= X__ >> > __LONG_MAX__? LP64 conflates a lot of things into one variable, and so > isn=E2=80=99t > defined for CHERI, but IMO it=E2=80=99s better to be precise *anyway* bec= ause then > it=E2=80=99s > more self-documenting why the #if is there. > ifdef is there to not rise an eye-brow for LP64. I agree __LP64__ is overloaded, but I used it to be in line with the rest of the codebase. Whatever better idiomatic way shows up (e.g., introduce LP32 to ifdef on instead?) this place will be easy to find with grep. That said, I have no strong opinion how this should look like, apart from not being just slapped in for LP64. --=20 Mateusz Guzik