Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
Andres Freund writes: > On 2017-05-31 11:57:16 -0400, Alvaro Herrera wrote: >> My vote would be to backpatch it all the way. > +1 Done, buildfarm seems happy. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
On 2017-05-31 11:57:16 -0400, Alvaro Herrera wrote: > My vote would be to backpatch it all the way. +1 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
Alvaro Herrera writes: > My vote would be to backpatch it all the way. That's my thought too. Otherwise it'll be five years before extension authors can stop worrying about this issue. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
Christoph Berg wrote: > Re: Tom Lane 2017-05-31 <28752.1496238...@sss.pgh.pa.us> > > Next question: should we back-patch this change, or just do it in HEAD? > > Debian "needs" it for 9.6, but I've already pushed the s390x patch in > the original posting, so I could just live with it being just in head. > But of course it would be nice to have everything in sync. I think it's only a problem for you in 9.6-only because you've not tried pglogical or some other large-shlib extension with earlier branches; in other words, eventually this is going to bite somebody using the old branches as well, unless we believe that the platforms are dead enough that nobody really cares other than for academic purposes. My vote would be to backpatch it all the way. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
Re: Tom Lane 2017-05-31 <28752.1496238...@sss.pgh.pa.us> > OK, this looks good to me. Just to make sure everyone's on the > same page, what I propose to do is simplify all our platform-specific > Makefiles that use either -fpic or -fPIC to use -fPIC unconditionally. > This affects the netbsd, linux, and openbsd ports. Looks like we should > also change the example link commands in dfunc.sgml. Ack. > Next question: should we back-patch this change, or just do it in HEAD? Debian "needs" it for 9.6, but I've already pushed the s390x patch in the original posting, so I could just live with it being just in head. But of course it would be nice to have everything in sync. Christoph -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
Christoph Berg writes: > Re: Tom Lane 2017-05-30 <1564.1496176...@sss.pgh.pa.us> >> It'd be interesting if people could gather similar numbers on other >> platforms of more real-world relevance, such as ppc64. But based on >> this small sample, I wouldn't object to just going to -fPIC across >> the board. > [ more numbers ] OK, this looks good to me. Just to make sure everyone's on the same page, what I propose to do is simplify all our platform-specific Makefiles that use either -fpic or -fPIC to use -fPIC unconditionally. This affects the netbsd, linux, and openbsd ports. Looks like we should also change the example link commands in dfunc.sgml. Next question: should we back-patch this change, or just do it in HEAD? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
Re: Tom Lane 2017-05-30 <1564.1496176...@sss.pgh.pa.us> > It'd be interesting if people could gather similar numbers on other > platforms of more real-world relevance, such as ppc64. But based on > this small sample, I wouldn't object to just going to -fPIC across > the board. ppc64el, Debian unstable: textdata bss dec hex filename -fpic: 79520 9281768 82216 14128 postgres_fdw.so -fPIC: 79520 9281768 82216 14128 postgres_fdw.so -> no change s390x, Debian unstable: textdata bss dec hex filename -fpic: 807352552 48 83335 14587 postgres_fdw.so -fPIC: 812472552 48 83847 14787 postgres_fdw.so -> +0.61% arm64, Debian unstable: textdata bss dec hex filename -fpic: 641302600 48 66778 104da postgres_fdw.so -fPIC: 642742600 48 66922 1056a postgres_fdw.so -> +0.22% sparc64, Debian unstable: textdata bss dec hex filename -fpic: 758043296 48 79148 1352c postgres_fdw.so -fPIC: 72748 920 48 73716 11ff4 postgres_fdw.so -> 6.9% decrease (!) 9.6.3, gcc (Debian 6.3.0-18) 6.3.0 20170516, -O2, all objects unstripped (sparc64 is gcc (Debian 6.3.0-17) 6.3.0 20170510) Christoph -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
On Tue, May 30, 2017 at 4:38 PM, Tom Lane wrote: > It'd be interesting if people could gather similar numbers on other > platforms of more real-world relevance, such as ppc64. But based on > this small sample, I wouldn't object to just going to -fPIC across > the board. That seems pretty sensible to me. I think we should aim for a configuration that works by default. If we use -fPIC where -fpic would have been better, the result is that on some platforms there might be a speed penalty, probably small. In the reverse situation, the build fails. I believe that the average PostgreSQL extension developer will prefer the first situation. If somebody has got an extension which is small enough to use -fpic and that person cares about minimizing the performance penalty on s390 or other platforms where they have this problem, then they can arrange an override in the opposite direction. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
I wrote: > Very possibly true, but I wish we had some hard facts and not just > guesses. As a simple but on-point test, I compared sizes of postgres_fdw.so built with -fpic and -fPIC. I no longer have access to a wide variety of weird architectures, but on what I do have in my office: x86_64, RHEL6: -fpic: textdata bss dec hex filename 854672632 64 88163 15863 postgres_fdw.so -fPIC: textdata bss dec hex filename 854672632 64 88163 15863 postgres_fdw.so This seems to confirm Andres' opinion that it makes no difference on x86_64. PPC, FreeBSD 10.3: -fpic: textdata bss dec hex filename 86638 420 32 87090 15432 postgres_fdw.so -fPIC: textdata bss dec hex filename 864741860 32 88366 1592e postgres_fdw.so that's a 1.47% penalty PPC, NetBSD 5.1.5: -fpic: textdata bss dec hex filename 81880 420 56 82356 141b4 postgres_fdw.so -fPIC: textdata bss dec hex filename 816882044 56 83788 1474c postgres_fdw.so that's a 1.74% penalty HPPA, HPUX 10.20: -fpic: textdata bss dec hex filename 97253 17296 8 114557 1bf7d postgres_fdw.sl -fPIC: textdata bss dec hex filename 102629 17320 8 119957 1d495 postgres_fdw.sl that's a 4.7% penalty It's somewhat noteworthy that the PPC builds show a large increase in data segment size. That likely corresponds to relocatable pointer fields that have to be massaged by the dynamic linker during shlib load. Thus one could speculate that shlib load might be noticeably slower with -fPIC, at least on PPC. But people rarely complain about the speed of that anyway, so it's unlikely to be worth worrying about. It'd be interesting if people could gather similar numbers on other platforms of more real-world relevance, such as ppc64. But based on this small sample, I wouldn't object to just going to -fPIC across the board. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
Re: Tom Lane 2017-05-30 <25131.1496163...@sss.pgh.pa.us> > Christoph Berg writes: > > My main point here would be that we are already setting this for all > > extensions for sparc and sparc64, so s390(x) would just follow suit. > > For some values of "we", sure ;-). Afaict for all values of "we": ifeq "$(findstring sparc,$(host_cpu))" "sparc" CFLAGS_SL = -fPIC else CFLAGS_SL = -fpic endif https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/makefiles/Makefile.linux Christoph -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
Christoph Berg writes: > My main point here would be that we are already setting this for all > extensions for sparc and sparc64, so s390(x) would just follow suit. For some values of "we", sure ;-). But I think what is really under discussion here is whether to change -fpic to -fPIC for all platforms. It looks like Makefile.netbsd and Makefile.openbsd would be affected along with Makefile.linux. Some other platforms such as freebsd are already in the fPIC-always camp. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
Re: Andres Freund 2017-05-30 <20170530161541.koj5xbvvovrrt...@alap3.anarazel.de> > I think we can fix this easily enough in Citus, postgis, and whatever. > But it's not a particularly good user/developer experience, and > presumably is going to become more and more common. On x86 there > shouldn't be a difference in efficiency between the two, but some others > will see some. Given that most distributions switched to building the > main executables with -fPIE anyway, to allow for ASLR, it seems unlikely > that the intra extension overhead is going to be very meaningful in > comparison. My main point here would be that we are already setting this for all extensions for sparc and sparc64, so s390(x) would just follow suit. -fPIC is the default in Debian now, see the discussion starting at https://lists.debian.org/debian-devel/2016/05/msg00028.html including the Fedora: https://lists.debian.org/debian-devel/2016/05/msg00219.html and Ubuntu: https://lists.debian.org/debian-devel/2016/05/msg00225.html situation, which all do that. Christoph -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
Andres Freund writes: > On 2017-05-29 15:45:11 -0400, Tom Lane wrote: >> Maybe this is small enough to not be something we need to worry about, >> but I'm wondering if we should ask citus and other large .so's to set >> some additional make flag that would cue usage of -fPIC over -fpic. > I think we can fix this easily enough in Citus, postgis, and whatever. > But it's not a particularly good user/developer experience, and > presumably is going to become more and more common. On x86 there > shouldn't be a difference in efficiency between the two, but some others > will see some. Given that most distributions switched to building the > main executables with -fPIE anyway, to allow for ASLR, it seems unlikely > that the intra extension overhead is going to be very meaningful in > comparison. Very possibly true, but I wish we had some hard facts and not just guesses. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
On 2017-05-29 15:45:11 -0400, Tom Lane wrote: > Christoph Berg writes: > > Re: To Andres Freund 2016-04-28 <20160428080824.ga22...@msg.df7cb.de> > >>> I'm not clear why citus triggers this, when other extensions don't? > > >> Maybe it's simply because citus.so is bigger than all the other > >> extension .so files: > > I wonder what the overhead is of using -fPIC when -fpic would be > sufficient. Whatever it is, the proposed patch imposes it on every > shlib or extension, to accommodate one single extension that isn't > even one we ship. > Maybe this is small enough to not be something we need to worry about, > but I'm wondering if we should ask citus and other large .so's to set > some additional make flag that would cue usage of -fPIC over -fpic. I think we can fix this easily enough in Citus, postgis, and whatever. But it's not a particularly good user/developer experience, and presumably is going to become more and more common. On x86 there shouldn't be a difference in efficiency between the two, but some others will see some. Given that most distributions switched to building the main executables with -fPIE anyway, to allow for ASLR, it seems unlikely that the intra extension overhead is going to be very meaningful in comparison. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
Robert Haas writes: > On Mon, May 29, 2017 at 3:45 PM, Tom Lane wrote: >> I wonder what the overhead is of using -fPIC when -fpic would be >> sufficient. > Do we have an idea how to measure the increased overhead? Just from > reading the description, I'm guessing that the increased cost would > happen when the extension calls back into core, but maybe that doesn't > happen often enough to worry about? My gut feeling is that it'd be a pretty distributed cost, because every internal cross-reference in the .so (for instance, loading the address of a string literal) would involve a bit more overhead to support a wider offset field. An easy thing to look at would be how much the code expands by. That might or might not be a good proxy for the runtime slowdown percentage, but it seems like it ought to serve as a zero-order approximation. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
On Mon, May 29, 2017 at 3:45 PM, Tom Lane wrote: > I wonder what the overhead is of using -fPIC when -fpic would be > sufficient. Whatever it is, the proposed patch imposes it on every > shlib or extension, to accommodate one single extension that isn't > even one we ship. > > Maybe this is small enough to not be something we need to worry about, > but I'm wondering if we should ask citus and other large .so's to set > some additional make flag that would cue usage of -fPIC over -fpic. Do we have an idea how to measure the increased overhead? Just from reading the description, I'm guessing that the increased cost would happen when the extension calls back into core, but maybe that doesn't happen often enough to worry about? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
Christoph Berg writes: > Re: To Andres Freund 2016-04-28 <20160428080824.ga22...@msg.df7cb.de> >>> I'm not clear why citus triggers this, when other extensions don't? >> Maybe it's simply because citus.so is bigger than all the other >> extension .so files: I wonder what the overhead is of using -fPIC when -fpic would be sufficient. Whatever it is, the proposed patch imposes it on every shlib or extension, to accommodate one single extension that isn't even one we ship. Maybe this is small enough to not be something we need to worry about, but I'm wondering if we should ask citus and other large .so's to set some additional make flag that would cue usage of -fPIC over -fpic. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x
Re: To Andres Freund 2016-04-28 <20160428080824.ga22...@msg.df7cb.de> > > I'm not clear why citus triggers this, when other extensions don't? > > Maybe it's simply because citus.so is bigger than all the other > extension .so files: > >-fpic > Generate position-independent code (PIC) suitable for use > in a shared library, if supported for the target machine. > Such code accesses all constant addresses through a global > offset table (GOT). The dynamic loader resolves the GOT > entries when the program starts (the dynamic loader is not > part of GCC; it is part of the operating system). If the > GOT size for the linked executable exceeds a machine- > specific maximum size, you get an error message from the > linker indicating that -fpic does not work; in that case, > recompile with -fPIC instead. (These maximums are 8k on > the SPARC and 32k on the m68k and RS/6000. The 386 has no > such limit.) > > Position-independent code requires special support, and > therefore works only on certain machines. For the 386, GCC > supports PIC for System V but not for the Sun 386i. Code > generated for the IBM RS/6000 is always > position-independent. > > When this flag is set, the macros "__pic__" and "__PIC__" > are defined to 1. > >-fPIC > If supported for the target machine, emit > position-independent code, suitable for dynamic linking and > avoiding any limit on the size of the global offset table. > This option makes a difference on the m68k, PowerPC and > SPARC. > > Position-independent code requires special support, and > therefore works only on certain machines. > > When this flag is set, the macros "__pic__" and "__PIC__" > are defined to 2. > > This doesn't mention s390(x), but citus.so 382952 bytes (on amd64) is > well beyond the 8k/32k limits mentioned above. > > PostgreSQL itself links correctly on s390x: > ... -I/usr/include/mit-krb5 -fPIC -pie -I../../../../src/include > > I'm not an expert in compiler flags, but it seems to me CFLAGS_SL is > wrong on s390(x) in Makefile.port, it should use -fPIC like sparc. After talking to a s390x Debian porter, -fPIC is the correct flag to use. The quoted patch made the previously failing builds for citus and pglogical (which have larger-than-average .so files) on s390x succeed (and the sparc64 case still works): --- a/src/makefiles/Makefile.linux +++ b/src/makefiles/Makefile.linux @@ -5,7 +5,8 @@ export_dynamic = -Wl,-E rpath = -Wl,-rpath,'$(rpathdir)',--enable-new-dtags DLSUFFIX = .so -ifeq "$(findstring sparc,$(host_cpu))" "sparc" +# Enable -fPIC to avoid "relocation truncated to fit" linker errors +ifneq "$(filter sparc% s390%,$(host_cpu))" "" CFLAGS_SL = -fPIC else CFLAGS_SL = -fpic The patch was made against 9.6; I'd opt to include it in master and the back branches. https://buildd.debian.org/status/logs.php?pkg=citus&arch=s390x https://buildd.debian.org/status/logs.php?pkg=pglogical&arch=s390x Christoph -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
