Re: [PATCH] mm: check memory reclaim bugs caused fs reentrance

[Dave Chinner <david@fromorbit.com>]

On Tue, Nov 11, 2014 at 03:49:50PM +0400, Dmitry Monakhov wrote:
> If filesystem holds transaction open 'current->journal_info' it should not
> performs memory allocations with __GFP_FS flag enabled otherwise this result in fs
> reentarance which lead to:
> 1) reentrance to itself : deadlock or internal assertion failure due to incorrect journal credits
> 1) entrance to another fs: assertion faulure or silient corruption due to incorrect journal
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
>  include/linux/kernel.h |    7 +++++++
>  mm/dmapool.c           |    1 +
>  mm/mempool.c           |    1 +
>  mm/page_alloc.c        |    1 +
>  mm/slab.c              |    1 +
>  mm/slub.c              |    1 +
>  6 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 3d770f5..69923d4 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -232,6 +232,13 @@ void might_fault(void);
>  static inline void might_fault(void) { }
>  #endif
>  
> +#ifdef CONFIG_PROVE_LOCKING
> +#define might_enter_fs_if(cond) \
> +	 WARN_ON_ONCE((cond) && current->journal_info)

XFS does not use current->journal_info, and so this won't ever
trigger on XFS. XFS uses PF_FSTRANS to indicate a transaction is in
progress.

Besides, isn't this redundant functionality? Lockdep already catches
these problems with it's reclaim context tracking and it's tracking
is more extensive than this simple check like this.  lockdep
regularly pointed out allocation/reclaim context problems in XFS
until we fixed them....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com