Re: [TRIVIAL PATCH 2.4.20] madvise_willneed makes bad limit comparison

Andrew Morton (akpm@digeo.com)
Sun, 08 Dec 2002 22:29:41 -0800


Kingsley Cheung wrote:
>
> Hi,
>
> 'madvise_willneed' makes an incorrect rss limit comparison. It
> directly compares rlim[RLIMIT_RSS].rlim_cur to rss. The former is in
> bytes, whereas the latter is in pages. The fix for this is trivial.
>

It's surely a bug, but looking at the code, one does ask "what
on earth is it trying to do"?

1) -EIO is not a recognised (or appropriate) return value.

2) If the MADV_WILLNEED call fails, all the user needs to do is to
use a smaller chunk, and walk across the file using that chunk
size! The only system-protecting limit here is the request queue
size.

3) We don't know that the application will try to map all that readahead
at the same time anyway. And if it does, the rlimits will catch it.

Linus used "half the size of the inactive list" in sys_readahead. That's
probably as good as anything else. I'd suggest that we just share
that bit of code in madvise.

hmm. Also the new readahead code will allocate all that memory up-front
before putting it under I/O. I'll fix up do_page_cache_readahead()
for that.

> [As an aside, one question is whether this limit check is needed at
> all. Most rss limit enforcement implementations that I've seen are
> 'soft', whereas this would give the limit 'hard' semantics. Do we
> really want 'hard' limit semantics?]
>

I agree that failing with an error is inappropriate.

We should limit the readahead according to machine size, disk bandwidth,
free memory availability, shoe size, etc. And once that's done then
it _has_ to return success. Otherwise the application would see
different results depending on system size and activity.

It is just "advice".
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/