Message-ID: <4953017.3z0GqUlq8o@wuerfel>
Date:	Tue, 21 Jun 2016 17:00:37 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	y2038@...ts.linaro.org
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Deepa Dinamani <deepa.kernel@...il.com>,
	Dave Kleikamp <shaggy@...nel.org>,
	jfs-discussion@...ts.sourceforge.net,
	Trond Myklebust <trond.myklebust@...marydata.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Chris Mason <clm@...com>,
	"adilger.kernel@...ger.ca" <adilger.kernel@...ger.ca>,
	buchino@...co.com, Thomas Gleixner <tglx@...utronix.de>,
	"Yan, Zheng" <zyan@...hat.com>, jejb@...ux.vnet.ibm.com,
	Paul Moore <paul@...l-moore.com>,
	Linux SCSI List <linux-scsi@...r.kernel.org>,
	Ilya Dryomov <idryomov@...il.com>,
	"linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
	Changman Lee <cm224.lee@...sung.com>,
	Mark Fasheh <mfasheh@...e.com>, sramars@...co.com,
	John Stultz <john.stultz@...aro.org>,
	Al Viro <viro@...iv.linux.org.uk>,
	David Sterba <dsterba@...e.com>,
	Jaegeuk Kim <jaegeuk@...nel.org>,
	ceph-devel <ceph-devel@...r.kernel.org>,
	Linux NFS Mailing List <linux-nfs@...r.kernel.org>,
	Alex Elder <elder@...nel.org>, Theodore Ts'o <tytso@....edu>,
	Sage Weil <sage@...hat.com>,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	Artem Bityutskiy <dedekind1@...il.com>,
	Josef Bacik <jbacik@...com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	hiralpat@...co.com,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Eric Paris <eparis@...hat.com>,
	"Linux F2FS DEV, Mailing List" 
	<linux-f2fs-devel@...ts.sourceforge.net>,
	Steve French <sfrench@...ba.org>, linux-audit@...hat.com,
	ocfs2-devel@....oracle.com, Jan Kara <jack@...e.com>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	linux-mtd <linux-mtd@...ts.infradead.org>,
	lustre-devel@...ts.lustre.org,
	Anna Schumaker <anna.schumaker@...app.com>,
	linux-btrfs <linux-btrfs@...r.kernel.org>,
	Joel Becker <jlbec@...lplan.org>
Subject: Re: [Y2038] [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros

On Monday, June 20, 2016 11:03:01 AM CEST you wrote:
> On Sun, Jun 19, 2016 at 5:26 PM, Deepa Dinamani <deepa.kernel@...il.com> wrote:
> > The series is aimed at getting rid of CURRENT_TIME and CURRENT_TIME_SEC macros.

> Gcc handles 8-byte structure returns (on most architectures) by
> returning them as two 32-bit registers (%edx:%eax on x86). But once it
> is timespec64, that will no longer be the case, and the calling
> convention will end up using a pointer to the local stack instead.

I guess we already have that today, as the implementation of
current_fs_time() is

static inline struct timespec64 tk_xtime(struct timekeeper *tk)
{
        struct timespec64 ts;

        ts.tv_sec = tk->xtime_sec;
        ts.tv_nsec = (long)(tk->tkr_mono.xtime_nsec >> tk->tkr_mono.shift);
        return ts;
}
extern struct timespec64 current_kernel_time64(void);
struct timespec64 current_kernel_time64(void)
{
        struct timekeeper *tk = &tk_core.timekeeper;
        struct timespec64 now;
        unsigned long seq;

        do {
                seq = read_seqcount_begin(&tk_core.seq);

                now = tk_xtime(tk);
        } while (read_seqcount_retry(&tk_core.seq, seq));

        return now;
}
static inline struct timespec current_kernel_time(void)
{
        struct timespec64 now = current_kernel_time64();

        return timespec64_to_timespec(now);
}
extern struct timespec current_fs_time(struct super_block *sb);
struct timespec current_fs_time(struct super_block *sb)
{       
        struct timespec now = current_kernel_time();
        return timespec_trunc(now, sb->s_time_gran);
}       

We can surely do a little better than this, independent of the
conversion in Deepa's patch set.

> So for 32-bit code generation, we *may* want to introduce a new model of doing
> 
>     set_inode_time(inode, ATTR_ATIME | ATTR_MTIME);
> 
> which basically just does
> 
>     inode->i_atime = inode->i_mtime = current_time(inode);
> 
> but with a much easier calling convention on 32-bit architectures.
> 
> But that is entirely orthogonal to this patch-set, and should be seen
> as a separate issue.

I've played around with that, but found it hard to avoid going
through memory other than going all the way to the tk_xtime()
access to copy both tk->xtime_sec and the nanoseconds into
the inode fields.

Without that, the set_inode_time() implementation ends up
being more expensive than
inode->i_atime = inode->i_ctime = inode->i_mtime = current_time(inode);
because we still copy through the stack but also have
a couple of conditional branches that we don't have at the
moment.

At the moment, the triple assignment becomes (here on ARM)

   c:   4668            mov     r0, sp
  12:   f7ff fffe       bl      0 <current_kernel_time64>
  3e:   f107 0520       add.w   r5, r7, #32
                        12: R_ARM_THM_CALL      current_kernel_time64
  16:   f106 0410       add.w   r4, r6, #16
  1a:   e89d 000f       ldmia.w sp, {r0, r1, r2, r3} # load from stack
  1e:   e885 000f       stmia.w r5, {r0, r1, r2, r3} # store into i_atime
  22:   e884 000f       stmia.w r4, {r0, r1, r2, r3} #            i_ctime
  26:   e886 000f       stmia.w r6, {r0, r1, r2, r3} #            i_mtime

and a slightly more verbose version of the same thing on x86
(storing only 12 bytes instead of 16 is cheaper there, while
ARM does a store-multiple to copy the entire structure).

        Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists