linux-kernel - Re: mmotm 2009-06-02-16-11 uploaded (readahead)

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]

Date:	Thu,  4 Jun 2009 10:25:37 +0900 (JST)
From:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	kosaki.motohiro@...fujitsu.com,
	Randy Dunlap <randy.dunlap@...cle.com>,
	linux-kernel@...r.kernel.org, hifumi.hisashi@....ntt.co.jp,
	Jens Axboe <jens.axboe@...cle.com>,
	Wu Fengguang <fengguang.wu@...el.com>
Subject: Re: mmotm 2009-06-02-16-11 uploaded (readahead)

> On Tue, 02 Jun 2009 20:54:39 -0700
> Randy Dunlap <randy.dunlap@...cle.com> wrote:
> 
> > akpm@...ux-foundation.org wrote:
> > > The mm-of-the-moment snapshot 2009-06-02-16-11 has been uploaded to
> > > 
> > >    http://userweb.kernel.org/~akpm/mmotm/
> > > 
> > > and will soon be available at
> > > 
> > >    git://git.zen-sources.org/zen/mmotm.git
> > 
> > 
> > readahead-add-blk_run_backing_dev.patch:
> > 
> > mm/readahead.c: In function 'page_cache_async_readahead':
> > mm/readahead.c:559: error: implicit declaration of function 'blk_run_backing_dev'
> 
> hm, yeah, CONFIG_BLOCK=n.
> 
> Doing a block-specific call from inside page_cache_async_readahead() is
> a bit of a layering violation - this may not be a block-backed
> filesystem at all.
> 
> otoh, perhaps blk_run_backing_dev() is wrongly named and defined in the
> wrong place.  Perhaps non-block-backed backing_devs want to implement
> an unplug-style function too?  In which case the whole thing should be
> renamed and moved outside blkdev.h.
> 
> If we don't want to do that, shouldn't backing_dev_info.unplug* be
> wrapped in #ifdef CONFIG_BLOCK?  And wasn't it a layering violation to
> put block-specific things into the backing_dev_info?
> 
> Jens, talk to me!
> 
> From the readahead POV: does it make sense to call the backing-dev's
> "unplug" function even if that isn't a block-based device?  Or was this
> just a weird block-device-only performance problem?  Hard to say.

More problematic.

The patch comment says 

+	/*
+	* Normally the current page is !uptodate and lock_page() will be
+	* immediately called to implicitly unplug the device. However this
+	* is not always true for RAID conifgurations, where data arrives
+	* not strictly in their submission order. In this case we need to
+	* explicitly kick off the IO.


However, hifumi-san's test result doesn't have IO reordering log.
At least the comment is wrong. and We still don't know why nobody can
reproduce his issue.




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