Re: [PATCH 3/3] raid5: allow r5l_io_unit allocations to fail

[NeilBrown <neilb@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 1950 bytes --]

On Sat, Dec 19 2015, Shaohua Li wrote:

> On Fri, Dec 18, 2015 at 12:25:35PM +0100, Christoph Hellwig wrote:
>> [can you trim your replies please?  I had to trim almost 150 lines of
>>  junk before getting to your reply!]
>> 
>> > >  	spin_unlock_irqrestore(&log->io_list_lock, flags);
>> > > 
>> > > or is that too simplistic?
>> > 
>> > maybe add a new list and run the list at the begining of r5l_do_reclaim().
>> 
>> What's the advantage of another list?
>
> I mean simply waking up reclaim thread doesn't work as r5l_do_reclaim()
> will return if reclaimable == 0. There is no guarantee reclaimable space
> is available in the allocation failure. So we'd better move the retry at
> the begining of r5l_do_reclaim(). If yes, we can't reuse the
> no_space_stripes list.

They certainly are conceptually different lists.  Whether the same
linked list can be used for both is an engineering question.

One list is for updates that cannot be handled yet because the log is
full.  Once the head of the log is cleared and the start pointer
updated, those requests can be handled (or some of them can).

The other list is for updates that cannot be handled yet because a
kmalloc failed.  There is no clear trigger for when to handle those
again so we would need to retry quite frequently.  It would be easy to
miss retrying in rare circumstances...

I wonder if we should have a mempool for these io units too.
We would allocate with GFP_ATOMIC (or similar) so the allocation woult
fail instead of blocking, but we would then know that an allocation
could only fail if there was another request in flight.  So the place
where we free an io_unit would be the obviously correct place to trigger
a retry of the delayed-due-to-mem-allocation-failure stripes.

So I think I would prefer two lists, another mempool, and very well
defined places to retry the two lists.  Is that over-engineering?

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]