From: Alexander Zarochentsev <[email protected]>
Don't let fuse_readpages leave the @pages list not empty when exiting
on error.
Signed-off-by: Alexander Zarochentsev <[email protected]>
Signed-off-by: Miklos Szeredi <[email protected]>
---
Index: linux/fs/fuse/file.c
===================================================================
--- linux.orig/fs/fuse/file.c 2006-08-10 09:41:12.000000000 +0200
+++ linux/fs/fuse/file.c 2006-08-10 11:13:35.000000000 +0200
@@ -395,14 +395,16 @@ static int fuse_readpages(struct file *f
struct fuse_readpages_data data;
int err;
+ err = -EIO;
if (is_bad_inode(inode))
- return -EIO;
+ goto clean_pages_up;
data.file = file;
data.inode = inode;
data.req = fuse_get_req(fc);
+ err = PTR_ERR(data.req);
if (IS_ERR(data.req))
- return PTR_ERR(data.req);
+ goto clean_pages_up;
err = read_cache_pages(mapping, pages, fuse_readpages_fill, &data);
if (!err) {
@@ -412,6 +414,10 @@ static int fuse_readpages(struct file *f
fuse_put_request(fc, data.req);
}
return err;
+
+clean_pages_up:
+ put_pages_list(pages);
+ return err;
}
static size_t fuse_send_write(struct fuse_req *req, struct file *file,
Index: linux/mm/readahead.c
===================================================================
--- linux.orig/mm/readahead.c 2006-08-10 09:41:14.000000000 +0200
+++ linux/mm/readahead.c 2006-08-10 11:12:27.000000000 +0200
@@ -147,13 +147,7 @@ int read_cache_pages(struct address_spac
if (!pagevec_add(&lru_pvec, page))
__pagevec_lru_add(&lru_pvec);
if (ret) {
- while (!list_empty(pages)) {
- struct page *victim;
-
- victim = list_to_page(pages);
- list_del(&victim->lru);
- page_cache_release(victim);
- }
+ put_pages_list(pages);
break;
}
}
Index: linux/include/linux/mm.h
===================================================================
--- linux.orig/include/linux/mm.h 2006-08-10 11:06:05.000000000 +0200
+++ linux/include/linux/mm.h 2006-08-10 11:12:46.000000000 +0200
@@ -336,6 +336,7 @@ static inline void init_page_count(struc
}
void put_page(struct page *page);
+void put_pages_list(struct list_head *pages);
void split_page(struct page *page, unsigned int order);
Index: linux/mm/swap.c
===================================================================
--- linux.orig/mm/swap.c 2006-08-10 11:02:34.000000000 +0200
+++ linux/mm/swap.c 2006-08-10 11:18:59.000000000 +0200
@@ -54,6 +54,24 @@ void put_page(struct page *page)
}
EXPORT_SYMBOL(put_page);
+/**
+ * Release page list. Currently used by read_cache_pages() and related
+ * cleanup code.
+ *
+ * @pages: list of pages threaded on page->lru
+ */
+void put_pages_list(struct list_head *pages)
+{
+ while (!list_empty(pages)) {
+ struct page *victim;
+
+ victim = list_entry(pages->prev, struct page, lru);
+ list_del(&victim->lru);
+ page_cache_release(victim);
+ }
+}
+EXPORT_SYMBOL(put_pages_list);
+
/*
* Writeback is about to end against a page which has been marked for immediate
* reclaim. If it still appears to be reclaimable, move it to the tail of the
On Thu, 10 Aug 2006 11:28:48 +0200
Miklos Szeredi <[email protected]> wrote:
> From: Alexander Zarochentsev <[email protected]>
>
> Don't let fuse_readpages leave the @pages list not empty when exiting
> on error.
>
Oh dear. read_cache_pages_release_page() is not a readahead thing. It is
a dhowells fscache thing.
>
> Index: linux/fs/fuse/file.c
> ===================================================================
> --- linux.orig/fs/fuse/file.c 2006-08-10 09:41:12.000000000 +0200
> +++ linux/fs/fuse/file.c 2006-08-10 11:13:35.000000000 +0200
> @@ -395,14 +395,16 @@ static int fuse_readpages(struct file *f
> struct fuse_readpages_data data;
> int err;
>
> + err = -EIO;
> if (is_bad_inode(inode))
> - return -EIO;
> + goto clean_pages_up;
>
> data.file = file;
> data.inode = inode;
> data.req = fuse_get_req(fc);
> + err = PTR_ERR(data.req);
> if (IS_ERR(data.req))
> - return PTR_ERR(data.req);
> + goto clean_pages_up;
>
> err = read_cache_pages(mapping, pages, fuse_readpages_fill, &data);
> if (!err) {
> @@ -412,6 +414,10 @@ static int fuse_readpages(struct file *f
> fuse_put_request(fc, data.req);
> }
> return err;
> +
> +clean_pages_up:
> + put_pages_list(pages);
> + return err;
> }
>
> static size_t fuse_send_write(struct fuse_req *req, struct file *file,
> Index: linux/mm/readahead.c
> ===================================================================
> --- linux.orig/mm/readahead.c 2006-08-10 09:41:14.000000000 +0200
> +++ linux/mm/readahead.c 2006-08-10 11:12:27.000000000 +0200
> @@ -147,13 +147,7 @@ int read_cache_pages(struct address_spac
> if (!pagevec_add(&lru_pvec, page))
> __pagevec_lru_add(&lru_pvec);
> if (ret) {
> - while (!list_empty(pages)) {
> - struct page *victim;
> -
> - victim = list_to_page(pages);
> - list_del(&victim->lru);
> - page_cache_release(victim);
> - }
> + put_pages_list(pages);
> break;
> }
> }
So I shall drop the above hunk for now.
> Index: linux/include/linux/mm.h
> ===================================================================
> --- linux.orig/include/linux/mm.h 2006-08-10 11:06:05.000000000 +0200
> +++ linux/include/linux/mm.h 2006-08-10 11:12:46.000000000 +0200
> @@ -336,6 +336,7 @@ static inline void init_page_count(struc
> }
>
> void put_page(struct page *page);
> +void put_pages_list(struct list_head *pages);
>
> void split_page(struct page *page, unsigned int order);
>
> Index: linux/mm/swap.c
> ===================================================================
> --- linux.orig/mm/swap.c 2006-08-10 11:02:34.000000000 +0200
> +++ linux/mm/swap.c 2006-08-10 11:18:59.000000000 +0200
> @@ -54,6 +54,24 @@ void put_page(struct page *page)
> }
> EXPORT_SYMBOL(put_page);
>
> +/**
> + * Release page list. Currently used by read_cache_pages() and related
> + * cleanup code.
> + *
> + * @pages: list of pages threaded on page->lru
> + */
> +void put_pages_list(struct list_head *pages)
> +{
> + while (!list_empty(pages)) {
> + struct page *victim;
> +
> + victim = list_entry(pages->prev, struct page, lru);
> + list_del(&victim->lru);
> + page_cache_release(victim);
> + }
> +}
> +EXPORT_SYMBOL(put_pages_list);
> +
> /*
> * Writeback is about to end against a page which has been marked for immediate
> * reclaim. If it still appears to be reclaimable, move it to the tail of the
Andrew Morton <[email protected]> wrote:
> > Don't let fuse_readpages leave the @pages list not empty when exiting
> > on error.
> >
>
> Oh dear. read_cache_pages_release_page() is not a readahead thing. It is
> a dhowells fscache thing.
The pages passed to read_cache_pages() may already be marked with metadata
information (such as to-be-cached marks from NFS or AFS calling FS-Cache), so
we can't just ditch the pages on an error, we have to clean them up properly.
David