---------- Forwarded message ---------
From: David CARLIER <devne...@gmail.com>
Date: Wed, 24 Apr 2024 at 07:51
Subject: Re: [PATCH] MEDIUM: shctx naming shared memory context
To: Willy Tarreau <w...@1wt.eu>
Hi
On Wed, 24 Apr 2024 at 07:45, Willy Tarreau <w...@1wt.eu> wrote:
> Hi David,
>
> On Sat, Apr 20, 2024 at 07:33:16AM +0100, David CARLIER wrote:
> > From d49d9d5966caead320f33f789578cb69f2aa3787 Mon Sep 17 00:00:00 2001
> > From: David Carlier <devne...@gmail.com>
> > Date: Sat, 20 Apr 2024 07:18:48 +0100
> > Subject: [PATCH] MEDIUM: shctx: Naming shared memory context
> >
> > From Linux 5.17, anonymous regions can be name via prctl/PR_SET_VMA
> > so caches can be identified when looking at HAProxy process memory
> > mapping.
>
> That's pretty cool, I wasn't aware of this feature, that's really
> interesting!
>
> However I disagree with this point:
>
> > +#if defined(USE_PRCTL) && defined(PR_SET_VMA)
> > + {
> > + /**
> > + * From Linux 5.17 (and if the `CONFIG_ANON_VMA_NAME`
> kernel config is set)`,
> > + * anonymous regions can be named.
> > + *
> > + * The naming can take up to 79 characters, accepting
> valid ASCII values
> > + * except [, ], \, $ and '.
> > + * As a result, when looking for /proc/<pid>/maps, we can
> see the anonymous range
> > + * as follow :
> > + * `7364c4fff000-736508000000 rw-s 00000000 00:01 3540
> [anon_shmem:HAProxy globalCache]`
> > + */
> > + int rn;
> > + char fullname[80];
> > +
> > + rn = snprintf(fullname, sizeof(fullname), "HAProxy %s",
> name);
> > + if (rn < 0 || prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME,
> (uintptr_t)shctx,
> > + totalsize, (uintptr_t)fullname) != 0) {
> > + munmap(shctx, totalsize);
> > + shctx = NULL;
> > + ret = SHCTX_E_ALLOC_CACHE;
> > + goto err;
> > + }
> > + }
> > +#endif
>
> You're making the mapping fail on any kernel that does not support the
> feature (hence apparently anything < 5.17 according to your description).
> IMHO we should just silently fall back to the default behavior,
Usually when using this feature, it is what is usually done indeed. However,
I thought with HAProxy you would prefer this behavior I guess I was wrong
:)
Ok changing it.
> i.e. we
> couldn't assign the name, fine, we continue to run without a name, thus
> something like this:
>
> rn = snprintf(fullname, sizeof(fullname), "HAProxy %s", name);
> if (rn >= 0)
> prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, (uintptr_t)shctx,
> totalsize, (uintptr_t)fullname);
>
> Could you please adjust it and verify that it's still OK for you ?
> Likewise I can check it on a 5.15 here.
>
> Thank you!
> Willy
>
