On Tue 16-06-15 08:42:27, Josef Bacik wrote:
> >> /*
> >>- * Wait for writeback on an inode to complete. Caller must have inode pinned.
> >>+ * Wait for writeback on an inode to complete during eviction. Caller must have
> >>+ * inode pinned.
> >> */
> >> void inode_wait_for_writeback(struct inode *inode)
> >> {
> >>+ BUG_ON(!(inode->i_state & I_FREEING));
> >>+
> >> spin_lock(&inode->i_lock);
> >> __inode_wait_for_writeback(inode);
> >> spin_unlock(&inode->i_lock);
> >>+
> >>+ /*
> >>+ * bd_inode's will have already had the bdev free'd so don't bother
> >>+ * doing the bdi_clear_inode_writeback.
> >>+ */
> >>+ if (!sb_is_blkdev_sb(inode->i_sb))
> >>+ bdi_clear_inode_writeback(inode_to_bdi(inode), inode);
> >> }
> >
> >Why do we bother with not putting bdev inode back?
> >
>
> My memory is rusty on this, but if the inode is the inode for a bdev
> we will have already free'd the bdev at this point and we get a null
> pointer deref, so this just skips that bit.
Ah, the reason likely is that bdev->bd_disk is NULL (already cleaned up in
__blkdev_put()) at this moment and thus bdev_get_queue() called from
inode_to_bdi() will oops. Can you please add these details to the comment?
It's a bit subtle...
Also we shouldn't have any pages in the block device mapping anymore
because of the work done in __blkdev_put() (and thus inode shouldn't be in
the writeback list) but I'd be calmer if we asserted
list_empty(&inode->i_wb_list). Can you please add that? Thanks!
Honza
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
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
