Messages in this thread | | | From | David Howells <> | Subject | Re: [PATCH 13/14] FS-Cache: Release page->private in failed readahead [try #8] | Date | Fri, 12 May 2006 17:23:18 +0100 |
| |
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/
| |