On 2/4/20 12:21 AM, Christoph Hellwig wrote: >> - ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL)); >> + ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL)); > > I think this is a very bad interface. Either we keep our good old > xfs_isilocked that just operates on the inode and lock flags, or > we use something that gets the actual lock passed. But an interface > that encodes the lock in both the function called and the flags, and > one that doesn't follow neither the XFS lock flags conventions nor > the core kernel convention is just not very useful. I think this came out of Dave's suggestion on the previous patchset, but I agree with you Chrisoph. Even if there is a future reason to split it out into a function for each type, I don't see a reason to do it now, and this interface is awkward. I'd prefer to keep xfs_isilocked() with the current calling convention and just change its internals to use lockdep. Dave spotted a bug in the current implementation, but I think that can be fixed. Splitting out the 3 lock testing functions seems to me like complexity creep that doesn't need to be in this series. Dave, thoughts? -Eric
