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: <20121219153725.GD7795@thunk.org>
Date:	Wed, 19 Dec 2012 10:37:25 -0500
From:	Theodore Ts'o <tytso@....edu>
To:	Jan Kara <jack@...e.cz>
Cc:	Eric Sandeen <sandeen@...hat.com>,
	ext4 development <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH RFC] jbd: don't wake kjournald unnecessarily

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....

      	  	      	      - Ted
--
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