On Fri, Jul 23, 2010 at 10:51 PM, David Rientjes <rientjes@xxxxxxxxxx> wrote:
> 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@xxxxxxxxxx>
>> > Signed-off-by: David Rientjes <rientjes@xxxxxxxxxx>
>> > ---
>> > 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@xxxxxxxxxx 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().
Btw, you probably want __GFP_NOWARN here if you expect the allocation
to fail under normal conditions.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
