Re: [patch 3/6] fs: remove dependency on __GFP_NOFAIL

[David Rientjes <rientjes@google.com>]

On Fri, 23 Jul 2010, Andrew Morton wrote:

> > The kmalloc() in bio_integrity_prep() is failable, so remove __GFP_NOFAIL
> > from its mask.
> > 
> > Cc: Jens Axboe <jens.axboe@oracle.com>
> > Signed-off-by: David Rientjes <rientjes@google.com>
> > ---
> >  fs/bio-integrity.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
> > --- a/fs/bio-integrity.c
> > +++ b/fs/bio-integrity.c
> > @@ -413,7 +413,7 @@ int bio_integrity_prep(struct bio *bio)
> >  
> >  	/* Allocate kernel buffer for protection data */
> >  	len = sectors * blk_integrity_tuple_size(bi);
> > -	buf = kmalloc(len, GFP_NOIO | __GFP_NOFAIL | q->bounce_gfp);
> > +	buf = kmalloc(len, GFP_NOIO | q->bounce_gfp);
> >  	if (unlikely(buf == NULL)) {
> >  		printk(KERN_ERR "could not allocate integrity buffer\n");
> >  		return -EIO;
> 
>                         ^^^  what?
> 

Right, I'm not sure why that decision was made, but it looks like it can 
be changed over to -ENOMEM without harming anything.  I'm concerned that 
the printk will spam the kernel log endlessly, though, if we're really oom 
and GFP_NOIO has no hope of freeing memory.  This code has never been 
active, so I'd like to wait for some feedback from Al and Jens (now with a 
corrected email address, jens.axboe@oracle.com bounced) to see if we want 
to return -ENOMEM, if the printk is really necessary, and if it would be 
better to just convert this to a loop with a congestion_wait() instead of 
returning from bio_integrity_prep().

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>