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:
signature.asc
Description: PGP signature
