On 2004.11.18 00:22, Nathan Scott wrote:
On Mon, Nov 15, 2004 at 11:31:32AM +0100, Miquel van Smoorenburg wrote:
> I've sent this to the list a month ago, but I think it was overlooked.
Sorry about that.
> I think this (actually, the second patch) should go as a bugfix
> into 2.6.10 .. So I'm reposting it, if I'm completely wrong about
> this tell me and I'll shut up. Thanks!
Yep, I agree we can certainly improve things, and your patches are
a move in the right direction. A couple of things:
- I'd prefer to keep the loop, we've been bitten before by "nofail"
allocations, uhm, failing, and ultimately causing corruption.
- should also consider printk_ratelimit, but not clear if that'd be
better "in addition to" or "in place of" the approaches you've taken
so far here.
- it'd be better to keep the manipulation of "retries", esp. the
modulo part till after the first allocation attempt, since the usual
case is for the allocation to succeed, so the fewer instructions we
add in beforehand the better.
- theres a similar spot in xfs_buf.c where we need to retry page
cache allocations when initial attempts don't work out - maybe the
block_congestion check would be useful there too? Not clear to me
yet though, we may get ourselves in a bind adding that there, but
probably not.
OK, did that. This time I kept as much of the code intact as possible.
Things I changed:
- in linux-2.6/kmem.c:
o add __GFP_NOWARN to gfp flags so that kmalloc/vmalloc don't
spew warnings when we're in a loop
o added call to blk_congestion_wait() at the end of the loop
o made error printk a bit clearer
- in linux-2.6/xfs_buf.c
o add __GFP_NOWARN to gfp flags so that alloc_pages() doesn't
spew warnings when we're in a loop
o added call to blk_congestion_wait() at the end of the loop
o made error printk a bit clearer
o since there was already a 10 ms delay in there, if
blk_congestion_wait() waited for less than 10 ms make sure
we wait for the remaining amount of time (which is 20 ms now).
I'm not sure if we actually need the sleep-at-least-10-ms code,
but I'm not sure what it does and at least this way, actual
behaviour isn't changed much.
Patch has been tested on a busy usenet newsserver.
Mike.
xfs-shutup-kmem.patch3
Description: Text Data
|