Re: f2fs: introduce f2fs_issue_discard() to clean up

Re: f2fs: introduce f2fs_issue_discard() to clean up

[Dan Carpenter]

Hello Jaegeuk Kim,

The patch 3fa7fb17dc9a: "f2fs: introduce f2fs_issue_discard() to clean
up" from Nov 12, 2013, leads to the following static checker warning:
	"fs/f2fs/segment.c:274 f2fs_issue_discard()
	 warn: should 'blklen << sbi->log_sectors_per_block' be a 64 bit
	 type?"

fs/f2fs/segment.c
   270  static void f2fs_issue_discard(struct f2fs_sb_info *sbi,
   271                                  block_t blkstart, block_t blklen)
   272  {
   273          sector_t sector_addr = blkstart << sbi->log_sectors_per_block;
   274          sector_t sector_len = blklen << sbi->log_sectors_per_block;

block_t is a 32 bit type and sector_t is a 64 bit type.  The upper 32
bits of the sector_t are not used because the shift will wrap.

   275  
   276          blkdev_issue_discard(sbi->sb->s_bdev, sector_addr, sector_len,
   277                                                          GFP_NOFS, 0);
   278          trace_f2fs_issue_discard(sbi->sb, blkstart, blklen);
   279  }

regards,
dan carpenter


------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing 
conversations that shape the rapidly evolving mobile landscape. Sign up now. 
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk

Re: f2fs: introduce f2fs_issue_discard() to clean up

[Jaegeuk Kim]

Hi Dan,

Thank you for the report.

How about this?
Thanks,

>From e053b711dcf9424974d232829b9c589269c98b4d Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk.kim@samsung.com>
Date: Tue, 12 Nov 2013 16:55:17 +0900
Subject: [PATCH] f2fs: introduce f2fs_issue_discard() to clean up
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net

Change log from v1:
 o fix 32bit drops reported by Dan Carpenter

This patch adds f2fs_issue_discard() to clean up blkdev_issue_discard()
flows.

Dan carpenter reported:
"block_t is a 32 bit type and sector_t is a 64 bit type.  The upper 32
bits of the sector_t are not used because the shift will wrap."

Bug-Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
---
 fs/f2fs/segment.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 505a889..f95397b 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -266,6 +266,14 @@ static void locate_dirty_segment(struct
f2fs_sb_info *sbi, unsigned int segno)
 	mutex_unlock(&dirty_i->seglist_lock);
 }
 
+static void f2fs_issue_discard(struct f2fs_sb_info *sbi,
+				block_t blkstart, block_t blklen)
+{
+	blkdev_issue_discard(sbi->sb->s_bdev,
+			blkstart << sbi->log_sectors_per_block,
+			blklen << sbi->log_sectors_per_block, GFP_NOFS, 0);
+}
+
 static void add_discard_addrs(struct f2fs_sb_info *sbi,
 			unsigned int segno, struct seg_entry *se)
 {
@@ -354,22 +362,15 @@ void clear_prefree_segments(struct f2fs_sb_info
*sbi)
 		if (!test_opt(sbi, DISCARD))
 			continue;
 
-		blkdev_issue_discard(sbi->sb->s_bdev,
-				START_BLOCK(sbi, start) <<
-				sbi->log_sectors_per_block,
-				(1 << (sbi->log_sectors_per_block +
-				sbi->log_blocks_per_seg)) * (end - start),
-				GFP_NOFS, 0);
+		f2fs_issue_discard(sbi, START_BLOCK(sbi, start),
+				(end - start) << sbi->log_blocks_per_seg);
 	}
 	mutex_unlock(&dirty_i->seglist_lock);
 
 	/* send small discards */
 	list_for_each_safe(this, next, head) {
 		entry = list_entry(this, struct discard_entry, list);
-		blkdev_issue_discard(sbi->sb->s_bdev,
-				entry->blkaddr << sbi->log_sectors_per_block,
-				(1 << sbi->log_sectors_per_block) * entry->len,
-				GFP_NOFS, 0);
+		f2fs_issue_discard(sbi, entry->blkaddr, entry->len);
 		list_del(&entry->list);
 		SM_I(sbi)->nr_discards -= entry->len;
 		kmem_cache_free(discard_entry_slab, entry);
-- 
1.8.4.474.g128a96c



-- 
Jaegeuk Kim
Samsung


------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk

Re: f2fs: introduce f2fs_issue_discard() to clean up

[Dan Carpenter]

On Wed, Nov 27, 2013 at 11:46:21AM +0900, Jaegeuk Kim wrote:
> Hi Dan,
> 
> Thank you for the report.
> 
> How about this?
> Thanks,

You've just disguised the bug instead of fixing it.  Later versions of
this static checker will complain about the new code as well.  We're
unlikely to support 18TB with f2fs so let's just leave the code as is
with the original warning.

regards,
dan carpenter



------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk

Re: f2fs: introduce f2fs_issue_discard() to clean up

[Jaegeuk Kim]

Hi,

2013-11-27 (수), 11:31 +0300, Dan Carpenter:
> On Wed, Nov 27, 2013 at 11:46:21AM +0900, Jaegeuk Kim wrote:
> > Hi Dan,
> > 
> > Thank you for the report.
> > 
> > How about this?
> > Thanks,
> 
> You've just disguised the bug instead of fixing it.  Later versions of
> this static checker will complain about the new code as well.  We're
> unlikely to support 18TB with f2fs so let's just leave the code as is
> with the original warning.

Oops. I was out of mind at that time.
It should be done casting from block_t to sector_t explicitly, since it
can be a problem after the volume size becomes over 2TB, 2^(32-3) * 4KB.
Let me fix the potential problem as follows.
Thanks,

From b07d78e9693dbdb3bb44bdecb190444fade86b39 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk.kim@samsung.com>
Date: Tue, 12 Nov 2013 16:55:17 +0900
Subject: [PATCH] f2fs: introduce f2fs_issue_discard() to clean up
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net

Change log from v1:
 o fix 32bit drops reported by Dan Carpenter

This patch adds f2fs_issue_discard() to clean up blkdev_issue_discard()
flows.

Dan carpenter reported:
"block_t is a 32 bit type and sector_t is a 64 bit type.  The upper 32
bits of the sector_t are not used because the shift will wrap."

Bug-Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
---
 fs/f2fs/segment.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 505a889..c51fa4b 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -266,6 +266,14 @@ static void locate_dirty_segment(struct
f2fs_sb_info *sbi, unsigned int segno)
 	mutex_unlock(&dirty_i->seglist_lock);
 }
 
+static void f2fs_issue_discard(struct f2fs_sb_info *sbi,
+				block_t blkstart, block_t blklen)
+{
+	sector_t start = ((sector_t)blkstart) << sbi->log_sectors_per_block;
+	sector_t len = ((sector_t)blklen) << sbi->log_sectors_per_block;
+	blkdev_issue_discard(sbi->sb->s_bdev, start, len, GFP_NOFS, 0);
+}
+
 static void add_discard_addrs(struct f2fs_sb_info *sbi,
 			unsigned int segno, struct seg_entry *se)
 {
@@ -354,22 +362,15 @@ void clear_prefree_segments(struct f2fs_sb_info
*sbi)
 		if (!test_opt(sbi, DISCARD))
 			continue;
 
-		blkdev_issue_discard(sbi->sb->s_bdev,
-				START_BLOCK(sbi, start) <<
-				sbi->log_sectors_per_block,
-				(1 << (sbi->log_sectors_per_block +
-				sbi->log_blocks_per_seg)) * (end - start),
-				GFP_NOFS, 0);
+		f2fs_issue_discard(sbi, START_BLOCK(sbi, start),
+				(end - start) << sbi->log_blocks_per_seg);
 	}
 	mutex_unlock(&dirty_i->seglist_lock);
 
 	/* send small discards */
 	list_for_each_safe(this, next, head) {
 		entry = list_entry(this, struct discard_entry, list);
-		blkdev_issue_discard(sbi->sb->s_bdev,
-				entry->blkaddr << sbi->log_sectors_per_block,
-				(1 << sbi->log_sectors_per_block) * entry->len,
-				GFP_NOFS, 0);
+		f2fs_issue_discard(sbi, entry->blkaddr, entry->len);
 		list_del(&entry->list);
 		SM_I(sbi)->nr_discards -= entry->len;
 		kmem_cache_free(discard_entry_slab, entry);
-- 
1.8.4.474.g128a96c



-- 
Jaegeuk Kim
Samsung



------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: f2fs: introduce f2fs_issue_discard() to clean up

[Dan Carpenter]

On Wed, Nov 27, 2013 at 06:04:57PM +0900, Jaegeuk Kim wrote:
> Hi,
> 
> 2013-11-27 (수), 11:31 +0300, Dan Carpenter:
> > On Wed, Nov 27, 2013 at 11:46:21AM +0900, Jaegeuk Kim wrote:
> > > Hi Dan,
> > > 
> > > Thank you for the report.
> > > 
> > > How about this?
> > > Thanks,
> > 
> > You've just disguised the bug instead of fixing it.  Later versions of
> > this static checker will complain about the new code as well.  We're
> > unlikely to support 18TB with f2fs so let's just leave the code as is
> > with the original warning.
> 
> Oops. I was out of mind at that time.
> It should be done casting from block_t to sector_t explicitly, since it
> can be a problem after the volume size becomes over 2TB, 2^(32-3) * 4KB.
> Let me fix the potential problem as follows.
> Thanks,

Perfect.  There were some other related warnings as well:

fs/f2fs/segment.c:910 submit_write_page() warn: should 'blk_addr << ((sbi)->log_blocksize - 9)' be a 64 bit type?
fs/f2fs/checkpoint.c:429 get_valid_checkpoint() warn: should '1 << ()' be a 64 bit type?
fs/f2fs/data.c:408 f2fs_readpage() warn: should 'blk_addr << ((sbi)->log_blocksize - 9)' be a 64 bit type?
fs/f2fs/data.c:457 submit_read_page() warn: should 'blk_addr << ((sbi)->log_blocksize - 9)' be a 64 bit type?
fs/f2fs/data.c:525 get_data_block_ro() warn: should 'i << blkbits' be a 64 bit type?

And a couple other unrelated errors.  :)

fs/f2fs/gc.c:667 do_garbage_collect() warn: 'sum_page' isn't an ERR_PTR
fs/f2fs/f2fs.h:795 f2fs_put_page() warn: 'page' isn't an ERR_PTR

regards,
dan carpenter

------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: f2fs: introduce f2fs_issue_discard() to clean up

[Jaegeuk Kim]

Thank you Dan.

I wrote two patches to address the warnings below.
Thanks.

>From 1745cd1e14eff75089a8d88fe07f07549e1f3946 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk.kim@samsung.com>
Date: Thu, 28 Nov 2013 12:44:05 +0900
Subject: [PATCH 1/2] f2fs: bug fix on bit overflow from 32bits to 64bits
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net

This patch fixes some bit overflows by the shift operations.

Dan Carpenter reported potential bugs on bit overflows as follows.

fs/f2fs/segment.c:910 submit_write_page()
	warn: should 'blk_addr << ((sbi)->log_blocksize - 9)' be a 64 bit type?
fs/f2fs/checkpoint.c:429 get_valid_checkpoint()
	warn: should '1 << ()' be a 64 bit type?
fs/f2fs/data.c:408 f2fs_readpage()
	warn: should 'blk_addr << ((sbi)->log_blocksize - 9)' be a 64 bit type?
fs/f2fs/data.c:457 submit_read_page()
	warn: should 'blk_addr << ((sbi)->log_blocksize - 9)' be a 64 bit type?
fs/f2fs/data.c:525 get_data_block_ro()
	warn: should 'i << blkbits' be a 64 bit type?

Bug-Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
---
 fs/f2fs/checkpoint.c | 3 ++-
 fs/f2fs/data.c       | 2 +-
 fs/f2fs/segment.c    | 4 ++--
 fs/f2fs/segment.h    | 4 ++--
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 3e62987..21e7215 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -426,7 +426,8 @@ int get_valid_checkpoint(struct f2fs_sb_info *sbi)
 	cp1 = validate_checkpoint(sbi, cp_start_blk_no, &cp1_version);
 
 	/* The second checkpoint pack should start at the next segment */
-	cp_start_blk_no += 1 << le32_to_cpu(fsb->log_blocks_per_seg);
+	cp_start_blk_no += ((unsigned long long)1) <<
+				le32_to_cpu(fsb->log_blocks_per_seg);
 	cp2 = validate_checkpoint(sbi, cp_start_blk_no, &cp2_version);
 
 	if (cp1 && cp2) {
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 8e2937d..899d78c 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -525,7 +525,7 @@ static int get_data_block_ro(struct inode *inode,
sector_t iblock,
 				!= (dn.data_blkaddr + i)) || maxblocks == i)
 				break;
 		map_bh(bh_result, inode->i_sb, dn.data_blkaddr);
-		bh_result->b_size = (i << blkbits);
+		bh_result->b_size = (((size_t)i) << blkbits);
 	}
 	f2fs_put_dnode(&dn);
 	trace_f2fs_get_data_block(inode, iblock, bh_result, 0);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 1e837139..fc0a10c 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -270,8 +270,8 @@ static void locate_dirty_segment(struct f2fs_sb_info
*sbi, unsigned int segno)
 static void f2fs_issue_discard(struct f2fs_sb_info *sbi,
 				block_t blkstart, block_t blklen)
 {
-	sector_t start = ((sector_t)blkstart) << sbi->log_sectors_per_block;
-	sector_t len = ((sector_t)blklen) << sbi->log_sectors_per_block;
+	sector_t start = SECTOR_FROM_BLOCK(sbi, blkstart);
+	sector_t len = SECTOR_FROM_BLOCK(blklen);
 	blkdev_issue_discard(sbi->sb->s_bdev, start, len, GFP_NOFS, 0);
 	trace_f2fs_issue_discard(sbi->sb, blkstart, blklen);
 }
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index b84dd23..07887e1 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -86,9 +86,9 @@
 #define TOTAL_SECS(sbi)	(sbi->total_sections)
 
 #define SECTOR_FROM_BLOCK(sbi, blk_addr)				\
-	(blk_addr << ((sbi)->log_blocksize - F2FS_LOG_SECTOR_SIZE))
+	(((sector_t)blk_addr) << (sbi)->log_sectors_per_block)
 #define SECTOR_TO_BLOCK(sbi, sectors)					\
-	(sectors >> ((sbi)->log_blocksize - F2FS_LOG_SECTOR_SIZE))
+	(sectors >> (sbi)->log_sectors_per_block)
 #define MAX_BIO_BLOCKS(max_hw_blocks)					\
 	(min((int)max_hw_blocks, BIO_MAX_PAGES))
 
-- 
1.8.4.474.g128a96c

>From 478cabf84b707e4bf857fc6f9d1233511faa4072 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk.kim@samsung.com>
Date: Thu, 28 Nov 2013 12:55:13 +0900
Subject: [PATCH 2/2] f2fs: remove unnecessary condition checks
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net

This patch removes the unnecessary condition checks on:

fs/f2fs/gc.c:667 do_garbage_collect() warn: 'sum_page' isn't an ERR_PTR
fs/f2fs/f2fs.h:795 f2fs_put_page() warn: 'page' isn't an ERR_PTR

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
---
 fs/f2fs/f2fs.h | 2 +-
 fs/f2fs/gc.c   | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 3134259..ea8a05c 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -793,7 +793,7 @@ static inline unsigned int valid_inode_count(struct
f2fs_sb_info *sbi)
 
 static inline void f2fs_put_page(struct page *page, int unlock)
 {
-	if (!page || IS_ERR(page))
+	if (!page)
 		return;
 
 	if (unlock) {
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index b7ad1ec..5fa54c1 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -664,8 +664,6 @@ static void do_garbage_collect(struct f2fs_sb_info
*sbi, unsigned int segno,
 
 	/* read segment summary of victim */
 	sum_page = get_sum_page(sbi, segno);
-	if (IS_ERR(sum_page))
-		return;
 
 	blk_start_plug(&plug);
 
-- 
1.8.4.474.g128a96c




-- 
Jaegeuk Kim
Samsung


------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk

Re: f2fs: introduce f2fs_issue_discard() to clean up

[Dan Carpenter]

On Thu, Nov 28, 2013 at 01:04:47PM +0900, Jaegeuk Kim wrote:
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index b7ad1ec..5fa54c1 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -664,8 +664,6 @@ static void do_garbage_collect(struct f2fs_sb_info
> *sbi, unsigned int segno,
>  
>  	/* read segment summary of victim */
>  	sum_page = get_sum_page(sbi, segno);
> -	if (IS_ERR(sum_page))
> -		return;

Didn't you want to check for NULL here?

regards,
dan carpenter

>  
>  	blk_start_plug(&plug);
>  


------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk

Re: f2fs: introduce f2fs_issue_discard() to clean up

[Jaegeuk Kim]

2013-11-28 (목), 11:08 +0300, Dan Carpenter:
> On Thu, Nov 28, 2013 at 01:04:47PM +0900, Jaegeuk Kim wrote:
> > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> > index b7ad1ec..5fa54c1 100644
> > --- a/fs/f2fs/gc.c
> > +++ b/fs/f2fs/gc.c
> > @@ -664,8 +664,6 @@ static void do_garbage_collect(struct f2fs_sb_info
> > *sbi, unsigned int segno,
> >  
> >  	/* read segment summary of victim */
> >  	sum_page = get_sum_page(sbi, segno);
> > -	if (IS_ERR(sum_page))
> > -		return;
> 
> Didn't you want to check for NULL here?

It was needless, since get_sum_page() returns always valid page.

-- 
Jaegeuk Kim
Samsung



------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: f2fs: introduce f2fs_issue_discard() to clean up

[Dan Carpenter]

On Thu, Nov 28, 2013 at 06:24:41PM +0900, Jaegeuk Kim wrote:
> 2013-11-28 (목), 11:08 +0300, Dan Carpenter:
> > On Thu, Nov 28, 2013 at 01:04:47PM +0900, Jaegeuk Kim wrote:
> > > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> > > index b7ad1ec..5fa54c1 100644
> > > --- a/fs/f2fs/gc.c
> > > +++ b/fs/f2fs/gc.c
> > > @@ -664,8 +664,6 @@ static void do_garbage_collect(struct f2fs_sb_info
> > > *sbi, unsigned int segno,
> > >  
> > >  	/* read segment summary of victim */
> > >  	sum_page = get_sum_page(sbi, segno);
> > > -	if (IS_ERR(sum_page))
> > > -		return;
> > 
> > Didn't you want to check for NULL here?
> 
> It was needless, since get_sum_page() returns always valid page.

Oh yeah.  I wasn't looking carefully.

regards,
dan carpenter

------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel