On Thu, Jan 12, 2017 at 11:33:15PM -0800, Christoph Hellwig wrote: > 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. > I may have lied about iolock holders.. I don't think we have the iolock in the xfs_inactive() case. That said, I don't think there's any harm in doing so there. It may have just been that way since we're breaking down the inode in that context. I agree that this is ugly. I'll try to kill off that param as suggested. > 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). > The need_iolock case is also currently a trylock, so I think we can retain that logic in the release case without being disruptive. > Btw, there is a comment incorrectly referring to xfs_free_eofblocks > in xfs_inode_free_cowblocks while we're at it. I would send that as a standalone patch because it's an entirely separate feature (and causes unnecessary backport churn, even though it's just a comment), so I'm not sure it's really worth fixing with this series. Brian > -- > 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 -- 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
