public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Fabio Olive Leite <fleite@redhat.com>
To: Ray Lee <ray-lk@madrabbit.org>
Cc: Julia Lawall <julia@diku.dk>,
	autofs@linux.kernel.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org, hpa@zytor.com
Subject: Re: [autofs] [PATCH 1/4] fs/autofs: Use time_before, time_before_eq, etc.
Date: Fri, 28 Dec 2007 12:21:09 -0200	[thread overview]
Message-ID: <20071228142109.GA3801@sleipnir.redhat.com> (raw)
In-Reply-To: <2c0942db0712261158q34cddebeifceef6ceda683320@mail.gmail.com>

On Wed, Dec 26, 2007 at 11:58:56AM -0800, Ray Lee wrote:
> 
> On Dec 26, 2007 7:21 AM, Julia Lawall <julia@diku.dk> wrote:
> > -               if (jiffies - ent->last_usage < timeout)
> > +               if (time_before(jiffies, ent->last_usage + timeout))
> 
> I don't think this is a safe change? subtraction is always safe (if
> you think about it as 'distance'), addition isn't always safe unless
> you know the range.

This thinking is wrong, and is exactly the kind of wrong thinking that
led to the time_{before,after}{,_eq} macros. If jiffies just wrapped,
your safe subtraction will underflow, and you'll be comparing very
different unsigned values.

This is only a concern for 32bit architectures, of course. With 64bit
jiffies, even if HZ is set to 1000000 in the far future, this
civilization will still not outlive the jiffies counter (insert
shocking soundtrack!) as it will wrap in half a million years.

Sadly with 32bit jiffies it currently wraps every 50 days, and the
sign (if cast to signed) changes every 25 days.

> The time_before macro will expand that out to
> (effectively):
> 
>   if ( (long)(ent->last_usage + timeout) - (long)(jiffies) < 0 )

After contemplating this for a very long time (for an NFS fix at the
time) I _think_ the magic is in the casts to signed. The construct
manages to compare correctly any given jiffie values within 2^31 of
each other (on 32bit arches) even with wraps, assuming you really have
timestamps where the second one is taken less than 2^31 jiffies after
the first.

Of course, if the highest jiffies bit wraps twice between the first
and second timestamps, you'll be comparing apples and oranges. If the
timing is right this can happen if you compare two timestamps i.e. 26
days from each other, but I think only NFS is wicked enough to produce
such long living (and stale) kernel data structures.

So in summary: don't question time_after and friends. Not without a
very good analysis and testing with a matrix of timestamps. :)

Cheers!
Fábio Olivé
-- 
ex sed lex awk yacc, e pluribus unix, amem

      parent reply	other threads:[~2007-12-28 14:22 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-26 15:21 [PATCH 1/4] fs/autofs: Use time_before, time_before_eq, etc Julia Lawall
2007-12-26 17:48 ` H. Peter Anvin
2007-12-26 19:19   ` Julia Lawall
2007-12-26 19:58 ` Ray Lee
2007-12-26 20:45   ` H. Peter Anvin
2007-12-27  7:08     ` Julia Lawall
2007-12-28  1:39       ` YOSHIFUJI Hideaki / 吉藤英明
2007-12-28  4:34       ` [autofs] " Ian Kent
2007-12-28  9:27     ` Julia Lawall
2007-12-28 14:21   ` Fabio Olive Leite [this message]

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=20071228142109.GA3801@sleipnir.redhat.com \
    --to=fleite@redhat.com \
    --cc=autofs@linux.kernel.org \
    --cc=hpa@zytor.com \
    --cc=julia@diku.dk \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ray-lk@madrabbit.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
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