lkml.org 
[lkml]   [2006]   [May]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 13/14] FS-Cache: Release page->private in failed readahead [try #8]
Date
Andrew Morton <akpm@osdl.org> wrote:

> > SetPageLocked(page);
>
> if (TestSetPagLocked(page))
> BUG();
>
> would make me more comfortable..

That shouldn't be necessary if add_to_page_cache() also doesn't do that, but
if you wish, I can do that - it's the error handling path, so it doesn't
matter too much performancewise.

> > > For the second hunk, is it not possible to do this cleanup in the callback
> > > function?
> >
> > Which callback function?
>
> I was referring to the filler_t thingy. Is it not possible to get control
> of that?

Well, the filler_t thing is generally a_ops->readpage from the caller fs, but
we don't want to call that if add_to_page_cache() failed, and we don't want to
call it if we're just discarding a bunch of pages we've now no intention of
actually reading.

I suppose we could add another callback for ditching pages we don't want to
keep. This has the potential to be called quite a lot because of the way
readahead works on Linux.

> So please, can we have some comments in there which describe the new
> behaviour in a manner sufficient for a maintainer to follow so people don't
> break your stuff?

Okay... I'll add more comments. I should probably also extend the
documentation on releasepage(). It won't be till Monday though.

> > Out of interest, why do we need PG_private to say there's something in
> > page->private? Can't it just be assumed either that if page->private is
> > non-zero or that if a_ops->releasepage() is non-NULL, then we need to
> > "release" the page?
>
> page->private is an unsigned long, not a pointer. The core kernel hence
> cannot determine from its value whether or not it is live. For example, the
> fs might choose to treat it as a bitmap of which-blocks-are-uptodate and
> which-blocks-are-dirty.

Then the second option is still possible (calling releasepage()
unconditionally).

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

\
 
 \ /
  Last update: 2006-05-12 18:26    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site