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

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

[Akinobu Mita]

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 <akinobu.mita@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 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)) {

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

[Theodore Tso]

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

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

[Akinobu Mita]

2008/2/17, Theodore Tso <tytso@mit.edu>:
> 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.