LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	y2038 Mailman List <y2038@lists.linaro.org>,
	linux-riscv@lists.infradead.org,
	Palmer Dabbelt <palmer@sifive.com>
Subject: Re: [PATCH v2 3/7] riscv: Include asm-generic/compat.h
Date: Fri, 6 Jul 2018 13:42:46 +0200
Message-ID: <CAK8P3a2L9z+Ok1Wd0=5F=Xt7nvqL6be+AS++F3rZzByN+a+9QA@mail.gmail.com> (raw)
In-Reply-To: <CABeXuvoj_WwvnaqY1wao+axx1JLpPte-iXP=Ti7jsT+r6VrVUg@mail.gmail.com>

On Fri, Jul 6, 2018 at 1:56 AM, Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> On Thu, Jul 5, 2018 at 3:21 PM, Christoph Hellwig <hch@infradead.org> wrote:
>> On Thu, Jul 05, 2018 at 02:36:00PM -0700, Deepa Dinamani wrote:
>>> defconfig, allmodconfig and nomodconfig.
>>> And hence does not inlude definitions for compat data types.
>>>
>>> Now that time syscalls are being reused in non CONFIG_COMPAT
>>> modes, include asm-generic definitions for riscv.
>>>
>>> Alternative would be to make compat_time.h to be conditional on
>>> CONFIG_COMPAT_32BIT_TIME. But, since riscv is already has an
>>> asm/compat.h include the generic version instead.
>>
>> Two comments here:
>>
>> First I think the current riscv compat.h is completely bogus.
>> As you mentioned riscv does not actually have a compat mode, so
>> having a compat.h makes no sensse at all, and the COMPAT_UTS_MACHINE
>> override which is the only thing implemented is included in that
>> statement.
>
> I was leaving the decision on how to clean up compat mode to the
> architecture maintainers.
> I wasn't sure if they were still in the middle of implementing it.

If we only need it for 32 bit time_t, we can probably just use the
asm-generic/compat.h for now.

>> Second I think abusing compat.h for old syscall compatibility of any
>> form is a really bad idea.  I think you need to split that part out,
>> and preferably not using compat in the name, but something like
>> old-time.h or time32.h for the name.
>
> Are you talking about just the header file or the way we are reusing
> compat syscalls?
> Either way, we have merged quite a few patches this way already.
> I agree that having something like time32.h might be less confusing.
> But, if you are worried about the former, then maybe we should propose
> a cleanup after we finish what we are doing or back out the merged
> patches.
> For instance, posix_clock apis have already been changed this way.

It would be easy to rename compat_time_t, compat_timespec and
compat_timeval to something else if we could come up with a good
name for them (we already have too many variants of each of
them though, otherwise we end up being more confusing rather
than less).

We can also rename all the compat syscalls that are now shared
with 32-bit, e.g. using sys_waitid_time32() instead of
compat_sys_waitid(), and that would be consistent with the
new _time64() naming that we are introducing for some of them.
Right now, this affects around 30 syscalls; with my test tree on
ARM, I have this set of calls, the exact set is architecture
dependent of course:

compat_sys_time
compat_sys_stime
compat_sys_utime
compat_sys_gettimeofday
compat_sys_settimeofday
compat_sys_old_select
compat_sys_setitimer
compat_sys_getitimer
compat_sys_select
compat_sys_sched_rr_get_interval
compat_sys_nanosleep
compat_sys_rt_sigtimedwait
compat_sys_futex
compat_sys_io_getevents
compat_sys_timer_settime
compat_sys_timer_gettime
compat_sys_clock_settime
compat_sys_clock_gettime
compat_sys_clock_getres
compat_sys_clock_nanosleep
compat_sys_utimes
compat_sys_mq_timedsend
compat_sys_mq_timedreceive
compat_sys_futimesat
compat_sys_pselect6
compat_sys_ppoll
compat_sys_utimensat
compat_sys_timerfd_settime
compat_sys_timerfd_gettime
compat_sys_recvmmsg
compat_sys_io_pgetevents

Completely separating them from the compat code
would add further complexity though, as some of the
system calls take another argument that is different
between 32-bit and 64-bit kernels, in particular
pselect6, ppoll, io_pgetevents, recvmmsg, and waitid.

       Arnd

  reply index

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-05 21:35 [PATCH v2 0/7] Introduce struct __kernel_timex Deepa Dinamani
2018-07-05 21:35 ` [PATCH v2 1/7] arm64: Make basic compat_* types always available Deepa Dinamani
2018-07-05 21:35 ` [PATCH v2 2/7] sparc: Make thread_info.h available directly Deepa Dinamani
2018-07-05 21:36 ` [PATCH v2 3/7] riscv: Include asm-generic/compat.h Deepa Dinamani
2018-07-05 22:21   ` Christoph Hellwig
2018-07-05 23:56     ` Deepa Dinamani
2018-07-06 11:42       ` Arnd Bergmann [this message]
2018-07-07  4:23         ` Deepa Dinamani
2018-07-12  8:32         ` Christoph Hellwig
2018-07-12 12:31           ` Arnd Bergmann
2018-07-12 12:42         ` Geert Uytterhoeven
2018-07-12 13:51           ` Arnd Bergmann
2018-07-05 21:36 ` [PATCH v2 4/7] timex: prepare compat helpers for y2038 changes Deepa Dinamani
2018-07-05 21:36 ` [PATCH v2 5/7] time: Add struct __kernel_timex Deepa Dinamani
2018-07-05 21:36 ` [PATCH v2 6/7] timex: use __kernel_timex internally Deepa Dinamani
2018-07-05 21:36 ` [PATCH v2 7/7] timex: change syscalls to use struct __kernel_timex Deepa Dinamani

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAK8P3a2L9z+Ok1Wd0=5F=Xt7nvqL6be+AS++F3rZzByN+a+9QA@mail.gmail.com' \
    --to=arnd@arndb.de \
    --cc=deepa.kernel@gmail.com \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@sifive.com \
    --cc=tglx@linutronix.de \
    --cc=y2038@lists.linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git