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

Dmitry Monakhov
2014-11-12T08:58:35Z

[-- Attachment #1: Type: text/plain, Size: 2066 bytes --]

Dave Chinner <david@fromorbit.com> 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@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.
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@fromorbit.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]