Re: [PATCH 0/9]

[Alex Elder <elder@inktank.com>]

On 04/04/2013 10:03 PM, Josh Durgin wrote:
> On 04/04/2013 09:16 AM, Alex Elder wrote:
>> (The following patches are available in branch "review/wip-3761"
>> on the ceph-client git respository.)
>>
>> These are actually a few sets of patches but I'm just going to
>> post them as a single series this time.
>>
>>                     -Alex
>>
>> [PATCH 1/9] ceph: use page_offset() in ceph_writepages_start()
>>      Fixes a potential bug in ceph_writepages_start().

. . .

>> [PATCH 9/9] ceph: build osd request message later for writepages
>>      Defers "building" a request message until right before
>>      it's submitted to the osd client to start its execution.
>>      Also stops having the length field in a message header
>>      get updated by the file system code.
> 
> These all look good. The one thing I'm uncertain about is changing
> the mempool allocation failure from a WARN to a BUG, but it seems
> there's no good way to recover at that point.

It's reality.  About 20 lines later, pages is dereferenced.
I think it's better to stop at the point of the failure
and report exactly where it occurred than to (most likely)
crash more mysteriously a little later on.

If we exhaust the mempool, it wasn't big enough, and
that's a bug in the size of the mempool or the design.

Thanks a lot for the review.  More on their way shortly.

					-Alex

> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
>