[PATCH] reiserfs: use __GFP_NOFAIL instead of yield and retry loop for allocation

[PATCH] reiserfs: use __GFP_NOFAIL instead of yield and retry loop for allocation

[Pekka J Enberg]

From: Pekka Enberg <penberg@cs.helsinki.fi>

This patch replaces yield and retry loop with __GFP_NOFAIL in
alloc_journal_list(). Compile-tested only.

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 fs/reiserfs/journal.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Index: 2.6/fs/reiserfs/journal.c
===================================================================
--- 2.6.orig/fs/reiserfs/journal.c
+++ 2.6/fs/reiserfs/journal.c
@@ -2446,12 +2446,8 @@ static int journal_read(struct super_blo
 static struct reiserfs_journal_list *alloc_journal_list(struct super_block *s)
 {
 	struct reiserfs_journal_list *jl;
-      retry:
-	jl = kzalloc(sizeof(struct reiserfs_journal_list), GFP_NOFS);
-	if (!jl) {
-		yield();
-		goto retry;
-	}
+	jl = kzalloc(sizeof(struct reiserfs_journal_list),
+		     GFP_NOFS | __GFP_NOFAIL);
 	INIT_LIST_HEAD(&jl->j_list);
 	INIT_LIST_HEAD(&jl->j_working_list);
 	INIT_LIST_HEAD(&jl->j_tail_bh_list);

Re: [PATCH] reiserfs: use __GFP_NOFAIL instead of yield and retry loop for allocation

[Andrew Morton]

Pekka J Enberg <penberg@cs.Helsinki.FI> wrote:
>
>  -      retry:
>  -	jl = kzalloc(sizeof(struct reiserfs_journal_list), GFP_NOFS);
>  -	if (!jl) {
>  -		yield();
>  -		goto retry;
>  -	}
>  +	jl = kzalloc(sizeof(struct reiserfs_journal_list),
>  +		     GFP_NOFS | __GFP_NOFAIL);

yup, that's what __GFP_NOFAIL is for: to consolidate and identify all those
places which want to lock up when we're short of memory...  They all need
fixing, really.

Re: [PATCH] reiserfs: use __GFP_NOFAIL instead of yield and retry loop for allocation

[Pekka J Enberg]

Hi,

Pekka J Enberg <penberg@cs.Helsinki.FI> wrote:
> >
> >  -      retry:
> >  -	jl = kzalloc(sizeof(struct reiserfs_journal_list), GFP_NOFS);
> >  -	if (!jl) {
> >  -		yield();
> >  -		goto retry;
> >  -	}
> >  +	jl = kzalloc(sizeof(struct reiserfs_journal_list),
> >  +		     GFP_NOFS | __GFP_NOFAIL);

On Thu, 12 Jan 2006, Andrew Morton wrote:
> yup, that's what __GFP_NOFAIL is for: to consolidate and identify all those
> places which want to lock up when we're short of memory...  They all need
> fixing, really.

Out of curiosity, are there any potential problems with combining GFP_NOFS 
and __GFP_NOFAIL? Can we really guarantee to give out memory if we're not 
allowed to page out?

			Pekka

Re: [PATCH] reiserfs: use __GFP_NOFAIL instead of yield and retry loop for allocation

[Andrew Morton]

Pekka J Enberg <penberg@cs.Helsinki.FI> wrote:
>
> Hi,
> 
> Pekka J Enberg <penberg@cs.Helsinki.FI> wrote:
> > >
> > >  -      retry:
> > >  -	jl = kzalloc(sizeof(struct reiserfs_journal_list), GFP_NOFS);
> > >  -	if (!jl) {
> > >  -		yield();
> > >  -		goto retry;
> > >  -	}
> > >  +	jl = kzalloc(sizeof(struct reiserfs_journal_list),
> > >  +		     GFP_NOFS | __GFP_NOFAIL);
> 
> On Thu, 12 Jan 2006, Andrew Morton wrote:
> > yup, that's what __GFP_NOFAIL is for: to consolidate and identify all those
> > places which want to lock up when we're short of memory...  They all need
> > fixing, really.
> 
> Out of curiosity, are there any potential problems with combining GFP_NOFS 
> and __GFP_NOFAIL? Can we really guarantee to give out memory if we're not 
> allowed to page out?
> 

GFP_NOFS increases the risk (relative to GFP_KERNEL) because page reclaim
can do less things than GFP_KERNEL to free memory.

GFP_NOFS allocations can still perform swapspace writes, however.  GFP_NOIO
cannot even do that.

Re: [PATCH] reiserfs: use __GFP_NOFAIL instead of yield and retry loop for allocation

[Hans Reiser]

Do you guys think you could write some nice long comments on these flags
regarding what they mean and the policies for using them?

I gotta tell you, lots of people end up just guessing as best as they can.

Hans

Andrew Morton wrote:

>Pekka J Enberg <penberg@cs.Helsinki.FI> wrote:
>  
>
>>Hi,
>>
>>Pekka J Enberg <penberg@cs.Helsinki.FI> wrote:
>>    
>>
>>>> -      retry:
>>>> -	jl = kzalloc(sizeof(struct reiserfs_journal_list), GFP_NOFS);
>>>> -	if (!jl) {
>>>> -		yield();
>>>> -		goto retry;
>>>> -	}
>>>> +	jl = kzalloc(sizeof(struct reiserfs_journal_list),
>>>> +		     GFP_NOFS | __GFP_NOFAIL);
>>>>        
>>>>
>>On Thu, 12 Jan 2006, Andrew Morton wrote:
>>    
>>
>>>yup, that's what __GFP_NOFAIL is for: to consolidate and identify all those
>>>places which want to lock up when we're short of memory...  They all need
>>>fixing, really.
>>>      
>>>
>>Out of curiosity, are there any potential problems with combining GFP_NOFS 
>>and __GFP_NOFAIL? Can we really guarantee to give out memory if we're not 
>>allowed to page out?
>>
>>    
>>
>
>GFP_NOFS increases the risk (relative to GFP_KERNEL) because page reclaim
>can do less things than GFP_KERNEL to free memory.
>
>GFP_NOFS allocations can still perform swapspace writes, however.  GFP_NOIO
>cannot even do that.
>
>
>  
>