linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Theodore Ts'o <tytso@mit.edu>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Linux Filesystem Development List <linux-fsdevel@vger.kernel.org>,
	Ext4 Developers List <linux-ext4@vger.kernel.org>,
	Linux btrfs Developers List <linux-btrfs@vger.kernel.org>,
	XFS Developers <xfs@oss.sgi.com>
Subject: Re: [PATCH-v3 3/6] vfs: don't let the dirty time inodes get more than a day stale
Date: Tue, 25 Nov 2014 10:55:22 -0500	[thread overview]
Message-ID: <20141125155522.GH31339@thunk.org> (raw)
In-Reply-To: <87ioi3iayu.fsf@rasmusvillemoes.dk>

On Tue, Nov 25, 2014 at 03:58:01PM +0100, Rasmus Villemoes wrote:
> 
> I think days_since_boot was a lot clearer than daycode. In any case,
> please make the comment and the code consistent.

Yeah, I was going back and forth between days since the epoch and days
since boot, but found it was more efficient to calculate the days
since boot.  Sure, I'll go back to days_since_boot.

> You should probably divide by the number of seconds in a day, not the
> number of jiffies in a day.

Right, brain hiccup on my part, will fix.

> Isn't div_u64 mostly for when the divisor is not known at compile time?
> Technically, "(u64)uptime.tv_sec / 86400" is of course a u64/u64 division,
> but the compiler should see that the divisor is only 32 bits and hence
> should be able to generate code which is at least as efficient as
> whatever inline asm the arch provides for u64/u32 divisions.

The problem with doing u64/u64 divisions is that the compiler will
call out to a (non-existent) library function on some architectures.
For example, try compiling the following using: with "gcc -S -m32
foo.c"

int main(int argc, char **argv)
{
	unsigned long long t = time(0);

	printf("%llu\n", t / 86400);
}

You will find in the .S file the following:

	...
	pushl	$0
	pushl	$86400
	pushl	%edx
	pushl	%eax
	call	__udivdi3
	...

It will work finn compiling for x86_64, but if you do this and push
out a git branch, you will soon get a very nice e-mail from the ktest
bot explaining how you've broken the build on the i386 architecture
since the kernel doesn't provide __udivdi3.
	
Hence if you are doing any kind of divisions involving u64, you have
to use the functions in include/linux/math64.h, and div_u64 is, per
math64.h:

/**
 * div_u64 - unsigned 64bit divide with 32bit divisor
 *
 * This is the most common 64bit divide and should be used if possible,
 * as many 32bit archs can optimize this variant better than a full 64bit
 * divide.
 */

Cheers,

					- Ted

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2014-11-25 15:55 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-25  5:34 [PATCH-v3 0/6] add support for a lazytime mount option Theodore Ts'o
2014-11-25  5:34 ` [PATCH-v3 1/6] fs: split update_time() into update_time() and write_time() Theodore Ts'o
2014-11-25 16:00   ` David Sterba
2014-11-25  5:34 ` [PATCH-v3 2/6] vfs: add support for a lazytime mount option Theodore Ts'o
2014-11-25  5:34 ` [PATCH-v3 3/6] vfs: don't let the dirty time inodes get more than a day stale Theodore Ts'o
2014-11-25 14:58   ` Rasmus Villemoes
2014-11-25 15:55     ` Theodore Ts'o [this message]
2014-11-25  5:34 ` [PATCH-v3 4/6] vfs: add lazytime tracepoints for better debugging Theodore Ts'o
2014-11-25  5:34 ` [PATCH-v3 5/6] ext4: add support for a lazytime mount option Theodore Ts'o
2014-11-25  5:34 ` [PATCH-v3 6/6] btrfs: add an is_readonly() so btrfs can use common code for update_time() Theodore Ts'o
2014-11-25 16:02   ` David Sterba

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=20141125155522.GH31339@thunk.org \
    --to=tytso@mit.edu \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=xfs@oss.sgi.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).