xfs
[Top] [All Lists]

Re: [PATCH-v2 3/5] vfs: don't let the dirty time inodes get more than a

To: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH-v2 3/5] vfs: don't let the dirty time inodes get more than a day stale
From: Theodore Ts'o <tytso@xxxxxxx>
Date: Mon, 24 Nov 2014 12:10:54 -0500
Cc: linux-fsdevel@xxxxxxxxxxxxxxx, Ext4 Developers List <linux-ext4@xxxxxxxxxxxxxxx>, xfs@xxxxxxxxxxx, linux-btrfs@xxxxxxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=thunk.org; s=ef5046eb; h=In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=sKieOi904OntYwG9TX2fzgjAkRmhc0ntwr9KxxWjGR0=; b=Uh7DVmz3BTggvKAA8h92UgI5ahpbJaVRzaH62sAkKMFQfSCcA8aVIeUiNfqXmI27mOoz3Kv6b98KTd6Ku+UtNIQ6m0cPy4++u5AIVu+dJG64SX21prCqa8lLGlY8FSwS4Uq2rEy06gjRqDFgvF6Dbx6ketgVbbMQDmcIsqqBvbQ=;
In-reply-to: <87egss3hsm.fsf@xxxxxxxxxxxxxxxxxx>
References: <1416675267-2191-1-git-send-email-tytso@xxxxxxx> <1416675267-2191-4-git-send-email-tytso@xxxxxxx> <87egss3hsm.fsf@xxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
On Mon, Nov 24, 2014 at 01:27:21PM +0100, Rasmus Villemoes wrote:
> On Sat, Nov 22 2014, Theodore Ts'o <tytso@xxxxxxx> wrote:
> 
> > Guarantee that the on-disk timestamps will be no more than 24 hours
> > stale.
> >
> > +   unsigned short days_since_boot = jiffies / (HZ * 86400);
> 
> This seems to wrap every 49 days (assuming 32 bit jiffies and HZ==1000),
> so on-disk updates can be delayed indefinitely, assuming just the right
> delays between writes.

Good point, I'll fix this.

> Would it make sense to introduce days_since_boot as a global variable
> and avoid these issues? This would presumably also make update_time a
> few cycles faster (avoiding a division-by-constant), but not sure if
> that's important. And something of course needs to update
> days_since_boot, but that should be doable.

I can do this fairly simply like this:

        get_monotonic_boottime(&uptime);
        daycode = uptime.tv_sec / (HZ * 86400);

and we only need to do this if lazytime is set, and the inode isn't
marked as I_DIRTY_TIME:

        if ((inode->i_sb->s_flags & MS_LAZYTIME) &&
            !(flags & S_VERSION)) {
                if (inode->i_state & I_DIRTY_TIME)
                        return 0;
                get_monotonic_boottime(&uptime);
                daycode = do_div64(uptime.tv_sec do_div, (HZ * 86400));
                if (!inode->i_ts_dirty_day ||
                    inode->i_ts_dirty_day == daycode) {
                        spin_lock(&inode->i_lock);
                        inode->i_state |= I_DIRTY_TIME;
                        spin_unlock(&inode->i_lock);
                        inode->i_ts_dirty_day = daycode;
                        return 0;
                }
        }

So I'm not entirely sure it's worth it to create a global variable for
days since boot; I've been runnin with this patch in my laptop, we
wouldn't be triggering the get_monotonic_bootime() function all that
often.  (Since once the dirty_time flg is set, we don't need to check
about whether we need to set it again.)  And if we *did* care, it
would be simple enough to use a static counter which only recalculates
daycode every 30 or 60 minutes.


Cheers,

                                                        - Ted

<Prev in Thread] Current Thread [Next in Thread>