| 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: <20120823091125.GA12745@localhost>
Date: Thu, 23 Aug 2012 17:11:25 +0800
From: Fengguang Wu <fengguang.wu@...el.com>
To: Naoya Horiguchi <n-horiguchi@...jp.nec.com>
Cc: 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
On Wed, Aug 22, 2012 at 11:17:35AM -0400, Naoya Horiguchi wrote:
> "HWPOISON: report sticky EIO for poisoned file" still has a corner case
> where we have possibilities of data lost. This is because in this fix
> AS_HWPOISON is cleared when the inode cache is dropped.
>
> For example, consider an application in which a process periodically
> (every 10 minutes) writes some logs on a file (and closes it after
> each writes,) and at the end of each day some batch programs run using
> the log file. If a memory error hits on dirty pagecache of this log file
> just after periodic write/close and the inode cache is cleared before the
> next write, then this application is not aware of the error and the batch
> programs will work wrongly.
>
> To avoid this, this patch makes us pin the hwpoisoned inode on memory
> until we remove or completely truncate the hwpoisoned file.
Good point!
> Signed-off-by: Naoya Horiguchi <n-horiguchi@...jp.nec.com>
> ---
> fs/inode.c | 12 ++++++++++++
> include/linux/pagemap.h | 11 +++++++++++
> mm/memory-failure.c | 2 +-
> mm/truncate.c | 2 ++
> 4 files changed, 26 insertions(+), 1 deletion(-)
>
> 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?
> + /*
> * 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.
> inode->i_state |= I_FREEING;
> if (!list_empty(&inode->i_lru))
> inode_lru_list_del(inode);
> diff --git v3.6-rc1.orig/include/linux/pagemap.h v3.6-rc1/include/linux/pagemap.h
> index 4d8d821..9fce4e4 100644
> --- v3.6-rc1.orig/include/linux/pagemap.h
> +++ v3.6-rc1/include/linux/pagemap.h
> @@ -59,11 +59,22 @@ static inline int mapping_hwpoison(struct address_space *mapping)
> {
> return test_bit(AS_HWPOISON, &mapping->flags);
> }
> +static inline void mapping_set_hwpoison(struct address_space *mapping)
> +{
> + set_bit(AS_HWPOISON, &mapping->flags);
> +}
> +static inline void mapping_clear_hwpoison(struct address_space *mapping)
> +{
> + clear_bit(AS_HWPOISON, &mapping->flags);
> +}
> #else
> static inline int mapping_hwpoison(struct address_space *mapping)
> {
> return 0;
> }
> +static inline void mapping_clear_hwpoison(struct address_space *mapping)
> +{
> +}
> #endif
>
> static inline gfp_t mapping_gfp_mask(struct address_space * mapping)
> diff --git v3.6-rc1.orig/mm/memory-failure.c v3.6-rc1/mm/memory-failure.c
> index a1e7e00..ca064c6 100644
> --- v3.6-rc1.orig/mm/memory-failure.c
> +++ v3.6-rc1/mm/memory-failure.c
> @@ -652,7 +652,7 @@ static int me_pagecache_dirty(struct page *p, unsigned long pfn)
> * the first EIO, but we're not worse than other parts
> * of the kernel.
> */
> - set_bit(AS_HWPOISON, &mapping->flags);
> + mapping_set_hwpoison(mapping);
> }
>
> return me_pagecache_clean(p, pfn);
> diff --git v3.6-rc1.orig/mm/truncate.c v3.6-rc1/mm/truncate.c
> index 75801ac..82a994f 100644
> --- v3.6-rc1.orig/mm/truncate.c
> +++ v3.6-rc1/mm/truncate.c
> @@ -574,6 +574,8 @@ void truncate_setsize(struct inode *inode, loff_t newsize)
>
> oldsize = inode->i_size;
> i_size_write(inode, newsize);
> + if (unlikely(mapping_hwpoison(inode->i_mapping) && !newsize))
It might be a bit better to test !newsize first.
> + mapping_clear_hwpoison(inode->i_mapping);
>
> truncate_pagecache(inode, oldsize, newsize);
> }
> --
> 1.7.11.4
--
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