Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit a7a7387

Browse files
committed
Further improve code for probing the availability of ARM CRC instructions.
Andrew Gierth pointed out that commit 1c72ec6 would yield the wrong answer on big-endian ARM systems, because the data being CRC'd would be different. To fix that, and avoid the rather unsightly hard-wired constant, simply compare the hardware and software implementations' results. While we're at it, also log the resulting decision at DEBUG1, and error out if the hw and sw results unexpectedly differ. Also, since this file must compile for both frontend and backend, avoid incorrect dependencies on backend-only headers. In passing, add a comment to postmaster.c about when the CRC function pointer will get initialized. Thomas Munro, based on complaints from Andrew Gierth and Tom Lane Discussion: https://postgr.es/m/HE1PR0801MB1323D171938EABC04FFE7FA9E3110@HE1PR0801MB1323.eurprd08.prod.outlook.com
1 parent 30c66e7 commit a7a7387

File tree

2 files changed

+39
-8
lines changed

2 files changed

+39
-8
lines changed

src/backend/postmaster/postmaster.c

+9-1
Original file line numberDiff line numberDiff line change
@@ -957,7 +957,15 @@ PostmasterMain(int argc, char *argv[])
957957
*/
958958
CreateDataDirLockFile(true);
959959

960-
/* read control file (error checking and contains config) */
960+
/*
961+
* Read the control file (for error checking and config info).
962+
*
963+
* Since we verify the control file's CRC, this has a useful side effect
964+
* on machines where we need a run-time test for CRC support instructions.
965+
* The postmaster will do the test once at startup, and then its child
966+
* processes will inherit the correct function pointer and not need to
967+
* repeat the test.
968+
*/
961969
LocalProcessControlFile(false);
962970

963971
/*

src/port/pg_crc32c_armv8_choose.c

+30-7
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,15 @@
1818
*-------------------------------------------------------------------------
1919
*/
2020

21-
#include "c.h"
21+
#ifndef FRONTEND
22+
#include "postgres.h"
23+
#else
24+
#include "postgres_fe.h"
25+
#endif
2226

2327
#include <setjmp.h>
28+
#include <signal.h>
2429

25-
#include "libpq/pqsignal.h"
2630
#include "port/pg_crc32c.h"
2731

2832

@@ -33,7 +37,7 @@ static sigjmp_buf illegal_instruction_jump;
3337
* isn't available, we expect to get SIGILL, which we can trap.
3438
*/
3539
static void
36-
illegal_instruction_handler(int signo)
40+
illegal_instruction_handler(SIGNAL_ARGS)
3741
{
3842
siglongjmp(illegal_instruction_jump, 1);
3943
}
@@ -42,16 +46,35 @@ static bool
4246
pg_crc32c_armv8_available(void)
4347
{
4448
uint64 data = 42;
45-
bool result;
49+
int result;
4650

51+
/*
52+
* Be careful not to do anything that might throw an error while we have
53+
* the SIGILL handler set to a nonstandard value.
54+
*/
4755
pqsignal(SIGILL, illegal_instruction_handler);
4856
if (sigsetjmp(illegal_instruction_jump, 1) == 0)
49-
result = (pg_comp_crc32c_armv8(0, &data, sizeof(data)) == 0xdd439b0d);
57+
{
58+
/* Rather than hard-wiring an expected result, compare to SB8 code */
59+
result = (pg_comp_crc32c_armv8(0, &data, sizeof(data)) ==
60+
pg_comp_crc32c_sb8(0, &data, sizeof(data)));
61+
}
5062
else
51-
result = false;
63+
{
64+
/* We got the SIGILL trap */
65+
result = -1;
66+
}
5267
pqsignal(SIGILL, SIG_DFL);
5368

54-
return result;
69+
#ifndef FRONTEND
70+
/* We don't expect this case, so complain loudly */
71+
if (result == 0)
72+
elog(ERROR, "crc32 hardware and software results disagree");
73+
74+
elog(DEBUG1, "using armv8 crc32 hardware = %d", (result > 0));
75+
#endif
76+
77+
return (result > 0);
5578
}
5679

5780
/*

0 commit comments

Comments
 (0)