Re: [patch] vmfixes-2.4.0-test9-B2
From: Andrea Arcangeli <hidden>
Date: 2000-09-25 01:31:28
On Sun, Sep 24, 2000 at 05:09:40PM -0700, Linus Torvalds wrote:
[..] as with the shm_swap() thing this is probably something we do want to fix eventually.
both shm_swap and regular rw_swap_cache have the same deadlock problematic w.r.t. __GFP_IO. We could do that on a raw device, but if we swap on top of the filesystem then we could have deadlock problems again. Really since with the swapfile blocks are just allocated with ext2 we should not deadlock (but maybe some other fs have a lock_super in the get_block path anyway). Thus it's safer not to swapout anything when __GFP_IO is not set. Also some linux/net/* code is using (or better abusing since __GFP_IO originally was only meant as a deadlock avoidance thing not a thing to only shrink the clean cache) GFP_BUFFER to not block (so actually we would hurt networking too by causing _any_ kind of block in a GFP_BUFFER allocation). It would been better to introduce a new flag for allocations that must not block for latency requirements but that wants still to shrink the clean cache (instead of finishing the atomic queue). This is trivially fixable grepping for GFP_BUFFER.
The icache shrinker probably has similar problems with clear_inode.
Yep. And it sure does blocking I/O because it have to sync the dirty inodes.
I suspect that it might be a good idea to try to fix this issue, because
it will probably keep coming up otherwise. And it's likely to be fairly
easily debugged, by just making getblk() have some debugging code that
basically says something like
lock_super()
{
.. do the lock ..
+ current->super_locked++;
}
unlock_super()
{
+ if (current->super_locked < 1)
+ BUG();
+ current->super_locked--;
.. do the unlock ..
}
getblk()
{
+ if (current->super_locked)
+ BUG();
.. do the getblk ..
}BTW (running offtopic), I collected such information in 2.2.x too (but for another reason). ftp://ftp.us.kernel.org/pub/linux/kernel/people/andrea/patches/v2.2/2.2.18pre9/VM-global-2.2.18pre9-6.bz2 I trapped all the down on the inode semaphore in the same way (I called it current->fs_locks for both down and superlock). I'm using such information to know if there's any lock held in the context of the task to know if I can do I/O or not without risking to deadlock on any inode semaphore or on any superblock lock. With that change I could then also use GFP_KERNEL in getblk in 2.2.x (I admit at first I did that :), but then I preferred to stay on the safe side for things like loop that _have_ to work in 2.2.x :). So now we know when we can writepage a dirty MAP_SHARED page in swap_out and we do it from the task that is trying to allocate memory, so the task that is trying to allocate memory will block waiting some dirty buffer to be written in writepage->wakeup_bdflush(1). In 2.2.x (as we do in 2.4.x) we _need_ to writeout the page ourself from swapout (not async queueing into kpiod) because kpiod is completly asynchrous and so without this change GFP was returning, we was allocating memory again, and we was entering GFP again, all at fast rate. In the meantime kpiod was still blocked in mark_buffer_dirty->wakeup_bdflush(1) and then the tasks allocating memory (who thought to have done some progress because it queued many pages into kpiod) was getting killed. Of course then I also killed kpiod since it wasn't necessary anymore and now MAP_SHARED semgments doesn't kill tasks anymore.
and just making it a new rule that you cannot call getblk() with any locks held.
Yes I see it would certainly trap the deadlock cases.
(the superblock lock is quite contended right now, and the reason for that
Right (on large fs is going to be quite painful for scalability) and the BUG would have the benefit of partly solving it. I'm thinking that dropping the superblock lock completly wouldn't be much more difficult than this mid stage. The only cases where we block in critical sections protected by the superblock lock is in getblk/bread (bread calls getblk) and ll_rw_block and mark_buffer_dirty. Once we drop the lock for the first cases it should not be more difficult to drop it completly. Not sure if this is the right moment for those changes though, I'm not worried about ext2 but about the other non-netoworked fses that nobody uses regularly. Andrea -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux.eu.org/Linux-MM/