On 4 December 2012 12:46, Vineet Gupta <[email protected]> wrote: > On Monday 03 December 2012 10:00 PM, Markos Chandras wrote: >> On 28 November 2012 10:58, Vineet Gupta <[email protected]> wrote: >>> On Wednesday 28 November 2012 03:18 PM, Markos Chandras wrote: >>>> On 28 November 2012 09:27, Markos Chandras <[email protected]> >>>> wrote: >>>>> On 27 November 2012 23:36, Rich Felker <[email protected]> wrote: >>>>>> The claim of "ABI break" is nonsense. There is presently NO WAY to use >>>>>> this legacy stat structure on no-legacy-syscall kernels. So there is >>>>>> no way anything could be using it. Really, on no-legacy-syscall >>>>>> kernels, _FILE_OFFSET_BITS should always be 64 and the legacy structs >>>>>> should not even exist. But if you insist on providing them anyway, you >>>>>> should use the same layout as the 64-bit struct that the syscall >>>>>> supports, rather than writing nasty bloat to convert to a completely >>>>>> non-native legacy structure that has nothing to do with the kernel. >>>>> You seem to ignore part of my replies. I explained that 32-bit >>>>> syscalls need to be present >>>>> so existing applications can link and work as expected. For example, >>>>> many applications still use >>>>> stastfs() instead of statfs64(). As a result of which, we still need >>>>> the 32-bit structures around. >>>>> And the 32-bit structure you will pass to the INLINE_SYSCALL() will >>>>> end up in kernel space so it has to >>>>> have a similar layout with the kernel statfs structure which ( I >>>>> already said that ) is always defined in new architectures >>>>> in $KERNEL/include/asm-generic/statfs.h. So even if we provide our own >>>>> in uClibc, we can't change the kernel one. >>>>> Do I miss something? >>>> Right so I was partly wrong. We still need to have the 32-bit syscalls >>>> around but like you said we can define our own >>>> struct statfs which would look like the 64-bit one. So the function >>>> signatures will not have to change. Furthermore, >>>> within the 32-bit syscall we will pass this new struct statfs to the >>>> 64-bit syscall. >>> Precisely - now we are all on same page. So collecting all the ideas so >>> far for conclusion: >>> >>> (1) we keep stat/stat64 as completely different/independent and have >>> itemized copies in the syscall wrappers as needed - this is not good for >>> performance - typically a few fields in stat are ever looked at but >>> because of conversion we end up touching each one of them - and that too >>> twice. The slight advantage however for native 32 bit arches is that >>> 64bit emulation (assigning a 64 bit field to 32 bit field) is restricted >>> to xconv layer only (but only for !_FILE_OFFSET_BITS and/or >>> LARGEFILE64_SOURCE) >>> >>> (2) We unconditionally define _FILE_OFFSET_BITS=64 for no-legacy-syscall >>> kernel based uClibc ports and do away with 32bit (legacy) stat >>> completely. libc preprocessor tricks will compile time convert any stat >>> refs (struct/function) to stat64 variants. For native 32 bit archs this >>> means the 64 bit emulation code will be all over the place which can >>> cause some performance hit (e.g. Busybox ls will have 64 bit field - >>> although that is true even today because by default it is built with >>> FOB=64). Also can this cause potential issues with open source projects >>> which for some obscure reasons don't support FOB=64. >>> >>> (3) Have stat/stat64 with same overall layout but stat will internally >>> have 32 bit items (with appropriate padding) - allowing both structs to >>> be used in the 64bit syscall interface. This needs no interworking crap >>> except for some overflow checks (by testing high word). >>> >>> So in theory both #2 and #3 are acceptable solutions. I'd vote for #3. >>> >>> Comments ! >>> >>> That should work. I will send a new >>>> patch when I have something working. >>>> >>>> >>> To make this plug-n-play for uClibc ports for future no-legacy-syscalls >>> kernels, you probably would need to define a >>> libc/sysdeps/linux/common-no-legacy/bits/{stat.h,xxx} and have that be >>> selected based on !ARCH_HAS_DEPRECATED_SYSCALLS. >>> >>> Markos, I'm extremely sorry if I/we are making your life miserable - but >>> things touching the ABI are only done once so we better flush out any >>> issues (including performance) before casting it in stone. >>> >>> -Vineet >> I think we don't need to provide a separate struct stat for new >> architectures as functions such as fstatat(), will pass a >> kernel_stat64 >> to the fstatat64 syscall which will then be converted back to 'stat' >> using the __xstat32_conv helper (see the relevant patches I posted in >> this patch series). However, a struct statfs is indeed needed for >> fstatfs and statfs wrappers. > > Not sure if I understand you here. Come what may - we need to have a > struct stat in uClibc as independent of the ABI - user is still allowed > to write the following code with FILE_OFFSET_BITS != 64. > > #include <sys/stat.h> > int main(void) > { > struct stat sbuf; > if (stat("/xxx", &sbuf) != 0) { > return (-1); > } > printf("size %d\n", sizeof(sbuf)); > } > > Am I missing something here !
Yes he is, but stat() uses fstatat() syscall for new architectures which is then uses kernel_stat64 for the system call (note, kernel_stat64 and _not_ stat64). This is then converted back to the 32-bit struct using the __xstat32_conv function. I am not sure if passing stat64 (or a new stat for new architectures) instead of kernel_stat64 is a good alternative. It may be, but I haven't tried that yet. > > Also AFAIKR the last time we discussed, we'd agreed to try and remove > any 64 to 32 translation layer. Yes, that was for a direct conversion like that one I had eg: foo->a=bar->a; But uClibc provides the __xstatXX_conv functions already which are used in other syscalls as well. So what I'm saying here is to use these functions for converting a kernel_stat64 to stat structures and only implement a struct statfs for new syscalls. I will provide some links with the current implementation soon just to show you what I mean. -- Regards, Markos _______________________________________________ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
