On Thu, Jan 12, 2017 at 09:05:59AM -0500, Brian Foster wrote: > !need_iolock means the iolock is already held. I guess the name is kind > of confusing. !need_iolock doesn't mean that the lock is unnecessary, it > just means that we're calling from a context where it's already held. > See the xfs_icache_free_eofblocks() call from > xfs_file_buffered_aio_write() for reference. > > I suppose I could add an ASSERT(xfs_isilocked()) after that block to > better document that.. Yeah. In fact I'd prefer to kill that parameter at all, it's horrible. Instead we should always expect the lock and assert that it's held, and have the two current need_iolock = false callers take it manually. This will increase lock hold times a bit for the current need_iolock callers, but the most important one, xfs_release already does a previous unlocked xfs_can_free_eofblocks check, and xfs_inode_free_eofblocks is only called it the inode is tagged, so this should not be an issue (and if it is a xfs_can_free_eofblocks call comes to rescue). Btw, there is a comment incorrectly referring to xfs_free_eofblocks in xfs_inode_free_cowblocks while we're at it. -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html