LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
From: Christophe Leroy <christophe.leroy@c-s.fr>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Ben Hutchings <ben.hutchings@codethink.co.uk>,
	y2038 Mailman List <y2038@lists.linaro.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Nicholas Piggin <npiggin@gmail.com>,
	Paul Mackerras <paulus@samba.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	Allison Randal <allison@lohutok.net>
Subject: Re: [Y2038] [PATCH 07/23] y2038: vdso: powerpc: avoid timespec references
Date: Mon, 2 Dec 2019 13:55:55 +0100
Message-ID: <85ba697d-1a09-f1b0-b6b6-a511a2920f93@c-s.fr> (raw)
In-Reply-To: <CAK8P3a1UmAYTx=Vv06xP=O-oYD8_HzNqMG0-p7GPP2xgzs+75w@mail.gmail.com>



Le 27/11/2019 à 12:03, Arnd Bergmann a écrit :
> On Thu, Nov 21, 2019 at 5:25 PM Christophe Leroy
> <christophe.leroy@c-s.fr> wrote:
>> Arnd Bergmann <arnd@arndb.de> a écrit :
>>> On Wed, Nov 20, 2019 at 11:43 PM Ben Hutchings
>>> <ben.hutchings@codethink.co.uk> wrote:
>>>>
>>>> On Fri, 2019-11-08 at 22:07 +0100, Arnd Bergmann wrote:
>>>>> @@ -192,7 +190,7 @@ V_FUNCTION_BEGIN(__kernel_time)
>>>>>        bl      __get_datapage@local
>>>>>        mr      r9, r3                  /* datapage ptr in r9 */
>>>>>
>>>>> -     lwz     r3,STAMP_XTIME+TSPEC_TV_SEC(r9)
>>>>> +     lwz     r3,STAMP_XTIME_SEC+LOWPART(r9)
>>>>
>>>> "LOWPART" should be "LOPART".
>>>>
>>>
>>> Thanks, fixed both instances in a patch on top now. I considered folding
>>> it into the original patch, but as it's close to the merge window I'd
>>> rather not rebase it, and this way I also give you credit for
>>> finding the bug.
>>
>> Take care, might conflict with
>> https://github.com/linuxppc/linux/commit/5e381d727fe8834ca5a126f510194a7a4ac6dd3a
> 
> Sorry for my late reply. I see this commit and no other variant of it has
> made it into linux-next by now, so I assume this is not getting sent for v5.5
> and it's not stopping me from sending my own pull request.
> 
> Please let me know if I missed something and this will cause problems.
> 
> On a related note: are you still working on the generic lib/vdso support for
> powerpc? Without that, future libc implementations that use 64-bit time_t
> will have to use the slow clock_gettime64 syscall instead of the vdso,
> which has a significant performance impact.

I have left this generic lib/vdso subject aside for the moment, because 
performance is disappointing and its architecture doesn't real fit with 
powerpc ABI.

 From a performance point of view, it is manipulating 64 bits vars where 
is could use 32 bits vars. Of course I understand that y2038 will anyway 
force the use of 64 bits for seconds, but at the time being powerpc32 
VDSO is using 32 bits vars for both secs and ns, it make the difference. 
Also, the generic VDSO is playing too much with data on stacks and 
associated memory read/write/copies, which kills performance on RISC 
processors like powerpc. Inlining do_hres() for instance significantly 
improves that as it allow handling the 'struct __kernel_timespec ts' on 
registers instead of using stack.

Regarding powerpc ABI, the issue is that errors shall be reported by 
setting the SO bit in CR register, and this cannot be done in C.
This means:
- The VDSO entry point must be in ASM and the generic VDSO C function 
must be called from there, it cannot be the VDSO entry point.
- The VDSO fallback (ie the system call) cannot be done from the generic 
VDSO C function, it must be called from the ASM as well.

Last point/question, what's the point in using 64 bits for nanoseconds 
on 32 bits arches ?

Christophe

  reply index

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-08 21:02 [PATCH 00/23] y2038 cleanups Arnd Bergmann
2019-11-08 21:07 ` [PATCH 03/23] y2038: vdso: change timeval to __kernel_old_timeval Arnd Bergmann
2019-11-13 21:56   ` Thomas Gleixner
2019-11-08 21:07 ` [PATCH 07/23] y2038: vdso: powerpc: avoid timespec references Arnd Bergmann
2019-11-20 22:43   ` [Y2038] " Ben Hutchings
2019-11-21 14:23     ` Arnd Bergmann
2019-11-21 16:25       ` Christophe Leroy
2019-11-27 11:03         ` Arnd Bergmann
2019-12-02 12:55           ` Christophe Leroy [this message]
2019-12-02 14:03             ` Arnd Bergmann
2019-11-08 21:07 ` [PATCH 08/23] y2038: ipc: remove __kernel_time_t reference from headers Arnd Bergmann
2019-11-20 22:49   ` [Y2038] " Ben Hutchings
2019-11-21 14:28     ` Arnd Bergmann
2019-11-08 21:07 ` [PATCH 09/23] y2038: stat: avoid 'time_t' in 'struct stat' Arnd Bergmann
2019-11-08 21:12 ` [PATCH 12/23] y2038: syscalls: change remaining timeval to __kernel_old_timeval Arnd Bergmann
2019-11-11 12:44   ` Christian Brauner
2019-11-13 22:39   ` Rafael J. Wysocki
2019-11-13 21:40 ` [PATCH 00/23] y2038 cleanups Arnd Bergmann

Reply instructions:

You may reply publically 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=85ba697d-1a09-f1b0-b6b6-a511a2920f93@c-s.fr \
    --to=christophe.leroy@c-s.fr \
    --cc=allison@lohutok.net \
    --cc=arnd@arndb.de \
    --cc=ben.hutchings@codethink.co.uk \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=npiggin@gmail.com \
    --cc=paulus@samba.org \
    --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

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

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

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


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