* [PATCH] f2fs: move put_gc_inode into gc_mutex @ 2014-11-27 9:42 Changman Lee 2014-11-28 3:55 ` [f2fs-dev] " Jaegeuk Kim 0 siblings, 1 reply; 3+ messages in thread From: Changman Lee @ 2014-11-27 9:42 UTC (permalink / raw) To: linux-fsdevel, linux-f2fs-devel; +Cc: Changman Lee There in no any lock to protect gc_inode list so let's move into gc_mutex, otherwise it might be lost links of list. Signed-off-by: Changman Lee <cm224.lee@samsung.com> --- fs/f2fs/gc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index 657683c9..99e1720 100644 --- a/fs/f2fs/gc.c +++ b/fs/f2fs/gc.c @@ -733,9 +733,9 @@ gc_more: if (gc_type == FG_GC) write_checkpoint(sbi, &cpc); stop: - mutex_unlock(&sbi->gc_mutex); - put_gc_inode(&ilist); + + mutex_unlock(&sbi->gc_mutex); return ret; } -- 1.9.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: move put_gc_inode into gc_mutex 2014-11-27 9:42 [PATCH] f2fs: move put_gc_inode into gc_mutex Changman Lee @ 2014-11-28 3:55 ` Jaegeuk Kim 2014-11-28 6:08 ` Changman Lee 0 siblings, 1 reply; 3+ messages in thread From: Jaegeuk Kim @ 2014-11-28 3:55 UTC (permalink / raw) To: Changman Lee; +Cc: linux-fsdevel, linux-f2fs-devel Hi Changman, On Thu, Nov 27, 2014 at 06:42:54PM +0900, Changman Lee wrote: > There in no any lock to protect gc_inode list so let's move into > gc_mutex, otherwise it might be lost links of list. Could you explain why the links can be lost? Cause the ilist is a local variable. IIRC, the reason why put_gc_inode is called outside of gc_mutex is to avoid deadlock between f2fs_evict_inode and gc operations. I'm not sure it still has a problem, but it is unclear that we have to move put_gc_inode inside gc_mutex. Are you facing with any bug on this? Thanks, > > Signed-off-by: Changman Lee <cm224.lee@samsung.com> > --- > fs/f2fs/gc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > index 657683c9..99e1720 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -733,9 +733,9 @@ gc_more: > if (gc_type == FG_GC) > write_checkpoint(sbi, &cpc); > stop: > - mutex_unlock(&sbi->gc_mutex); > - > put_gc_inode(&ilist); > + > + mutex_unlock(&sbi->gc_mutex); > return ret; > } > > -- > 1.9.1 > > > ------------------------------------------------------------------------------ > Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server > from Actuate! Instantly Supercharge Your Business Reports and Dashboards > with Interactivity, Sharing, Native Excel Exports, App Integration & more > Get technology previously reserved for billion-dollar corporations, FREE > http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: move put_gc_inode into gc_mutex 2014-11-28 3:55 ` [f2fs-dev] " Jaegeuk Kim @ 2014-11-28 6:08 ` Changman Lee 0 siblings, 0 replies; 3+ messages in thread From: Changman Lee @ 2014-11-28 6:08 UTC (permalink / raw) To: Jaegeuk Kim; +Cc: linux-fsdevel, linux-f2fs-devel On Thu, Nov 27, 2014 at 07:55:14PM -0800, Jaegeuk Kim wrote: > Hi Changman, > > On Thu, Nov 27, 2014 at 06:42:54PM +0900, Changman Lee wrote: > > There in no any lock to protect gc_inode list so let's move into > > gc_mutex, otherwise it might be lost links of list. > > Could you explain why the links can be lost? > Cause the ilist is a local variable. Hi Jaegeuk, Oh, I missed ilist is a local variable. Sorry, ignore this patch. Thanks, > > IIRC, the reason why put_gc_inode is called outside of gc_mutex is to avoid > deadlock between f2fs_evict_inode and gc operations. > I'm not sure it still has a problem, but it is unclear that we have to move > put_gc_inode inside gc_mutex. > > Are you facing with any bug on this? > > Thanks, > > > > > Signed-off-by: Changman Lee <cm224.lee@samsung.com> > > --- > > fs/f2fs/gc.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > > index 657683c9..99e1720 100644 > > --- a/fs/f2fs/gc.c > > +++ b/fs/f2fs/gc.c > > @@ -733,9 +733,9 @@ gc_more: > > if (gc_type == FG_GC) > > write_checkpoint(sbi, &cpc); > > stop: > > - mutex_unlock(&sbi->gc_mutex); > > - > > put_gc_inode(&ilist); > > + > > + mutex_unlock(&sbi->gc_mutex); > > return ret; > > } > > > > -- > > 1.9.1 > > > > > > ------------------------------------------------------------------------------ > > Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server > > from Actuate! Instantly Supercharge Your Business Reports and Dashboards > > with Interactivity, Sharing, Native Excel Exports, App Integration & more > > Get technology previously reserved for billion-dollar corporations, FREE > > http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk > > _______________________________________________ > > Linux-f2fs-devel mailing list > > Linux-f2fs-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-11-28 6:08 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-11-27 9:42 [PATCH] f2fs: move put_gc_inode into gc_mutex Changman Lee 2014-11-28 3:55 ` [f2fs-dev] " Jaegeuk Kim 2014-11-28 6:08 ` Changman Lee
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).