On Fri, Oct 08, 2010 at 04:21:27PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Now that the inode LRU and IO lists are split apart, we can separate
> the locking for them. The IO lists are only ever accessed in the
> context of writeback, so a per-BDI lock for those lists separates
> them out nicely.
I think this description needs some updates. It seems like it's from
Nick's original patch that splits the lock, but at this point we still
have inode_lock anyway.
>
> -static inline struct backing_dev_info *inode_to_bdi(struct inode *inode)
> -{
> - struct super_block *sb = inode->i_sb;
> -
> - if (strcmp(sb->s_type->name, "bdev") == 0)
> - return inode->i_mapping->a_bdi;
> -
> - return sb->s_bdi;
> -}
Please don't extent the scope of this one. Just add a new inode_wb_del
or similar helper to remove and inode from the writeback list.
> struct inode *inode = list_entry(wb->b_io.prev,
> @@ -475,7 +475,6 @@ static int writeback_sb_inodes(struct super_block *sb, struct bdi_writeback *wb,
> redirty_tail(inode);
> continue;
> }
> -
> /*
> * The inode belongs to a different superblock.
> * Bounce back to the caller to unpin this and
spurious whitespace change.
> @@ -484,7 +483,7 @@ static int writeback_sb_inodes(struct super_block *sb, struct bdi_writeback *wb,
> return 0;
> }
>
> - if (inode->i_state & (I_NEW | I_WILL_FREE)) {
> + if (inode->i_state & (I_NEW | I_WILL_FREE | I_FREEING)) {
> requeue_io(inode);
> continue;
> }
What does this have to do with the rest of the patch?
> @@ -495,8 +494,11 @@ static int writeback_sb_inodes(struct super_block *sb, struct bdi_writeback *wb,
> if (inode_dirtied_after(inode, wbc->wb_start))
> return 1;
>
> - BUG_ON(inode->i_state & I_FREEING);
> + spin_lock(&inode->i_lock);
> iref_locked(inode);
> + spin_unlock(&inode->i_lock);
Shouldn't this become a plain iref now?
> +/*
> + * check against I_FREEING as inode writeback completion could race with
> + * setting the I_FREEING and removing the inode from the LRU.
> + */
> +void inode_lru_list_add(struct inode *inode)
> +{
> + spin_lock(&inode_lru_lock);
> + if (list_empty(&inode->i_lru) && !(inode->i_state & I_FREEING)) {
> + list_add(&inode->i_lru, &inode_lru);
> + percpu_counter_inc(&nr_inodes_unused);
> + }
> + spin_unlock(&inode_lru_lock);
> +}
Ah, here you introduce the lru list helpers I suggested earlier. Moving
them earlier in the series probably is a good idea to avoid exporting
nr_inodes_unused, even if the locking for the helpers will change in
this patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
