linux-kernel - Re: [PATCH 3/3] HWPOISON: improve handling/reporting of memory error on dirty pagecache

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 for Android: free password hash cracker in your pocket

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

Date:	Sun, 12 Aug 2012 11:19:21 -0400
From:	Naoya Horiguchi <n-horiguchi@...jp.nec.com>
To:	Andi Kleen <andi@...stfloor.org>
Cc:	Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
	Andi Kleen <andi.kleen@...el.com>,
	Wu Fengguang <fengguang.wu@...el.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Tony Luck <tony.luck@...el.com>,
	Rik van Riel <riel@...hat.com>,
	"Jun'ichi Nomura" <j-nomura@...jp.nec.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] HWPOISON: improve handling/reporting of memory error on dirty pagecache

Hi,

On Sun, Aug 12, 2012 at 05:28:44AM +0200, Andi Kleen wrote:
> > > That function uses a global lock. fdatawait is quite common. This will
> > > likely cause performance problems in IO workloads.
> > 
> > OK, I should avoid it.
> 
> Maybe just RCU the hash table.

OK.

> > > You need to get that lock out of the hot path somehow.
> > > 
> > > Probably better to try to put the data into a existing data structure,
> > > or if you cannot do that you would need some way to localize the lock.
> > 
> > Yes, I have thought about adding some data like new pagecache tag or
> > new members in struct address_space, but it makes the size of heavily
> > used data structure larger so I'm not sure it's acceptable.
> > And localizing the lock is worth trying, I think.
> 
> It's cheaper than a hash table lookup in the hot path.
>  
> > > Or at least make it conditional of hwpoison errors being around. 
> > 
> > I'll try to do your suggestions, but I'm not sure your point of the
> > last one. Can you explain more about 'make it conditional' option?
> 
> The code should check some flag first that is only set when hwpoison
> happened on the address space (or global, but that would mean that 
> performance can go down globally when any error is around)

I defined hwpoison_file_range() and hwpoison_partial_write() as wrapper
functions of __hwpoison_* variants, and they hold hwp_dirty_lock only
if AS_HWPOISON flag in mapping is set. So I hope we already did it.
But yes, I understand that in general a global lock is not good,
so I'll try to do other options.

Thank you,
Naoya
--
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/