Messages in this thread | | | Date | Wed, 14 Mar 2007 21:36:01 -0400 | From | Xiaoning Ding <> | Subject | Re: [PATCH] mm/filemap.c: unconditionally call mark_page_accessed |
| |
Dave Kleikamp wrote: > On Wed, 2007-03-14 at 22:33 +0100, Andreas Mohr wrote: >> Hi, >> >> On Wed, Mar 14, 2007 at 03:55:41PM -0500, Dave Kleikamp wrote: >>> On Wed, 2007-03-14 at 15:58 -0400, Ashif Harji wrote: >>>> This patch unconditionally calls mark_page_accessed to prevent pages, >>>> especially for small files, from being evicted from the page cache despite >>>> frequent access. >>> I guess the downside to this is if a reader is reading a large file, or >>> several files, sequentially with a small read size (smaller than >>> PAGE_SIZE), the pages will be marked active after just one read pass. >>> My gut says the benefits of this patch outweigh the cost. I would >>> expect real-world backup apps, etc. to read at least PAGE_SIZE. >> I also think that the patch is somewhat problematic, since the original >> intention seems to have been a reduction of the number of (expensive?) >> mark_page_accessed() calls, > > mark_page_accessed() isn't expensive. If called repeatedly, starting > with the third call, it will check two page flags and return. The only > real expense is that the page appears busier than it may be and will be > retained in memory longer than it should. > If we allow mark_page_accessed() called multiple times for a single page, a scan of large file with small-size reads would flush the buffer cache. mark_page_accessed() also requests lru_lock when moving page from inactive_list to active_list. It may also increase lock contention.
>> but this of course falls flat on its face in case >> of permanent single-page accesses or accesses with progressing but very small >> read size (single-byte reads or so), since the cached page content will expire >> eventually due to lack of mark_page_accessed() updates; thus this patch >> decided to call mark_page_accessed() unconditionally which may be a large >> performance penalty for subsequent tiny-sized reads. > > Any application doing many tiny-sized reads isn't exactly asking for > great performance. > >> I've been thinking hard how to avoid the mark_page_accessed() starvation in >> case of a fixed, (almost) non-changing access state, but this seems hard since >> it'd seem we need some kind of state management here to figure out good >> intervals of when to call mark_page_accessed() *again* for this page. E.g. >> despite non-changing access patterns you could still call mark_page_accessed() >> every 32 calls or so to avoid expiry, but this would need extra helper >> variables. >> >> A rather ugly way to do it may be to abuse ra.cache_hit or ra.mmap_hit content >> with a >> if ((prev_index != index) || (ra.cache_hit % 32 == 0)) >> mark_page_accessed(page); >> This assumes that ra.cache_hit gets incremented for every access (haven't >> checked whether this is the case). >> That way (combined with an enhanced comment properly explaining the dilemma) >> you would avoid most mark_page_accessed() invocations of subsequent same-page reads >> but still do page status updates from time to time to avoid page deprecation. >> >> Does anyone think this would be acceptable? Any better idea? > > I wouldn't go looking for anything more complicated than Ashif's patch, > unless testing shows it to be harmful in some realistic workload. > >> Andreas Mohr >> >> P.S.: since I'm not too familiar with this area I could be rather wrong after all... > > I could be missing something as well. :-) > > Shaggy
- 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/
| |