Re: [PATCH RFC v2 04/12] btrfs: add btrfs_read_repair_ctrl to record corrupted sectors

[Christoph Hellwig <hch@infradead.org>]

> > the I/O completion work queues, which can be rather deadlock heavy.
> > Never mind that just failing an I/O repair/recovery when we are out
> > of memory seems like a rather bad idea.
> 
> That's why there is a btrfs_read_repair_ctrl::error member.
> 
> If we failed the memory allocation, then we will not do any repair.

And that's a little broken, isn't it?

> To me, memory allocation is a much bigger problem.
> 
> 
> Although we can put the bitmap into btrfs_bio structure, and
> pre-allocate it for every bio we're going to submit.
> 
> But I'm not sure if the pre-allocation way is a good idea, considering
> read-repair should be a relatively code path.

And that is what a mempool is for.

> 
> > it in a new read_repair.c instead of in the giant extent_io.c and
> > inode.c files?
> 
> I was considering putting it into read_repair.c, and since you're also
> mentioning that, I guess it's a good idea now.
> 
> And if we're going to make that structure private, I guess we have to
> pre-allocate it in btrfs_bio as a pointer then.

I really think a mempool is the right choice here.  This gives us a
guaranteed number of objects to make progress, even allowing for more
if memory pressure allows for it.