Re: [PATCH 3/4] readahead: factor out duplicated code

From: Ram (linuxram_at_us.ibm.com)
Date: 02/03/05

  • Next message: hui: "Re: [patch, 2.6.11-rc2] sched: RLIMIT_RT_CPU_RATIO feature" 
    To: Oleg Nesterov <oleg@tv-sign.ru>
Date:	Wed, 02 Feb 2005 18:43:14 -0800

    On Sat, 2005-01-29 at 03:35, Oleg Nesterov wrote:
    > > This patch introduces make_ahead_window() function for
    > > simplification of page_cache_readahead.
    >
    > If you will count this patch acceptable, I'll rediff it against
    > next mm iteration.
    >
    > For your convenience here is the code with the patch applied.
    >
    > int make_ahead_window(mapping, filp, ra, int force)
    > {
    > int block, ret;
    >
    > ra->ahead_size = get_next_ra_size(ra);
    > ra->ahead_start = ra->start + ra->size;
    >
    > block = force || (ra->prev_page >= ra->ahead_start);
    > ret = blockable_page_cache_readahead(mapping, filp,
    > ra->ahead_start, ra->ahead_size, ra, block);
    >
    > if (!ret && !force) {

    As steve pointed out this should be :
             if ( !ret && ! block ) {

    > ra->ahead_start = 0;
    > ra->ahead_size = 0;
    > }
    >
    > return ret;
    > }
    >
    > unsigned long page_cache_readahead(mapping, ra, filp, offset, req_size)
    > {
    > unsigned long max, newsize = req_size;
    > int sequential = (offset == ra->prev_page + 1);
    >
    > if (offset == ra->prev_page && req_size == 1 && ra->size != 0)
    > goto out;
    > ra->prev_page = offset;
    > max = get_max_readahead(ra);
    > newsize = min(req_size, max);
    >
    > if (newsize == 0 || (ra->flags & RA_FLAG_INCACHE)) {
    > newsize = 1;

    At this point prev_page has to be updated:
                  ra->prev_page = offset;

    > goto out;
    > }
    >
    > ra->prev_page += newsize - 1;
    >
    > if ((ra->size == 0 && offset == 0) ||
    > (ra->size == -1 && sequential)) {
    > ra->size = get_init_ra_size(newsize, max);
    > ra->start = offset;
    > if (!blockable_page_cache_readahead(mapping, filp, offset, ra->size, ra, 1))
    > goto out;
    >
    > if (req_size >= max)
    > make_ahead_window(mapping, filp, ra, 1);
    >
    > goto out;
    > }
    >
    > if (!sequential || (ra->size == 0)) {
    > ra_off(ra);
    > blockable_page_cache_readahead(mapping, filp, offset, newsize, ra, 1);
    > goto out;
    > }
    >
    >
    > if (ra->ahead_start == 0) {
    > if (!make_ahead_window(mapping, filp, ra, 0))
    > goto out;
    > }
    >
    > if (ra->prev_page >= ra->ahead_start) {
    > ra->start = ra->ahead_start;
    > ra->size = ra->ahead_size;
    > make_ahead_window(mapping, filp, ra, 0);
    > }
    > out:
    > return newsize;
    > }
    Otherwise this code looks much cleaner and correct. Can you send me a
    updated patch. I will run it through my test harness.

    RP

    -
    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/

  • Next message: hui: "Re: [patch, 2.6.11-rc2] sched: RLIMIT_RT_CPU_RATIO feature"

    Relevant Pages