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] [day] [month] [year] [list]
Message-ID: <alpine.LFD.2.01.0907020956410.4830@localhost.localdomain>
Date:	Thu, 2 Jul 2009 10:10:14 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	"Zhang, Yanmin" <yanmin_zhang@...ux.intel.com>
cc:	Pavel Levshin <lpk@....spb.su>, wli@...ementarian.org,
	Nick Piggin <npiggin@...e.de>,
	Wu Fengguang <fengguang.wu@...el.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: fio mmap sequential read 30% regression



On Thu, 2 Jul 2009, Zhang, Yanmin wrote:
> 
> Bisect down to below patch.
> 
> ef00e08e26dd5d84271ef706262506b82195e752 ("readahead: clean up and 
> simplify the code for filemap page fault readahead")
> 
> The bisect is stable.

Hmm. That patch is the one patch in the whole series that should _not_ 
have changed any behavior, but it got forward-ported by something like 8 
months, so maybe something changed in the meantime.

Also, I do think it changes the exact details of "ra->mmap_miss" (along 
with the fault counters). The old code was really quite odd in the 
statistics department, and would do some odd things wrt the miss counts 
and statistics - because it had this re-try loop that essentially updated 
the stats twice (once on the first try, and then once more when doing the 
"retry_find" thing).

The patch gets rid of the hard-to-follow retry logic (it had that 
"did_readaround" logic to determine if it was on the first loop or the 
second one, and used those _sometimes_), and should _fix_ those stats, but 
obviously all historical tuning had happened with the old stats. So it's 
entirely possible that as part of "fixing" them, I actually broke what the 
old logic tried to do.

There's also one _intentional_ change: the new code doesn't take the lock 
on the page before it starts async read-ahead. That can certainly change 
timings a lot. It _should_ cause better behavior (more IO overlap), but 
depending on read-ahead code and the exact behavior of your IO subsystem, 
maybe it causes you problems.

But it is also possible that the patch simply changes behavior in 
unintended ways. Either originally, or due to being forward-ported with 
all the other read-ahead changes. I'm not seeing it, though - the patch 
still looks like it shouldn't change any semantics.

			Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ