Re: [PATCH 4/9] posix-clocks: Enable compat syscalls always

[Arnd Bergmann]

On Tue, Nov 14, 2017 at 12:26 AM, Deepa Dinamani <deepa.kernel@xxxxxxxxx> wrote:
> On Mon, Nov 13, 2017 at 1:03 PM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>> On Fri, 10 Nov 2017, Deepa Dinamani wrote:
>>>
>>> -#ifdef CONFIG_COMPAT
>>> -
>>>  COMPAT_SYSCALL_DEFINE2(nanosleep, struct compat_timespec __user *, rqtp,
>>>                      struct compat_timespec __user *, rmtp)
>>>  {
>>> @@ -1574,7 +1572,6 @@ COMPAT_SYSCALL_DEFINE2(nanosleep, struct compat_timespec __user *, rqtp,
>>>       current->restart_block.nanosleep.compat_rmtp = rmtp;
>>>       return hrtimer_nanosleep(&tu, HRTIMER_MODE_REL, CLOCK_MONOTONIC);
>>>  }
>>> -#endif
>>
>> So if I'm not missing something important this will make the compat syscall
>> define available even for
>>
>> CONFIG_X86_64=y
>> CONFIG_IA32_EMULATION=n
>> CONFIG_X86_X32=n
>>
>> which is wrong because in that configuration we don't have any 32bit
>> executable support. So why would we need a compat syscall in that case?
>
> I was thinking that we would switch the meaning of CONFIG_64BIT_TIME
> after all architectures have enabled support for 64 bit time_t
> syscalls.

I think at that point the CONFIG_64BIT_TIME symbol can just go away,
the name would not express any useful meaning any more once all
architectures use 64-bit time types.

> After that, I was going to use the config to mean enable only 64 bit
> time_t support and so all compat syscalls would have #ifndef
> CONFIG_64BIT_TIME around them.
> Compat syscalls do not mean compat anymore for these time syscalls as
> the data structure is defined in a way that they are the same on 64
> bit and 32 bit architectures.
>
> But, you are right. It will leave in compat syscalls when it is not
> required now.
> I could add a dependency on CONFIG_COMPAT and __BITS_PER_LONG for now
> if you prefer. I could introduce additional dependencies later on.

In kernel uapi headers, checking for __BITS_PER_LONG is appropriate,
but we can't check for CONFIG_COMPAT there, since user space
does not see the CONFIG_ namespace.

In the kernel, checking for 64-bit architectures is better done using
the CONFIG_64BIT symbol.

So we could do

-#ifdef CONFIG_COMPAT
+#if defined(CONFIG_COMPAT) || !defined(CONFIG_64BIT)
-
 COMPAT_SYSCALL_DEFINE2(nanosleep, struct compat_timespec __user *, rqtp,
                      struct compat_timespec __user *, rmtp)

The trick with renaming the Kconfig symbol worked better with the old
CONFIG_COMPAT_TIME name that I had in an earlier draft of the
series, it would also enable us to leave out the 32-bit time compat
syscall implementation for future architectures that never had the
old uapi.

        Arnd