xfs
[Top] [All Lists]

xfs_fsr / xfs_swap_extents() broken in 3.8-rc4?

To: Dave Chinner <david@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
Subject: xfs_fsr / xfs_swap_extents() broken in 3.8-rc4?
From: Torsten Kaiser <just.for.lkml@xxxxxxxxxxxxxx>
Date: Sat, 19 Jan 2013 18:15:40 +0100
Delivered-to: xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:date:message-id:subject:from:to :content-type; bh=g8yT9igdA1CycftqKwl1weFeQD98cF14fbLCHfCKHRs=; b=D8a/7yAIlEVwmr8RXCsMD20U/NdFkxBOj1Ypk+DywLz8NBoYISCtXgIoDUyGxyIcA8 eWSVXgdOQoakJ0uNjjp+nIsBTxh/zJySNrwrvAMz5Gukrm1WaXNlr4ySdDpgqcH2XLA3 NqCzRCLs4TvaAZtpcTP+Hp8BN4M6dy2tu6zlOvk20LwXqylrAjWvYWbMFM0BCaMp4FN7 JJ0yLG8JB372DwmkcupoqdRvg+d+Db1mP9p5c2kkwUsnCO4XEekGVvKkjnnaw7D9eBsr Oq2HuIiqK2wlmX1R1gXVppwQJecdJKBwuzRr+tdS0vrFLQ14re0KZatvwwicT7fHBxEa vWMw==
Hi,

I just used xfs_fsr the first time under a 3.8-rc kernel and was
spammed with an enormous amount of "XFS_IOC_SWAPEXT failed: ino=56118:
Invalid argument" messages.

I found the thread http://marc.info/?t=135727980600001&r=1&w=2 and
went to check the kernel sources for these additional checks, because
I wanted to know what was allowed earlier and is now failing.

I found commit fb59581404ab7ec5075299065c22cb211a9262a9:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=fb59581404ab7ec5075299065c22cb211a9262a9#patch2

--- a/fs/xfs/xfs_dfrag.c
+++ b/fs/xfs/xfs_dfrag.c
@@ -246,12 +246,10 @@ xfs_swap_extents(
                goto out_unlock;
        }

-       if (VN_CACHED(VFS_I(tip)) != 0) {
-               error = xfs_flushinval_pages(tip, 0, -1,
-                               FI_REMAPF_LOCKED);
-               if (error)
-                       goto out_unlock;
-       }
+       error = -filemap_write_and_wait(VFS_I(ip)->i_mapping);
+       if (error)
+               goto out_unlock;
+       truncate_pagecache_range(VFS_I(ip), 0, -1);

That looks really wrong to me. The old code was calling
xfs_flushinval_pages() for *tip*, but the new code is working on *ip*.
Was that change really intended? Is looks unintentional and is not
mentioned in the commit comment...

Thanks for looking / correcting me if I'm missing something obvious,
but I can't find a reason for this change...

Torsten

<Prev in Thread] Current Thread [Next in Thread>