public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tomas Vondra <[email protected]>
To: Tom Lane <[email protected]>
To: Robert Haas <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Rahila Syed <[email protected]>
Subject: Re: pgsql: Add function to get memory context stats for processes
Date: Sat, 26 Apr 2025 23:07:46 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<CA+Tgmoaey-kOP1k5FaUnQFd1fR0majVebWcL8ogfLbG_nt-Ytg@mail.gmail.com>
<zppwd3scfznxhptdwm765l73wttegi5mibaxyxdwihlmzg3khs@fdhzkq3uxyct>
<CA+Tgmob7yyONbdv-DRRrWxQPL9AsXFxzs3OH1F6vxSc14prLFA@mail.gmail.com>
<rkbjmh4fipn5wzf4kqnge3p2szvhd7kgxgccoho5dtntojmkvt@uppvdxpywiba>
<CA+TgmoZgOwiF1auY+wxUBC+7GcjKqXFBf7D97sF4WL7HER4LgA@mail.gmail.com>
<[email protected]>
<CA+Tgmob1HFe-EbDmLs-xoQQfWAj8aB_cPToW-OOfhs4mFLfizg@mail.gmail.com>
<[email protected]>
On 4/23/25 20:14, Tom Lane wrote:
> Robert Haas <[email protected]> writes:
>> My primary concern about the patch is that
>> ProcessGetMemoryContextInterrupt() can be called from any
>> CHECK_FOR_INTERRUPTS() and calls lots of DSA functions, including
>> dsa_create() and, via PublishMemoryContext(), dsa_allocate0(). I'm
>> shocked to hear that you and Andres think that's safe to do at any
>> current or future CHECK_FOR_INTERRUPTS() anywhere in the code; but
>> Andres seems very confident that it's fine, so perhaps I should just
>> stop worrying and be happy that we have the feature.
>
> Just for the record, it sounds quite unsafe to me too. I could
> credit it being all right to examine the process' MemoryContext data
> structures, but calling dsa_create() from CFI seems really insane.
> Way too many moving parts there.
>
Maybe I'm oblivious to some very obvious issues, but why is this so
different from everything else that is already called from
ProcessInterrupts()? Perhaps dsa_create() is more complex compared to
the other stuff (haven't checked), but why would it be unsafe?
The one risk I can think of is a risk of recursion - if any of the
functions called from ProcessGetMemoryContextInterrupt() does CFI, could
that be a problem if there's another pending signal. I see some other
handlers (e.g. ProcessParallelApplyMessages) handle this by explicitly
holding interrupts. Should ProcessGetMemoryContextInterrupt() do the
same thing?
In any case, if DSA happens to not be the right way to transfer this,
what should we use instead? The only thing I can think of is some sort
of pre-allocated chunk of shared memory.
regards
--
Tomas Vondra
view thread (6+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: pgsql: Add function to get memory context stats for processes
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox