+	int			freed;

I think this should be a bool..

 	bool			aborted;
 	bool			hold = !!(bip->bli_flags & XFS_BLI_HOLD);
 	bool			dirty = !!(bip->bli_flags & XFS_BLI_DIRTY);
+	bool			stale = !!(bip->bli_flags & XFS_BLI_STALE);
 #if defined(DEBUG) || defined(XFS_WARN)
 	bool			ordered = !!(bip->bli_flags & XFS_BLI_ORDERED);
 #endif
+	/*
+	 * The bli dirty state should match whether the blf has logged segments
+	 * except for ordered buffers, where only the bli should be dirty.
+	 */
+	ASSERT((!ordered && dirty == xfs_buf_item_dirty_format(bip)) ||
+	       (ordered && dirty && !xfs_buf_item_dirty_format(bip)));
+	ASSERT(!stale || (bip->__bli_format.blf_flags & XFS_BLF_CANCEL));
 
+	trace_xfs_buf_item_unlock(bip);
 
+	aborted = test_bit(XFS_LI_ABORTED, &lip->li_flags);
 
 	/*
+	 * Clear the buffer's association with this transaction and
+	 * per-transaction state from the bli, which has been copied above.
 	 */
+	bp->b_transp = NULL;
 	bip->bli_flags &= ~(XFS_BLI_LOGGED | XFS_BLI_HOLD | XFS_BLI_ORDERED);
 
 	/*
+	 * Drop the transaction bli reference and free the item if clean or
+	 * aborted and we had the last reference. In either case this is the
+	 * last opportunity to free the item since it won't be written back.
+	 * Otherwise, the bli is still referenced or dirty and will be freed on
+	 * final unpin of the buffer (if stale) or writeback completion.
 	 */
+	freed = atomic_dec_and_test(&bip->bli_refcount);
+	if (freed && (aborted || !dirty)) {
+		ASSERT(!aborted || XFS_FORCED_SHUTDOWN(lip->li_mountp));
+		ASSERT(!stale || aborted);
+		/* an aborted item may be in the AIL, remove it first */
+		if (aborted)
+			xfs_trans_ail_remove(lip, SHUTDOWN_LOG_IO_ERROR);
+		xfs_buf_item_relse(bp);
 	}
 
 	/*
+	 * If the buffer was invalidated, leave it locked on transaction commit
+	 * unless we just dropped the final reference. The latter case should
+	 * only ever happen on abort because invalidation dirties the
+	 * transaction and the log would have grabbed another bli reference when
+	 * the buffer was pinned. In the non-abort case, the buffer is unlocked
+	 * on final unpin and the bli freed since stale buffers are not written
+	 * back.
 	 */
+	if (stale && !freed)
+		return;
+	ASSERT(!stale || (aborted && freed));
 
 	if (!hold)
 		xfs_buf_relse(bp);

I find the logic much more convoluted than what was there before.

I seems like we could apply your stale fix without reshuffling the
code like this:

	if (atomic_dec_and_test(&bip->bli_refcount)) {
		if (aborted) {
			ASSERT(XFS_FORCED_SHUTDOWN(lip->li_mountp));
			xfs_trans_ail_remove(lip, SHUTDOWN_LOG_IO_ERROR);
			xfs_buf_item_relse(bp);
		} else if (!dirty) {
			ASSERT(!stale);
			xfs_buf_item_relse(bp);
		}
	} else {
		if (stale)
			return;
	}
	
	if (!hold)
		xfs_buf_relse(bp);

which at least to me is a lot easier to read.