On Mon, 2005-04-18 at 11:53 +0100, Christoph Hellwig wrote: > The VFS already has a method for freeing an struct inode pointer, and that > is ->destroy_inode. You're probably better off updating your GC state from > that place. destroy_inode() does not help. JFFS2 already makes use of clear_inode() which is in fact called even earlier (inode.c from 2.6.11.5, line 298):
static void dispose_list(struct list_head *head) { int nr_disposed = 0; while (!list_empty(head)) { struct inode *inode; inode = list_entry(head->next, struct inode, i_list); list_del(&inode->i_list); if (inode->i_data.nrpages) truncate_inode_pages(&inode->i_data, 0); clear_inode(inode); /* <--------- here --------- */ destroy_inode(inode); nr_disposed++; } spin_lock(&inode_lock); inodes_stat.nr_inodes -= nr_disposed; spin_unlock(&inode_lock); } I'll repeat my problem, please look tat the code and read my comments in it (inode.c:443): static void prune_icache(int nr_to_scan) { LIST_HEAD(freeable); int nr_pruned = 0; int nr_scanned; unsigned long reap = 0; down(&iprune_sem); spin_lock(&inode_lock); for (nr_scanned = 0; nr_scanned < nr_to_scan; nr_scanned++) { struct inode *inode; if (list_empty(&inode_unused)) break; inode = list_entry(inode_unused.prev, struct inode, i_list); if (inode->i_state || atomic_read(&inode->i_count)) { list_move(&inode->i_list, &inode_unused); continue; } if (inode_has_buffers(inode) || inode->i_data.nrpages) { __iget(inode); spin_unlock(&inode_lock); if (remove_inode_buffers(inode)) reap += invalidate_inode_pages(&inode- >i_data); iput(inode); spin_lock(&inode_lock); if (inode != list_entry(inode_unused.next, struct inode, i_list)) continue; /* wrong inode or list_empty */ if (!can_unuse(inode)) continue; } hlist_del_init(&inode->i_hash); list_move(&inode->i_list, &freeable); /* we have removed inode from the inode hash and from this moment forth it is not in the inode cache */ inode->i_state |= I_FREEING; nr_pruned++; } inodes_stat.nr_unused -= nr_pruned; spin_unlock(&inode_lock); /* we unlocked the spinlock an may be, say, preempted here. now inodes are not in the inode cache, but clear_inode()/destroy_inode() hasn't been called yet. JFFS2 thinks the inode in the inode cache and makes wrong things */ dispose_list(&freeable); up(&iprune_sem); if (current_is_kswapd()) mod_page_state(kswapd_inodesteal, reap); else mod_page_state(pginodesteal, reap); } > Umm, no. It's absolutely not a good reason. What jffs2 is doing right > now is to poke into VFS internals it shouldn't, and exporting more internals > to prevent it from doing so isn't making the situation any better. I fully agree with you that we ought to be very picky about exports. But this mutex is special case - it protects from races with the "external" kswapd. I suspect I'm not the last person who will ask to export it. If you or David suggest a workaround, you're welcome. Cheers, Artem. -- Best Regards, Artem B. Bityuckiy, St.-Petersburg, Russia. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/