Re: [PATCH 10/33] readahead: support functions

Wu Fengguang
2006-05-25T11:13:19Z

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