linux-ext4 - Re: [PATCH RFC] jbd: don't wake kjournald unnecessarily

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20121219171401.GB28042@quack.suse.cz>
Date:	Wed, 19 Dec 2012 18:14:01 +0100
From:	Jan Kara <jack@...e.cz>
To:	Theodore Ts'o <tytso@....edu>
Cc:	Jan Kara <jack@...e.cz>, Eric Sandeen <sandeen@...hat.com>,
	ext4 development <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH RFC] jbd: don't wake kjournald unnecessarily

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@...e.cz>
SUSE Labs, CR
--
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