From: Jan Kara <jack@suse.cz>
Subject: Re: [PATCH RFC] jbd: don't wake kjournald unnecessarily
Date: Wed, 19 Dec 2012 18:14:01 +0100
Message-ID: <20121219171401.GB28042@quack.suse.cz>
References: <50D0A1FD.7040203@redhat.com>
 <20121219012710.GF5987@quack.suse.cz>
 <20121219020526.GG5987@quack.suse.cz>
 <50D12FC3.6090209@redhat.com>
 <20121219081334.GB20163@quack.suse.cz>
 <20121219153725.GD7795@thunk.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Jan Kara <jack@suse.cz>, Eric Sandeen <sandeen@redhat.com>,
	ext4 development <linux-ext4@vger.kernel.org>
To: Theodore Ts'o <tytso@mit.edu>
Content-Disposition: inline
In-Reply-To: <20121219153725.GD7795@thunk.org>
Sender: linux-ext4-owner@vger.kernel.org

On Wed 19-12-12 10:37:25, Ted Tso wrote:
> On Wed, Dec 19, 2012 at 09:13:34AM +0100, Jan Kara wrote:
> > > I was wondering if, since the tid_g*() functions only work if the
> > > distance is half the unsigned int space, we can force a commit at some
> > > point if j_transaction_sequence has gotten too far ahead?  I'm not sure
> > > where or if that could be done...
> >
> >   I don't quiete understand. If someone stores tid = transaction->t_tid and
> > in two weeks calls log_start_commit(tid), I don't see how any forcing of
> > commits could solve that tid may now look ahead of the log...
> 
> Actually, what we can do is the reverse.  Right now when we modify an
> inode, we stash the tid to indicate "we need to commit to at least
> this tid".  The problem comes if we haven't modified the inode for a
> long time, but then later when we issue an fsync for that inode, it's
> after a tid wrap, so we trigger the warning.
> 
> What we could do is in cases were we aren't touching the inode note if
> the tid value is obviously out of date, and set it to some value which
> is one less than the current tid.  This avoids the wrap, and in cases
> where we are releasing the inode and nothing else is left, it's a safe
> thing to do.
> 
> The downside is that doing this would incur locking overhead, and it's
> not clear it's worth it.  Now that we understand what's going on,
> nothing bad is happening when the warning is triggered, so we could
> just remove it.
> 
> If we use a 64-bit in-memory tid, that would help avoid cases where
> the tid has wrapped and we get unlucky and hit the 1 in 2**32 case
> where we trigger a commit where one is not needed.  Given that the
> cost of an extra commit with probably 1 in 2**32 is pretty low, it's
> probably not worth the overhead of using a 64-bit tid, though....
  I agree. Just I'm still somewhat puzzled by those two reports pointed to
by Eric. In both cases stored tids were 0 and I cannot see how that happens
(well how it could happen in a reasonably likely way).

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR