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

[Dmitry Monakhov]

Dave Chinner <david@xxxxxxxxxxxxx> writes:

> 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@xxxxxxxxxx>
>> ---
>>  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.
Yes, I've simply forget about that.
>
> 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....
This is correct, but only partly, at this moment lockdep are not very good at
catching fs re-entrance. It has FS_RECLAIM machinery but it is not idial.
So I'll rewrite my patch and add explicit fs re-entrance rule to lockdep
>
> Cheers,
>
> Dave.
> -- 
> Dave Chinner
> david@xxxxxxxxxxxxx

Attachment: signature.asc
Description: PGP signature