* [PATCH] VFS: Go through the LRU list of inode from head @ 2012-06-21 9:00 Cong Meng 2012-06-21 9:47 ` Wanpeng Li 2012-06-21 9:52 ` Jan Kara 0 siblings, 2 replies; 5+ messages in thread From: Cong Meng @ 2012-06-21 9:00 UTC (permalink / raw) To: Al Viro; +Cc: linux-fsdevel, linux-kernel, Cong Meng Go through the LRU list of inode from head. (I'm not sure whether there is any trick here I doesn't get. If yes, any one could explain it) Signed-off-by: Cong Meng <mc@linux.vnet.ibm.com> --- fs/inode.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/fs/inode.c b/fs/inode.c index 775cbab..aac8449 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -704,7 +704,7 @@ void prune_icache_sb(struct super_block *sb, int nr_to_scan) if (list_empty(&sb->s_inode_lru)) break; - inode = list_entry(sb->s_inode_lru.prev, struct inode, i_lru); + inode = list_entry(sb->s_inode_lru.next, struct inode, i_lru); /* * we are inverting the sb->s_inode_lru_lock/inode->i_lock here, -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] VFS: Go through the LRU list of inode from head 2012-06-21 9:00 [PATCH] VFS: Go through the LRU list of inode from head Cong Meng @ 2012-06-21 9:47 ` Wanpeng Li 2012-06-21 9:52 ` Jan Kara 1 sibling, 0 replies; 5+ messages in thread From: Wanpeng Li @ 2012-06-21 9:47 UTC (permalink / raw) To: Cong Meng; +Cc: Al Viro, linux-fsdevel, linux-kernel, Wanpeng Li On Thu, Jun 21, 2012 at 05:00:27PM +0800, Cong Meng wrote: >Go through the LRU list of inode from head. IIRC, prune_icache_sb is called(as a shrinker in shrink_slab) in page reclaim path, it is used to free icache. Tail entry in LRU(Least Recently Used) list is the least recently used inode object. When the inode cache has to shrink, the kernel removes entries from the tail. > >(I'm not sure whether there is any trick here I doesn't get. If yes, >any one could explain it) > >Signed-off-by: Cong Meng <mc@linux.vnet.ibm.com> >--- > fs/inode.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > >diff --git a/fs/inode.c b/fs/inode.c >index 775cbab..aac8449 100644 >--- a/fs/inode.c >+++ b/fs/inode.c >@@ -704,7 +704,7 @@ void prune_icache_sb(struct super_block *sb, int nr_to_scan) > if (list_empty(&sb->s_inode_lru)) > break; > >- inode = list_entry(sb->s_inode_lru.prev, struct inode, i_lru); >+ inode = list_entry(sb->s_inode_lru.next, struct inode, i_lru); > > /* > * we are inverting the sb->s_inode_lru_lock/inode->i_lock here, >-- >1.7.5.4 > >-- >To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >the body of a message to majordomo@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html >Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] VFS: Go through the LRU list of inode from head 2012-06-21 9:00 [PATCH] VFS: Go through the LRU list of inode from head Cong Meng 2012-06-21 9:47 ` Wanpeng Li @ 2012-06-21 9:52 ` Jan Kara 2012-06-26 7:35 ` mc 1 sibling, 1 reply; 5+ messages in thread From: Jan Kara @ 2012-06-21 9:52 UTC (permalink / raw) To: Cong Meng; +Cc: Al Viro, linux-fsdevel, linux-kernel On Thu 21-06-12 17:00:27, Cong Meng wrote: > Go through the LRU list of inode from head. > > (I'm not sure whether there is any trick here I doesn't get. If yes, > any one could explain it) Look at inode_lru_list_add(). It adds at the head of the list. So you should take from the tail to get the least recently used element... Honza > > Signed-off-by: Cong Meng <mc@linux.vnet.ibm.com> > --- > fs/inode.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/fs/inode.c b/fs/inode.c > index 775cbab..aac8449 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -704,7 +704,7 @@ void prune_icache_sb(struct super_block *sb, int nr_to_scan) > if (list_empty(&sb->s_inode_lru)) > break; > > - inode = list_entry(sb->s_inode_lru.prev, struct inode, i_lru); > + inode = list_entry(sb->s_inode_lru.next, struct inode, i_lru); > > /* > * we are inverting the sb->s_inode_lru_lock/inode->i_lock here, > -- > 1.7.5.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Jan Kara <jack@suse.cz> SUSE Labs, CR ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] VFS: Go through the LRU list of inode from head 2012-06-21 9:52 ` Jan Kara @ 2012-06-26 7:35 ` mc 2012-06-27 16:08 ` Jan Kara 0 siblings, 1 reply; 5+ messages in thread From: mc @ 2012-06-26 7:35 UTC (permalink / raw) To: Jan Kara; +Cc: Al Viro, linux-fsdevel, linux-kernel Quoting Jan Kara <jack@suse.cz>: > On Thu 21-06-12 17:00:27, Cong Meng wrote: >> Go through the LRU list of inode from head. >> >> (I'm not sure whether there is any trick here I doesn't get. If yes, >> any one could explain it) > Look at inode_lru_list_add(). It adds at the head of the list. So you > should take from the tail to get the least recently used element... I still have a quetion about the subsequent code and comment: inode = list_entry(sb->s_inode_lru.prev, struct inode, i_lru); /* * we are inverting the sb->s_inode_lru_lock/inode->i_lock here, * so use a trylock. If we fail to get the lock, just move the * inode to the back of the list so we don't spin on it. */ if (!spin_trylock(&inode->i_lock)) { list_move_tail(&inode->i_lru, &sb->s_inode_lru); continue; } Shouldn't the inode be moved to the head to avoid spin on it? I note that list_move was replaced by list_move_tail purposely in a commit. and below piece of code (at the bottom of prune_icache_sb()): if (inode != list_entry(sb->s_inode_lru.next, struct inode, i_lru)) continue; /* wrong inode or list_empty */ Should the inode be compared against to the tail of the list other than the head after re-get the lru lock? thanks. cong. > > Honza > >> >> Signed-off-by: Cong Meng <mc@linux.vnet.ibm.com> >> --- >> fs/inode.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/fs/inode.c b/fs/inode.c >> index 775cbab..aac8449 100644 >> --- a/fs/inode.c >> +++ b/fs/inode.c >> @@ -704,7 +704,7 @@ void prune_icache_sb(struct super_block *sb, >> int nr_to_scan) >> if (list_empty(&sb->s_inode_lru)) >> break; >> >> - inode = list_entry(sb->s_inode_lru.prev, struct inode, i_lru); >> + inode = list_entry(sb->s_inode_lru.next, struct inode, i_lru); >> >> /* >> * we are inverting the sb->s_inode_lru_lock/inode->i_lock here, >> -- >> 1.7.5.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > Jan Kara <jack@suse.cz> > SUSE Labs, CR ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] VFS: Go through the LRU list of inode from head 2012-06-26 7:35 ` mc @ 2012-06-27 16:08 ` Jan Kara 0 siblings, 0 replies; 5+ messages in thread From: Jan Kara @ 2012-06-27 16:08 UTC (permalink / raw) To: mc; +Cc: Jan Kara, Al Viro, linux-fsdevel, linux-kernel On Tue 26-06-12 03:35:46, mc@linux.vnet.ibm.com wrote: > Quoting Jan Kara <jack@suse.cz>: > > >On Thu 21-06-12 17:00:27, Cong Meng wrote: > >>Go through the LRU list of inode from head. > >> > >>(I'm not sure whether there is any trick here I doesn't get. If yes, > >>any one could explain it) > > Look at inode_lru_list_add(). It adds at the head of the list. So you > >should take from the tail to get the least recently used element... > > I still have a quetion about the subsequent code and comment: > > inode = list_entry(sb->s_inode_lru.prev, struct inode, i_lru); > /* > * we are inverting the sb->s_inode_lru_lock/inode->i_lock here, > * so use a trylock. If we fail to get the lock, just move the > * inode to the back of the list so we don't spin on it. > */ > if (!spin_trylock(&inode->i_lock)) { > list_move_tail(&inode->i_lru, &sb->s_inode_lru); > continue; > } > > Shouldn't the inode be moved to the head to avoid spin on it? Yes, it should. > I note that list_move was replaced by list_move_tail purposely in a commit. Right, you are speaking about Christoph's commit 62a3ddef? I agree that commit looks bogus and should be reverted AFAICT. Christoph? > and below piece of code (at the bottom of prune_icache_sb()): > > if (inode != list_entry(sb->s_inode_lru.next, > struct inode, i_lru)) > continue; /* wrong inode or list_empty */ > > Should the inode be compared against to the tail of the list other > than the head > after re-get the lru lock? And you seem to be right here as well. Thanks for having a look! Honza > >>Signed-off-by: Cong Meng <mc@linux.vnet.ibm.com> > >>--- > >> fs/inode.c | 2 +- > >> 1 files changed, 1 insertions(+), 1 deletions(-) > >> > >>diff --git a/fs/inode.c b/fs/inode.c > >>index 775cbab..aac8449 100644 > >>--- a/fs/inode.c > >>+++ b/fs/inode.c > >>@@ -704,7 +704,7 @@ void prune_icache_sb(struct super_block *sb, > >>int nr_to_scan) > >> if (list_empty(&sb->s_inode_lru)) > >> break; > >> > >>- inode = list_entry(sb->s_inode_lru.prev, struct inode, i_lru); > >>+ inode = list_entry(sb->s_inode_lru.next, struct inode, i_lru); > >> > >> /* > >> * we are inverting the sb->s_inode_lru_lock/inode->i_lock here, > >>-- > >>1.7.5.4 -- Jan Kara <jack@suse.cz> SUSE Labs, CR ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-06-27 16:08 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-06-21 9:00 [PATCH] VFS: Go through the LRU list of inode from head Cong Meng 2012-06-21 9:47 ` Wanpeng Li 2012-06-21 9:52 ` Jan Kara 2012-06-26 7:35 ` mc 2012-06-27 16:08 ` Jan Kara
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).