lkml.org 
[lkml]   [2005]   [Apr]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATC] small VFS change for JFFS2
From
Date
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 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/

\
 
 \ /
  Last update: 2005-04-18 13:49    [W:0.049 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site