On 06/17/2015 03:34 AM, Jan Kara wrote:
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!
Won't it trip if we never sync before we drop the device tho? So we
write some stuff to the block device, it gets written out, we then drop
the device for whatever reason and boom, hit BUG_ON(&inode->i_wb_list)
because we're still on the writeback list even though it doesn't matter
because this disk is going away. Just an untested theory, what do you
think? If it is possible I suppose I could just add the clear'ing bit
for the bd inode to before we drop bd_disk if that would make you happy.
Thanks,
Josef
--
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
