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@xxxxxxxxxxxxx>
> ---
> 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, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>