logfs: Check for the return value after calling find_or_create_page()

PATCH Applied Younger Liu 3 participants 5 messages Jul 16, 2013 – Sep 18, 2013 VFS

lore

Share & tools

lore

Younger Liu

author

2013-07-16 09:38 UTC #0 lore raw

PATCH 1/1 logfs: Check for the return value after calling find_or_create_page()

In get_mapping_page(), after calling find_or_create_page(), the return value should be checked.

Signed-off-by Younger Liu <[email protected]>

---
 fs/logfs/segment.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

fs/logfs/segment.c +2 -1

index 038da09..2641e94 100644
@@ -62,7 +62,8 @@ static struct page *get_mapping_page(struct super_block *sb, pgoff_t index,
 		page = read_cache_page(mapping, index, filler, sb);
 	else {
 		page = find_or_create_page(mapping, index, GFP_NOFS);
-		unlock_page(page);
+		if(page)
+			unlock_page(page);
 	}
 	return page;
 }

-- 1.7.9.7

Prasad Joshi

Applied

2013-07-17 04:23 UTC #1 lore raw

Quoted 5 lines · Younger Liu

In get_mapping_page(), after calling find_or_create_page(), the return value should be checked.
Signed-off-by Younger Liu <[email protected]>

Thanks applied.

Regards,
Prasad

Quoted 21 lines

---
 fs/logfs/segment.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

fs/logfs/segment.c +2 -1

index 038da09..2641e94 100644
@@ -62,7 +62,8 @@ static struct page *get_mapping_page(struct super_block
 *sb, pgoff_t index,
                page = read_cache_page(mapping, index, filler, sb);
        else {
                page = find_or_create_page(mapping, index, GFP_NOFS);
-               unlock_page(page);
+               if(page)
+                       unlock_page(page);
        }
        return page;

}
-- 1.7.9.7

Vyacheslav Dubeyko

2013-07-18 11:57 UTC #2 lore raw

Quoted 16 lines · Prasad Joshi

Quoted 5 lines · Younger Liu
In get_mapping_page(), after calling find_or_create_page(), the return value should be checked.
Signed-off-by Younger Liu <[email protected]>
Thanks applied.
Regards,
Prasad
Quoted 4 lines
---
fs/logfs/segment.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

Quoted 11 lines

fs/logfs/segment.c static struct page *get_mapping_page(struct super_block · line 66

Quoted 11 lines

fs/logfs/segment.c +1 -1

index 038da09..2641e94 100644
@@ -62,7 +62,8 @@ static struct page *get_mapping_page(struct super_block
 *sb, pgoff_t index,
               page = read_cache_page(mapping, index, filler, sb);
       else {
               page = find_or_create_page(mapping, index, GFP_NOFS);
-               unlock_page(page);
+               if(page)

Sorry, but as I understand, this code breaks kernel coding style.

With the best regards,
Vyacheslav Dubeyko.

Quoted 10 lines

Quoted 6 lines
fs/logfs/segment.c +1 -0
+                       unlock_page(page);
       }
       return page;
}
-- 1.7.9.7
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Younger Liu

author

2013-07-23 09:17 UTC #3 lore raw

Quoted 33 lines · Vyacheslav Dubeyko

Quoted 16 lines · Prasad Joshi
Quoted 5 lines · Younger Liu
In get_mapping_page(), after calling find_or_create_page(), the return value should be checked.
Signed-off-by Younger Liu <[email protected]>
Thanks applied.
Regards,
Prasad
Quoted 4 lines
---
fs/logfs/segment.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Quoted 11 lines
fs/logfs/segment.c static struct page *get_mapping_page(struct super_block · line 66
Quoted 11 lines
fs/logfs/segment.c +1 -1
index 038da09..2641e94 100644
@@ -62,7 +62,8 @@ static struct page *get_mapping_page(struct super_block
 *sb, pgoff_t index,
               page = read_cache_page(mapping, index, filler, sb);
       else {
               page = find_or_create_page(mapping, index, GFP_NOFS);
-               unlock_page(page);
+               if(page)
Sorry, but as I understand, this code breaks kernel coding style.

Quoted 16 lines

With the best regards,
Vyacheslav Dubeyko.
Quoted 10 lines
Quoted 6 lines
fs/logfs/segment.c +1 -0
+                       unlock_page(page);
       }
       return page;
}
-- 1.7.9.7
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi, Prasad and Vyacheslav
I modified the coding style, and resent a new patch. Please drop the last patch, and apply this new patch. thanks.

Signed-off-by Younger Liu <[email protected]>

---
 fs/logfs/segment.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

fs/logfs/segment.c +2 -1

index d448a77..7f9b096 100644
@@ -62,7 +62,8 @@ static struct page *get_mapping_page(struct super_block *sb, pgoff_t index,
 		page = read_cache_page(mapping, index, filler, sb);
 	else {
 		page = find_or_create_page(mapping, index, GFP_NOFS);
-		unlock_page(page);
+		if (page)
+			unlock_page(page);
 	}
 	return page;
 }

Signature · 1 line

1.7.9.7

Younger Liu

author

2013-09-18 03:48 UTC #4 lore raw

In reply to Prasad Joshi

Quoted 5 lines · Prasad Joshi

Quoted 4 lines · Younger Liu
In get_mapping_page(), after calling find_or_create_page(), the return value should be checked.

Quoted 7 lines

Quoted 1 line
Signed-off-by Younger Liu <[email protected]>
Thanks applied.
Regards,
Prasad

Hi Prasad,
This patch is not merged into mainline 3.12-rc1. Should I resend this patch?
Younger

Quoted 27 lines

Quoted 18 lines

---
 fs/logfs/segment.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

fs/logfs/segment.c +2 -1

index 038da09..2641e94 100644
@@ -62,7 +62,8 @@ static struct page *get_mapping_page(struct super_block
 *sb, pgoff_t index,
                page = read_cache_page(mapping, index, filler, sb);
        else {
                page = find_or_create_page(mapping, index, GFP_NOFS);
-               unlock_page(page);
+               if(page)
+                       unlock_page(page);
        }
        return page;

Quoted 3 lines

}
-- 1.7.9.7

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html