Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitfd49d53

Browse files
committed
Avoid potential spinlock in a signal handler as part of global barriers.
On platforms without support for 64bit atomic operations where we alsocannot rely on 64bit reads to have single copy atomicity, such atomicsare implemented using a spinlock based fallback. That means it's notsafe to even read such atomics from within a signal handler (since thesignal handler might run when the spinlock already is held).To avoid this issue defer global barrier processing out of the signalhandler. Instead of checking local / shared barrier generation todetermine whether to set ProcSignalBarrierPending, introducePROCSIGNAL_BARRIER and always set ProcSignalBarrierPending whenreceiving such a signal. Additionally avoid redundant work inProcessProcSignalBarrier if ProcSignalBarrierPending is unnecessarily.Also do a small amount of other polishing.Author: Andres FreundReviewed-By: Robert HaasDiscussion:https://postgr.es/m/20200609193723.eu5ilsjxwdpyxhgz@alap3.anarazel.deBackpatch: 13-, where the code was introduced.
1 parent2fd2eff commitfd49d53

File tree

2 files changed

+52
-36
lines changed

2 files changed

+52
-36
lines changed

‎src/backend/storage/ipc/procsignal.c

Lines changed: 51 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ SendProcSignal(pid_t pid, ProcSignalReason reason, BackendId backendId)
320320
uint64
321321
EmitProcSignalBarrier(ProcSignalBarrierTypetype)
322322
{
323-
uint64flagbit=UINT64CONST(1) << (uint64)type;
323+
uint32flagbit=1 << (uint32)type;
324324
uint64generation;
325325

326326
/*
@@ -363,7 +363,11 @@ EmitProcSignalBarrier(ProcSignalBarrierType type)
363363
pid_tpid=slot->pss_pid;
364364

365365
if (pid!=0)
366+
{
367+
/* see SendProcSignal for details */
368+
slot->pss_signalFlags[PROCSIG_BARRIER]= true;
366369
kill(pid,SIGUSR1);
370+
}
367371
}
368372

369373
returngeneration;
@@ -383,6 +387,8 @@ WaitForProcSignalBarrier(uint64 generation)
383387
{
384388
longtimeout=125L;
385389

390+
Assert(generation <=pg_atomic_read_u64(&ProcSignal->psh_barrierGeneration));
391+
386392
for (inti=NumProcSignalSlots-1;i >=0;i--)
387393
{
388394
volatileProcSignalSlot*slot=&ProcSignal->psh_slot[i];
@@ -417,6 +423,23 @@ WaitForProcSignalBarrier(uint64 generation)
417423
pg_memory_barrier();
418424
}
419425

426+
/*
427+
* Handle receipt of an interrupt indicating a global barrier event.
428+
*
429+
* All the actual work is deferred to ProcessProcSignalBarrier(), because we
430+
* cannot safely access the barrier generation inside the signal handler as
431+
* 64bit atomics might use spinlock based emulation, even for reads. As this
432+
* routine only gets called when PROCSIG_BARRIER is sent that won't cause a
433+
* lot fo unnecessary work.
434+
*/
435+
staticvoid
436+
HandleProcSignalBarrierInterrupt(void)
437+
{
438+
InterruptPending= true;
439+
ProcSignalBarrierPending= true;
440+
/* latch will be set by procsignal_sigusr1_handler */
441+
}
442+
420443
/*
421444
* Perform global barrier related interrupt checking.
422445
*
@@ -428,22 +451,38 @@ WaitForProcSignalBarrier(uint64 generation)
428451
void
429452
ProcessProcSignalBarrier(void)
430453
{
431-
uint64generation;
454+
uint64local_gen;
455+
uint64shared_gen;
432456
uint32flags;
433457

458+
Assert(MyProcSignalSlot);
459+
434460
/* Exit quickly if there's no work to do. */
435461
if (!ProcSignalBarrierPending)
436462
return;
437463
ProcSignalBarrierPending= false;
438464

439465
/*
440-
* Read the current barrier generation, and then get the flags that are
441-
* set for this backend. Note that pg_atomic_exchange_u32 is a full
442-
* barrier, so we're guaranteed that the read of the barrier generation
443-
* happens before we atomically extract the flags, and that any subsequent
444-
* state changes happen afterward.
466+
* It's not unlikely to process multiple barriers at once, before the
467+
* signals for all the barriers have arrived. To avoid unnecessary work in
468+
* response to subsequent signals, exit early if we already have processed
469+
* all of them.
470+
*/
471+
local_gen=pg_atomic_read_u64(&MyProcSignalSlot->pss_barrierGeneration);
472+
shared_gen=pg_atomic_read_u64(&ProcSignal->psh_barrierGeneration);
473+
474+
Assert(local_gen <=shared_gen);
475+
476+
if (local_gen==shared_gen)
477+
return;
478+
479+
/*
480+
* Get and clear the flags that are set for this backend. Note that
481+
* pg_atomic_exchange_u32 is a full barrier, so we're guaranteed that the
482+
* read of the barrier generation above happens before we atomically
483+
* extract the flags, and that any subsequent state changes happen
484+
* afterward.
445485
*/
446-
generation=pg_atomic_read_u64(&ProcSignal->psh_barrierGeneration);
447486
flags=pg_atomic_exchange_u32(&MyProcSignalSlot->pss_barrierCheckMask,0);
448487

449488
/*
@@ -466,7 +505,7 @@ ProcessProcSignalBarrier(void)
466505
* things have changed further, it'll get fixed up when this function is
467506
* next called.
468507
*/
469-
pg_atomic_write_u64(&MyProcSignalSlot->pss_barrierGeneration,generation);
508+
pg_atomic_write_u64(&MyProcSignalSlot->pss_barrierGeneration,shared_gen);
470509
}
471510

472511
staticvoid
@@ -505,27 +544,6 @@ CheckProcSignal(ProcSignalReason reason)
505544
return false;
506545
}
507546

508-
/*
509-
* CheckProcSignalBarrier - check for new barriers we need to absorb
510-
*/
511-
staticbool
512-
CheckProcSignalBarrier(void)
513-
{
514-
volatileProcSignalSlot*slot=MyProcSignalSlot;
515-
516-
if (slot!=NULL)
517-
{
518-
uint64mygen;
519-
uint64curgen;
520-
521-
mygen=pg_atomic_read_u64(&slot->pss_barrierGeneration);
522-
curgen=pg_atomic_read_u64(&ProcSignal->psh_barrierGeneration);
523-
return (mygen!=curgen);
524-
}
525-
526-
return false;
527-
}
528-
529547
/*
530548
* procsignal_sigusr1_handler - handle SIGUSR1 signal.
531549
*/
@@ -546,6 +564,9 @@ procsignal_sigusr1_handler(SIGNAL_ARGS)
546564
if (CheckProcSignal(PROCSIG_WALSND_INIT_STOPPING))
547565
HandleWalSndInitStopping();
548566

567+
if (CheckProcSignal(PROCSIG_BARRIER))
568+
HandleProcSignalBarrierInterrupt();
569+
549570
if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_DATABASE))
550571
RecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_DATABASE);
551572

@@ -564,12 +585,6 @@ procsignal_sigusr1_handler(SIGNAL_ARGS)
564585
if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN))
565586
RecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN);
566587

567-
if (CheckProcSignalBarrier())
568-
{
569-
InterruptPending= true;
570-
ProcSignalBarrierPending= true;
571-
}
572-
573588
SetLatch(MyLatch);
574589

575590
latch_sigusr1_handler();

‎src/include/storage/procsignal.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ typedef enum
3333
PROCSIG_NOTIFY_INTERRUPT,/* listen/notify interrupt */
3434
PROCSIG_PARALLEL_MESSAGE,/* message from cooperating parallel backend */
3535
PROCSIG_WALSND_INIT_STOPPING,/* ask walsenders to prepare for shutdown */
36+
PROCSIG_BARRIER,/* global barrier interrupt */
3637

3738
/* Recovery conflict reasons */
3839
PROCSIG_RECOVERY_CONFLICT_DATABASE,

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp