linux-kernel - Re: [PATCH 3/3] HWPOISON: prevent inode cache removal to keep AS

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:	Fri, 24 Aug 2012 17:52:07 -0400
From:	Naoya Horiguchi <n-horiguchi@...jp.nec.com>
To:	Naoya Horiguchi <n-horiguchi@...jp.nec.com>
Cc:	Wu Fengguang <fengguang.wu@...el.com>,
	Andi Kleen <andi.kleen@...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: prevent inode cache removal to keep AS_HWPOISON sticky

Hello,

On Thu, Aug 23, 2012 at 04:31:43PM -0400, Naoya Horiguchi wrote:
> On Thu, Aug 23, 2012 at 05:11:25PM +0800, Fengguang Wu wrote:
> > On Wed, Aug 22, 2012 at 11:17:35AM -0400, Naoya Horiguchi wrote:
...
> > > diff --git v3.6-rc1.orig/fs/inode.c v3.6-rc1/fs/inode.c
> > > index ac8d904..8742397 100644
> > > --- v3.6-rc1.orig/fs/inode.c
> > > +++ v3.6-rc1/fs/inode.c
> > > @@ -717,6 +717,15 @@ void prune_icache_sb(struct super_block *sb, int nr_to_scan)
> > >  		}
> > >  
> > >  		/*
> > > +		 * Keep inode caches on memory for user processes to certainly
> > > +		 * be aware of memory errors.
> > > +		 */
> > > +		if (unlikely(mapping_hwpoison(inode->i_mapping))) {
> > > +			spin_unlock(&inode->i_lock);
> > > +			continue;
> > > +		}
> > 
> > That chunk prevents reclaiming all the cached pages. However the intention
> > is only to keep the struct inode together with the hwpoison bit?
> 
> Yes, we can not reclaim pagecaches from shrink_slab(), but we can do from
> shrink_zone(). So it shouldn't happen that cached pages on hwpoisoned file
> remain for long under high memory pressure.

I might lose your point. Are you suggesting this chunk should come after
if (inode_has_buffers(inode) || inode->i_data.nrpages) { ... } block,
aren't you?  I think that's right, so I'll try and test it this weekend.

> > > +		/*
> > >  		 * Referenced or dirty inodes are still in use. Give them
> > >  		 * another pass through the LRU as we canot reclaim them now.
> > >  		 */
> > > @@ -1405,6 +1414,9 @@ static void iput_final(struct inode *inode)
> > >  		inode->i_state &= ~I_WILL_FREE;
> > >  	}
> > >  
> > > +	if (unlikely(mapping_hwpoison(inode->i_mapping) && drop))
> > > +		mapping_clear_hwpoison(inode->i_mapping);
> > 
> > Is that clear necessary? Because the bit will be gone with the inode
> > struct: it's going to be de-allocated anyway.
> 
> With the chunk in prune_icache_sb() we keep the inode struct with
> AS_HWPOISON set on memory, so in order to remove it, we need explicitly
> clear the bit.
> Without this clear, the inode remains until system reboot.

And again, you are right here. Without this clear, this inode will be
cleared in destroy_inode().

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