Messages in this thread | | | Date | Fri, 01 Aug 1997 12:41:26 -0400 | From | Bill Hawes <> | Subject | Re: Simple fix for swap_out (the 1st August patch) |
| |
Dr. Werner Fink wrote: > > NOTE: The appended patch is against pre-patch-2.0.31-2.
I can't comment on the swap code changes (haven't studied that yet), but I would like to point out what I believe is a mistake in pre-patch-2.0.31-2. In 2.0.29 and 2.0.30 buffer.c the condition for waking up bdflush looks like
if (!(grow_buffers(GFP_ATOMIC,size))) wakeup_bdflush(1); needed -= PAGE_SIZE; goto repeat;
This was changed to something like: if ((grow_buffers(GFP_ATOMIC,size))) needed -= PAGE_SIZE; else wakeup_bdflush(1); goto repeat;
On the surface the original code looks erroneous, but intentional or not, the original code has the effect of reducing the memory goal _even if no memory was allocated_. This means that tasks trapped in "refill_freelist hell" have a chance of escaping under 2.0.29 or unpatched 2.0.30, but may be forever trapped under the new code.
If you've looked at the changes I've made to 2.1.47 fs/buffer.c, it's important to get tasks out of refill_freelist even if they haven't reached their full quota of buffers. Otherwise, you can reach a deadlock state where the tasks wanting buffers have all fallen into refill_freelist, and none can make their quota.
I would much prefer to see the code written as something like: grow_buffers(GFP_ATOMIC,size); if (free_list[BUFSIZE_INDEX(size)] != NULL) return; wakeup_bdflush(1); needed -= PAGE_SIZE; goto repeat;
i.e a direct test of success, but at the very least would recommend that the original code be used.
Regards, Bill
| |