linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Bo Zhang <zhangbo0325@gmail.com>
To: fmayle@google.com
Cc: jack@suse.cz, akpm@linux-foundation.org, david@kernel.org,
	kaleshsingh@google.com, linux-fsdevel@vger.kernel.org,
	linux-mm@kvack.org, ljs@kernel.org, lkp@intel.com,
	oe-lkp@lists.linux.dev, oliver.sang@intel.com, surenb@google.com,
	willy@infradead.org, baohua@kernel.org,
	Bo Zhang <zhangbo56@xiaomi.com>
Subject: Re: [linux-next:master] [mm] 7b32f64bc5: pts.svt-av1.Preset13.Bosphorus4K.frames_per_second 45.8% regression
Date: Mon, 27 Jul 2026 09:46:10 +0800	[thread overview]
Message-ID: <20260727014610.3479784-1-zhangbo56@xiaomi.com> (raw)
In-Reply-To: <CAHCxdc64d-6qNBwwSZX8mdz4-V+88No2x0wG4mex5WPXBDE9AA@mail.gmail.com>

On Thu, Jul 23, 2026 at 01:34:38PM -0700, Frederick Mayle wrote:
> I maybe missing something subtle, but, I think you've changed it from "don't
> read beyond the VMA" to "if there is a chance we could read beyond the VMA,
> don't read beyond the VMA", which seems like a more complex expression of the
> same behavior.

Hi Frederick,

The subtlety is in how async readahead interacts with _max_index. The two
are not equivalent because async readahead sets ra->start to the end of
the previous readahead window, not to vmf->pgoff. So even though the user
is still faulting well inside the VMA, the readahead target (ra->start)
has already moved past the VMA end, however _max_index blocks it.

Here is the concrete scenario (ra_pages=128, VMA covers pages 0-1023):

  1. Fault at page 0 triggers sync readahead: reads pages 0-127,
     places PG_readahead marker at page ~96.

  2. Sequential faults continue. Fault at page 96 hits PG_readahead,
     triggers async readahead in do_async_mmap_readahead().

  3. page_cache_async_ra() detects sequential pattern (index == expected)
     and does:
       ra->start += ra->size;  /* pushes start to end of previous window */
     So ra->start = 128 (NOT vmf->pgoff=96), ra->size = 256 which is doubled

  4. page_cache_ra_order() calculates:
       limit = min(file_end, ractl->_max_index)

  With the unconditional approach:
     _max_index = 1023 (always set)
     limit = 1023, ra->start(128) < limit (It works fine here)

  But after several ramp-ups, when the window reaches the boundary:

  5. Fault at page ~896 hits PG_readahead, async readahead fires.
     page_cache_async_ra() does ra->start += ra->size:
       ra->start = 1024  (previous window ended at 1023)
       ra->size  = 128

  6. page_cache_ra_order():
       limit = min(file_end, 1023) = 1023
       ra->start(1024) > limit(1023) (Which will read NOTHING)

  Result: page 1024 (in VMA2) is never prefetched. When the process
  enters VMA2, it takes a major fault and readahead ramps up from scratch.

With my conditional approach:
  At step 5, vmf->pgoff=896, vma_pages_left = 1024-896 = 128
  128 < 128 (the condition is false), so _max_index stays ULONG_MAX
  ra->start(1024) < ULONG_MAX, and it will prefetch pages 1024-1151 normally.
  Page 1024 is already cached when VMA2 is entered, only minor fault happens.

The key point: when mprotect splits a large file mapping into adjacent
VMAs (common for ELF segments, or read-then-write patterns), there's no
benefit in preventing readahead from crossing the boundary, so the data is
still sequential in the file. The limit only helps when we're actually
near the end of useful data (within ra_pages of the VMA end).

I confirmed this with a test program that mmap's a 256MB file and uses
mprotect to split it into 4MB segments (simulating mprotect-split VMAs):

  #include <stdio.h>
  #include <stdlib.h>
  #include <fcntl.h>
  #include <sys/mman.h>
  #include <time.h>
  #include <unistd.h>

  #define FILE_SIZE (256UL * 1024 * 1024)
  #define SEGMENT_SIZE (4UL * 1024 * 1024)

  int main() {
      int fd = open("/tmp/testfile", O_RDONLY);
      char *addr = mmap(NULL, FILE_SIZE, PROT_READ, MAP_PRIVATE, fd, 0);

      /* Split into 4MB VMAs via alternating mprotect */
      for (size_t off = 0; off < FILE_SIZE; off += SEGMENT_SIZE * 2)
          if (off + SEGMENT_SIZE < FILE_SIZE)
              mprotect(addr + off + SEGMENT_SIZE, SEGMENT_SIZE,
                       PROT_READ | PROT_WRITE);

      /* Sequential read across all VMA boundaries */
      struct timespec start, end;
      volatile unsigned long sum = 0;
      clock_gettime(CLOCK_MONOTONIC, &start);
      for (size_t i = 0; i < FILE_SIZE; i += 4096)
          sum += addr[i];
      clock_gettime(CLOCK_MONOTONIC, &end);

      double ms = (end.tv_sec - start.tv_sec) * 1000.0 +
                  (end.tv_nsec - start.tv_nsec) / 1e6;
      printf("%.1f ms (%.1f MB/s)\n", ms, 256000.0 / ms);
      munmap(addr, FILE_SIZE);
      close(fd);
  }

Results on Qualcomm SM8850 mobile phone (ra_pages=128, UFS 4.0),
sequential read across 64 mprotect-split VMAs (256MB file, 4MB segments):

  Without VMA limit (baseline):                  ~158 ms, ~1620 MB/s
  With unconditional _max_index (7b32f64bc512):  ~180 ms, ~1420 MB/s
  With conditional _max_index (this fix):        ~158 ms, ~1620 MB/s

The unconditional approach shows ~13% throughput regression for this
workload due to readahead stalling at every 4MB VMA boundary. The
conditional approach has zero measurable regression while still limiting
readahead for the last ra_pages of each VMA (the original goal).

Does this clarify the difference? The conditional check is "don't limit 
when the async readahead mechanism would set ra->start beyond the VMA
boundary and get blocked, even though the actual fault is still well
within the VMA."

Thanks,
Bo


  reply	other threads:[~2026-07-27  1:46 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-18  8:00 [linux-next:master] [mm] 7b32f64bc5: pts.svt-av1.Preset13.Bosphorus4K.frames_per_second 45.8% regression kernel test robot
2026-06-18  9:30 ` Jan Kara
2026-06-18 14:30   ` Lorenzo Stoakes
2026-06-18 16:03     ` Suren Baghdasaryan
2026-06-18 16:32       ` Pedro Falcato
2026-06-18 16:51         ` Suren Baghdasaryan
2026-06-19 11:11         ` Lorenzo Stoakes
2026-06-29 10:35     ` Lorenzo Stoakes
2026-07-23  2:45 ` Bo Zhang
2026-07-23 20:34   ` Frederick Mayle
2026-07-27  1:46     ` Bo Zhang [this message]
2026-07-28 10:33       ` Pedro Falcato
2026-07-28 14:09         ` Bo Zhang
2026-07-28  5:25     ` Bo Zhang
2026-07-27  2:17   ` Oliver Sang
2026-07-27  4:20     ` Bo Zhang
2026-07-28  2:22       ` Oliver Sang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260727014610.3479784-1-zhangbo56@xiaomi.com \
    --to=zhangbo0325@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=baohua@kernel.org \
    --cc=david@kernel.org \
    --cc=fmayle@google.com \
    --cc=jack@suse.cz \
    --cc=kaleshsingh@google.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=lkp@intel.com \
    --cc=oe-lkp@lists.linux.dev \
    --cc=oliver.sang@intel.com \
    --cc=surenb@google.com \
    --cc=willy@infradead.org \
    --cc=zhangbo56@xiaomi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).