| Date | Thu, 25 May 2006 19:13:18 +0800 | From | Wu Fengguang <> | Subject | Re: [PATCH 10/33] readahead: support functions |
| |
On Thu, May 25, 2006 at 03:13:16PM +1000, Nick Piggin wrote: > Wu Fengguang wrote: > > >+#ifdef CONFIG_ADAPTIVE_READAHEAD > >+ > >+/* > >+ * The nature of read-ahead allows false tests to occur occasionally. > >+ * Here we just do not bother to call get_page(), it's meaningless anyway. > >+ */ > >+static inline struct page *__find_page(struct address_space *mapping, > >+ pgoff_t offset) > >+{ > >+ return radix_tree_lookup(&mapping->page_tree, offset); > >+} > >+ > >+static inline struct page *find_page(struct address_space *mapping, > >+ pgoff_t offset) > >+{ > >+ struct page *page; > >+ > >+ read_lock_irq(&mapping->tree_lock); > >+ page = __find_page(mapping, offset); > >+ read_unlock_irq(&mapping->tree_lock); > >+ return page; > >+} > > > > > > Meh, this is just open-coded elsewhere in readahead.c; I'd either > open code it, or do a new patch to replace the existing callers. > find_page should be in mm/filemap.c, btw (or include/linux/pagemap.h).
Maybe it should stay in readahead.c.
I got this early warning from Andrew: find_page() is not meant to be a general API, for it can easily be abused.
> >+ > >+/* > >+ * Move pages in danger (of thrashing) to the head of inactive_list. > >+ * Not expected to happen frequently. > >+ */ > >+static unsigned long rescue_pages(struct page *page, unsigned long > >nr_pages) > > > > > > Should probably be in mm/vmscan.c
Maybe. It's a highly specialized function. It protects a continuous range of sequential readahead pages in a file. Do you mean to move it for the zone->lru_lock protected statements?
Regards, Wu - 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/
|