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

Christoph Hellwig
2011-10-10T14:14:38Z

On Mon, Oct 10, 2011 at 12:08:33PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> 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@lst.de>

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.