On 6/18/20 7:04 PM, Dave Chinner wrote:
On Fri, Jun 19, 2020 at 08:58:10AM +1000, Dave Chinner wrote:
On Thu, Jun 18, 2020 at 01:19:41PM -0400, Waiman Long wrote:
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 379cbff438bc..1b94b9bfa4d7 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -913,11 +913,33 @@ xfs_fs_freeze(
struct super_block *sb)
{
struct xfs_mount *mp = XFS_M(sb);
+ unsigned long pflags;
+ int ret;
+ /*
+ * A fs_reclaim pseudo lock is added to check for potential deadlock
+ * condition with fs reclaim. The following lockdep splat was hit
+ * occasionally. This is actually a false positive as the allocation
+ * is being done only after the frozen filesystem is no longer dirty.
+ * One way to avoid this splat is to add GFP_NOFS to the affected
+ * allocation calls. This is what PF_MEMALLOC_NOFS is for.
+ *
+ * CPU0 CPU1
+ * ---- ----
+ * lock(sb_internal);
+ * lock(fs_reclaim);
+ * lock(sb_internal);
+ * lock(fs_reclaim);
+ *
+ * *** DEADLOCK ***
+ */
The lockdep splat is detailed in the commit message - it most
definitely does not need to be repeated in full here because:
a) it doesn't explain why the splat occurring is, and
b) we most definitely don't care about how the lockdep check
that triggered it is implemented.
I should have added this:
c) a lot of people don't understand what lockdep reports
are telling them is a problem.
I get a lot of questions like "I saw this lockdep thing, but I can't
work out what it actually means, so can you have a look at it
Dave?". Hence I think directly quoting something people tend not to
understand to explain the problem they didn't understand isn't the
best approach to improving understanding of the problem...
OK, how about simplifying the comment to as follows:
/*
* Disable fs reclaim in memory allocation for fs freeze to avoid
* causing a possible circular locking dependency lockdep splat
* involving fs reclaim.
*/
Does that look good enough for you?
Cheers,
Longman
