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
| ||
|
Message-ID: <20130321224143.GA5066@quack.suse.cz> Date: Thu, 21 Mar 2013 23:41:43 +0100 From: Jan Kara <jack@...e.cz> To: Theodore Ts'o <tytso@....edu> Cc: Jan Kara <jack@...e.cz>, Ben Hutchings <ben@...adent.org.uk>, George Barnett <gbarnett@...assian.com>, linux-ext4@...r.kernel.org, Debian kernel maintainers <debian-kernel@...ts.debian.org> Subject: Re: [PATCH] ext4/jbd2: don't wait (forever) for stale tid caused by wraparound On Thu 21-03-13 17:09:40, Ted Tso wrote: > On Thu, Mar 21, 2013 at 09:46:38PM +0100, Jan Kara wrote: > > Good catch! But shouldn't we rather fix jbd2_log_wait_commit() instead of > > inventing new function? > > In most of the places where we call jbd2_log_start_commit(), we're > actually starting the current running transaction. So the fact that > we pass in a tid, and we're having to validate that the tid is > actually a valid one, is a bit of a waste. So in the long run I think > it's worth rethinking whether or not jbd2_log_{start,wait}_commit() > should exist in their current form, or whether we should reorganize > their functionality (i.e., by having a jbd2_start_running_commit(), > for example.). Piling on fixes to jbd2_log_wait_commit() would make > it get even more complicated, and I think if we separate out the > various ways in which we use these functions, we can make the code > simpler and easier to read. I don't find jbd2_log_wait_commit() that complex that it couldn't bear another if :) But given there are really two waiting operations that make sense: a) request commit of running transaction and wait for it b) wait for committing transaction then I agree there may be a better interface. OTOH I'm somewhat curious about the new interface because the only race-free way of identifying a transaction you want to wait for is using its tid. > In fact, I had started making this rather large set of changes when I > decided it would be better to save that kind of wholesale refactoring > for the next merge window. So the reason why I ended up fixing the > patch the way I did was to keep things simple. > > Also as I mentioned in the commit description, by using a single > function I was also able to optimize the locking the locking somewhat. Yeah. I'm not as much opposed to the new function doing start commit & wait but what I dislike is the fact that we have still exposed the function jbd2_log_wait_commit() which can possibly lockup if tid overflows. I agree there aren't currently any other callers where this could happen but in a few years who knows... 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
Powered by blists - more mailing lists