xfs
[Top] [All Lists]

Re: [PATCH 3/5] repair: handle memory allocation failure from blkmap_gro

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 3/5] repair: handle memory allocation failure from blkmap_grow
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Mon, 10 Oct 2011 10:14:37 -0400
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1318208915-14975-4-git-send-email-david@xxxxxxxxxxxxx>
References: <1318208915-14975-1-git-send-email-david@xxxxxxxxxxxxx> <1318208915-14975-4-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Mon, Oct 10, 2011 at 12:08:33PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> If blkmap_grow fails to allocate a new chunk of memory, it returns
> with a null blkmap. The sole caller of blkmap_grow does not check
> for this failure, and so will segfault if this error ever occurs.

Looks good,

Reviewed-by: Christoph Hellwig <hch@xxxxxx>

Two comments on the code not directly related to your change:

 - it seems like xfs_db has another copy of these blkmap routines,
   which even missed the last round of updates during the repair
   scalability work.  It seems like it should be switched to reuse
   the repair code
 - growing the map by four seems to be incredibly inefficient for
   large files.  Given that the only caller actually knows how many
   entries it processes in that batch we should grow it at least by
   the number, reducing the allocations to one per call to
   process_bmbt_reclist_int.

<Prev in Thread] Current Thread [Next in Thread>