Date: Wed, 14 Apr 2021 11:35:48 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: Jessica Clarke <jrtc27@freebsd.org> Cc: "src-committers@freebsd.org" <src-committers@freebsd.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@freebsd.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@freebsd.org> Subject: Re: git: 97ed4babb516 - main - zfs: avoid memory allocation in arc_prune_async Message-ID: <CAGudoHFfbFnnJQ48U4aoMxr9zVqCyH=DC-YsSfnEjJhYLOs8Aw@mail.gmail.com> In-Reply-To: <6F4745E7-CC46-4060-99A1-90C2A691EBC7@freebsd.org> References: <202104110643.13B6h91E076304@gitrepo.freebsd.org> <6F4745E7-CC46-4060-99A1-90C2A691EBC7@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 4/11/21, Jessica Clarke <jrtc27@freebsd.org> wrote: > On 11 Apr 2021, at 07:43, Mateusz Guzik <mjg@FreeBSD.org> wrote: >> >> The branch main has been updated by mjg: >> >> URL: >> https://cgit.FreeBSD.org/src/commit/?id=3D97ed4babb51636d8a4b11bc7b207c3= 219ffcd0e3 >> >> commit 97ed4babb51636d8a4b11bc7b207c3219ffcd0e3 >> Author: Mateusz Guzik <mjg@FreeBSD.org> >> AuthorDate: 2021-04-11 05:15:41 +0000 >> Commit: Mateusz Guzik <mjg@FreeBSD.org> >> 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 <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHFfbFnnJQ48U4aoMxr9zVqCyH=DC-YsSfnEjJhYLOs8Aw>