2008-02-17 06:02:33

by Akinobu Mita

Subject: [PATCH 1/3] ext2: improve ext2_readdir() return value

This patch improves ext2_readdir() return value for ext2_get_page() failure
by using the actual result of ext2_get_page().

Signed-off-by: Akinobu Mita <[email protected]>
Cc: Andrew Morton <[email protected]>
---
fs/ext2/dir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Index: 2.6-rc/fs/ext2/dir.c
===================================================================
--- 2.6-rc.orig/fs/ext2/dir.c
+++ 2.6-rc/fs/ext2/dir.c
@@ -299,7 +299,7 @@ ext2_readdir (struct file * filp, void *
"bad page in #%lu",
inode->i_ino);
filp->f_pos += PAGE_CACHE_SIZE - offset;
- return -EIO;
+ return PTR_ERR(page);
}
kaddr = page_address(page);
if (unlikely(need_revalidate)) {

2008-02-17 12:09:47

by Theodore Ts'o

Subject: Re: [PATCH 1/3] ext2: improve ext2_readdir() return value

On Sun, Feb 17, 2008 at 02:56:13PM +0900, Akinobu Mita wrote:
> This patch improves ext2_readdir() return value for ext2_get_page() failure
> by using the actual result of ext2_get_page().

> - return -EIO;
> + return PTR_ERR(page);

The patch is harmless, and maybe even a good thing from an abstract
point of view, but you realize that EIO is the only thing that
ext2_get_page() can return, right?

- Ted

2008-02-17 16:30:57

by Akinobu Mita

Subject: Re: [PATCH 1/3] ext2: improve ext2_readdir() return value

2008/2/17, Theodore Tso <[email protected]>:
> On Sun, Feb 17, 2008 at 02:56:13PM +0900, Akinobu Mita wrote:
> > This patch improves ext2_readdir() return value for ext2_get_page() failure
> > by using the actual result of ext2_get_page().
>
> > - return -EIO;
> > + return PTR_ERR(page);
>
> The patch is harmless, and maybe even a good thing from an abstract
> point of view, but you realize that EIO is the only thing that
> ext2_get_page() can return, right?

There are some cases that ext2_get_page() returns -ENOMEM logically.
(--> read_mapping_page --> read_cache_page --> read_cache_page_async -->
__read_cache_page --> page_cache_alloc_cold, or add_to_page_cache_lru)
But this is not likely to happen in reality.