On Wed, Apr 22, 2020 at 01:54:25PM -0400, Brian Foster wrote: > We have two AIL removal functions with slightly different semantics. > xfs_trans_ail_delete() expects the caller to have the AIL lock and > for the associated item to be AIL resident. If not, the filesystem > is shut down. xfs_trans_ail_remove() acquires the AIL lock, checks > that the item is AIL resident and calls the former if so. > > These semantics lead to confused usage between the two. For example, > the _remove() variant takes a shutdown parameter to pass to the > _delete() variant, but immediately returns if the AIL bit is not > set. This means that _remove() would never shut down if an item is > not AIL resident, even though it appears that many callers would > expect it to. > > Make the following changes to clean up both of these functions: > > - Most callers of xfs_trans_ail_delete() acquire the AIL lock just > before the call. Update _delete() to acquire the lock and open > code the couple of callers that make additional checks under AIL > lock. > - Drop the unnecessary ailp parameter from _delete(). > - Drop the unused shutdown parameter from _remove() and open code > the implementation. > > In summary, this leaves a _delete() variant that expects an AIL > resident item and a _remove() helper that checks the AIL bit. Audit > the existing callsites for use of the appropriate function and > update as necessary. > > Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx> In conjunction with the followed patch to combine the remove and delete functions, I'm good with this. Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx> -- Dave Chinner david@xxxxxxxxxxxxx
