From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933244AbdCUTNt (ORCPT ); Tue, 21 Mar 2017 15:13:49 -0400 Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:59315 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757551AbdCUTNl (ORCPT ); Tue, 21 Mar 2017 15:13:41 -0400 From: Calvin Owens Subject: Re: [PATCH v2] xfs: Honor FALLOC_FL_KEEP_SIZE when punching ends of files To: Brian Foster References: <22a11e65fd5037498a78de61f3ed4dae466ad854.1489791330.git.calvinowens@fb.com> <19504ff40a16efff2e51d85388fce5be578edbc3.1489985397.git.calvinowens@fb.com> <20170321113932.GA58653@bfoster.bfoster> CC: "Darrick J. Wong" , , , Christoph Hellwig , Dave Chinner , , , calvinowens Message-ID: Date: Tue, 21 Mar 2017 12:13:21 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170321113932.GA58653@bfoster.bfoster> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [2620:10d:c090:200::a:145f] X-ClientProxiedBy: DM5PR1101CA0017.namprd11.prod.outlook.com (10.174.246.27) To MWHPR15MB1232.namprd15.prod.outlook.com (10.175.2.150) X-MS-Office365-Filtering-Correlation-Id: b144f694-1f6b-46de-bb84-08d4708e5c9d X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:MWHPR15MB1232; X-Microsoft-Exchange-Diagnostics: 1;MWHPR15MB1232;3:jA6QXSzRLd6EmPXLxIJgY31q7VewdjZKQ07vW7KpDX98iLAHE84FrDvDNOwmYDQbZvLMDaLs1Pmgy5VCaSDpaw7NxQySW88o14NtOSlZgnGe5QjuT/32LyWgNAL0cyuVEbFqhDgxRYPwJ6ZL0EbXXVeNz7ShGsmrRaS9DrJgGfR3JQjpoqadQSni0nYiylFKZ6JpU9RMVsWLM0tXygo2VOstOCOEXfA7ByMIIs9SBNCEs1yuraJAOxcHN1pM7iDCYufM6ux4U6YlSXMJ3s/3mQ==;25:QHsVcdFk8W8j0/Yn7j5LpjcfeRPXqGBubRNPpr2Kh7tIsv7g3nctCwZrWF+oaFxZwrZvhMFiuJZsbjwjMId3nYoNOSW4oTLWwsPtRakG1gGNkocRCi1Y+5Kj+AhMht8kquCoZTYnNr9mJSxhBco7P6gDpVG6I+g/aiseZWkG/QQmQeVGbFa/Ojfg/YOVo4icrUMhUCMJfZENXjlDdDdoUzak32kVtfEKI0/gjtaHCSUkgm8qr2nUw3A8zTiF+MDcx3Syz/A1+/kg3y1trC8cGp6vNbjGIK1t/4uOQlJoNVC/6ea/i+avl1SvzY/le9XkJBXbKDraVIP2eTAVu53BZrb7ULsRUMuIjcuEjLf901WhynhztNqWWGY/t5NHu1Mqet+oC7W2hidjBcijvW8tzP45IEKZZBP4bU7eDzs12H3NoaAH4U4d4fkGNZvP9yDFqh5YvwF16qiZ08Tx1aUt5w== X-Microsoft-Exchange-Diagnostics: 1;MWHPR15MB1232;31:PHNbM1XK9P5ND0GYp3WXt37Yw99pScvS2yBXRPbUzH7/UW1/EEe+Ohu4urZbWHtd/M7ndvGCEtQLgYjxdUVqveXeKNrkRNv83aKYqKMaSUyKgTMAOHg4TAvMp/JL9RQzI8H/qixpIxewrlrK+oXpSveHgNEhAwLqwJwhH1ytK8lPp1Z6e587KdVq/N3fkVL7JyQ7FCa4A9PqJjTj62vFaDZzHwKecDZazg8aKvEQ1yAKaNjOxbZT9l0YkUQOhSgc;20:rAJG6cfE8rx3OFXjGWh2CcvjT5p5hkfff9PJbsIDRywTaOQH3Q3RXMfoWeri8ghK9c5Xe8FQ4TNg2RUMFQusd8wFl1B6ZrLdZorV37qvRnZzByNxdeJMNn/1mQ0y+FnEt8M9vQLOjN+61ZnMFxJXAXJrREXiP5r/5yWTapxS/RVMa8i1GsoLU9hmdZa1s867VwOo1V++KD08GyLRsBja8EReLeAPm/OCJ6j2qlblsVebdQoshCYR8Z8Xkw/Sy8ml2ciOsDv7DssKCXYPMi0VDcMhdwsXfjBZkq/nzjc1eRmiqqp1mb5AD7B23uiYy0ehlBoSV0Ybe6qoZ4D2et5Jm4CP4oauEqnkj9VpYxsE/1yckyAhNUH1CtV+AhZ+EQCXfN51DbQ7oleRtRbhgONO/AkNrGHPLTzbJ4xOuc/4Vbsnvl1e1DlPlVhUuKr6zRaNLzbOVGYEQ0VPdkLbnt8Cnq2r+V67ti+7HkIZbZ0EpqKY3vpBMzAg+rV1q83AkLAb X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(9452136761055)(67672495146484); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040375)(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001)(6041248)(20161123562025)(20161123560025)(20161123564025)(20161123558025)(20161123555025)(6072148);SRVR:MWHPR15MB1232;BCL:0;PCL:0;RULEID:;SRVR:MWHPR15MB1232; X-Microsoft-Exchange-Diagnostics: 1;MWHPR15MB1232;4:sGaGfPukDMhITr/m+JsEL0b6AyzxeFd4uBpfMl6CfSaJ9vJ1Mx7/ETP+VWGa2iDlc31Hanw8N2m3Rj9GhVghK3a81yN5rzZa+sOKJBqwJU9zrzRTTHoqD2wWTbbdmulrfSv81cgXP+2/HSaL+Yel0zM3p94o7XwgNlpJc98SFMHNC/jbuyoMYXTHADvagQMlqMFbqrotoNy0j0lFOaEBh22lePcv1ZKEVKvbtyOr7FrOeziaSVxtaieupJJn/f+3IW8Nh7hXxNBCMWzYPkPiQOFa/if5fG0sga54EEHhzeIO4J8KP0CAtHh+UkWPgieLjLFI44mKvKO/SAhfR5kVN+xfHXpst+To0HM+psTRlDHxoi049G1HpKC78fSNggyw4L95j9k5UmMz9+/1p69VBNXzfR1szNd+VYAp+MUpdmqVcavVe5vW/iGTY1uOipwWwY+3KXUVJ51r1qqLmX0NGnvf4vcIIMi0Zuq1AIPZzLETPeb4o6HCbydyL47FNLXMDWttWizKpsJ9VCPQGgoM+HXaVhoy5Lx1i0yseEVKabVbDB6QU5Oe6L44vza6KmLVW3v1hZVPaROB8eSjOiIy/B9KMORXE4ekr0YkH3LvHFt8DvThKAvSnNqYtLwPiiXNxeCcOH8hWyjYZ5cCxuvO+jqaGvD0OmMgxhSCXginm/1B9HCOBoIfXF+n5gHMOn7n X-Forefront-PRVS: 02530BD3AA X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6009001)(39450400003)(24454002)(377454003)(229853002)(50986999)(38730400002)(2950100002)(31686004)(31696002)(966004)(8676002)(7736002)(189998001)(86362001)(4001350100001)(36756003)(6486002)(81166006)(6116002)(33646002)(83506001)(2906002)(110136004)(6666003)(64126003)(6916009)(53936002)(23746002)(54356999)(76176999)(65956001)(42186005)(6306002)(230700001)(305945005)(50466002)(4326008)(54906002)(53546009)(5660300001)(47776003)(25786009)(422495003);DIR:OUT;SFP:1102;SCL:1;SRVR:MWHPR15MB1232;H:[IPv6:2620:10d:c082:10e2:c23f:d5ff:fe6b:54f7];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;MWHPR15MB1232;23:HJRgiEngeaUIFuuyFSw5CxS0zaV6ZpVUUNhw7?= =?Windows-1252?Q?hw87oDriMr/G6ggfOxbp2G65iFW1D4surTXCHxKBTqC+XD5lP4kK5F6h?= =?Windows-1252?Q?7F0pZXBxW63VuG/IvAh0fJZnFeWPw7I8y9rCIpJIo0CvInuA/y3CzY/I?= =?Windows-1252?Q?zJ2stu6AXeT4yR4naHK9fMND752giMUhJyIJZiU/dvDwLl7UWa0Vl+cG?= =?Windows-1252?Q?n332Em3k95Ccqa0jiGDFK7If8Y4gcgYHKC0TgaN1eeLBadCEgz/y49+k?= =?Windows-1252?Q?C5g0KP/J7ZtWWYlucEiy6rUrUmeZ5tsuvJjwW6WNpk2HeH+uacL4/7cj?= =?Windows-1252?Q?FP8KXkQ4R1KEvnQq8rAdPXElmmvMTHKNtg9kCEp4dem/ILdF0l7I/2cv?= =?Windows-1252?Q?CEVu+me/fW114NDCe5Lh+yZ+/kTvZ9BTN8+VR9hHVKHKw+s09Z85uWGo?= =?Windows-1252?Q?lejZ0KNR6nx/UAceJq6dp7y9exP1yOqxoNwewhzesWVHdMugPuMHMOQY?= =?Windows-1252?Q?59SBKv90IQn6LHh0m7bxS0C0rryqszbW0+M9qMVVqDGogHzX51NULZ7R?= =?Windows-1252?Q?wzday3QjaLQe0tZISzPuFn1/TzxP/TeBgUQGR0xg0SU3g2H1/UySbBrN?= =?Windows-1252?Q?GXeZN3khoyLb2E4Vy5xR95583CgsOwQkyWFYDc1IdSRqKeewMbVe87XS?= =?Windows-1252?Q?zmOJXxlGBWuSfXpnA0u0DYDHqrCIcehJi+TZGYFTSirDakO8SWUh9aDe?= =?Windows-1252?Q?n6JYHNMRYbRyYbg1ociBpitziobVDP1TwIiHo1+n9WHKW/G38OF3hYS2?= =?Windows-1252?Q?7+WvU+MOdzCWFiqwz5YCyoZ+nP2IJ9bqyEsDxfhefp9iWvt7rI8oi+a0?= =?Windows-1252?Q?GLGU978EI9e8cLVTrn/IQtIQ49SqFcgPwS4zJgu9PRMPFh1/8Uo7kXab?= =?Windows-1252?Q?Cldi9wn28hBNJOVRp1FqQP+xSK1o62QZxhNAQusYvwVlM6/pr2zW4zv/?= =?Windows-1252?Q?naU+qJROnPxWrVy8TsdLwRA5qhTkRtlxk8G4jQmhrEWmGd3j1urxSXrs?= =?Windows-1252?Q?8DpWrDkfP54DFAe+G8RCJtmf4w3oT+G/EdJgJ0CssOezfrHM+1UtD8dj?= =?Windows-1252?Q?SZn/IUmeVH4C7baaRrGHUcXurqRQ99MuMETbwW+oIwBkSjdz67kLYt88?= =?Windows-1252?Q?1V1T7jp0oG3cMx+XOkBI3U4vRFF2XE=3D?= X-Microsoft-Exchange-Diagnostics: 1;MWHPR15MB1232;6:rt2WmLEdbYl4iQz4RYY8t0XJhdAT/JRbbEheaepx6oAel7AGv3eDsSCPTrSnS/B5PtwMs4V9yxXXmdi4jYR2vK0vqNRbGvx0Bz6jUp8OSH77jnfoMe4/I7wo9N66KttJBLumKLCJgjfM5E7WPgyks1dT59BcPBRo2JgIvLAJirHO8HGY9lwHxTeYv7LKOoF83XTHNN5dCViaQv72n/7MJP44dAkPOBvnJXRdxNYn6ylEPs46W27BA8X1WGj72BIvVYOoQIDtOyMl0K6EtykSyELPUuLzLa6Ss2CoGpzAh2dvwnga8YmTswgGEnu6D0TFU58Yx3D9QruwKfwACPi557lLGOpdkDDULBi29xPownX2GlYYk2yRfFg674y69/3BZ2qfdLXwyW1UFBnig+Kn2Q==;5:piY4UrwgopvKecCClWkbzcpPi9kKGBTiQoCfjkj4MU+G93QtF0K7Bqwk2v9OSX7FtLNTnpvxIxNddLAxEDzxMEdR4ByQuP0tTOwr4fcpBXJP0ClGt/83y9KJxcLPQ0Srcc4cUyDTYOTpcNvU5zCeuw==;24:vEH1qTVD0XNR00IvEv5eD/JzcmfRpchQ9rvrD267ryBuqKZO8asSia8U+n6+gy2pzab+QIdI8CXlhx59JZLDx16fhNkL5nTNXI/VmHTyca4= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;MWHPR15MB1232;7:xB2A2JdXl1+JhcuBfiachnxvK4xgWljxsYpYzGez4zNeD1FXfkHXCyTd4u3gdj/TyJQPPepTCd6u4cghCeo8WWt4aHQYkRzZwJPFHE6KzEKmfV+WRiV1HZM6BtVYlWU0N0kDqOhAipOVuDNjRc9hrLWzJcdu6JGMV3HsYBh4kflC9DmgBc0mv4Mav1KDwGfhdGrH5N4YWdo4BLMj4ak2eR0KrSIVBdmRuX8CZsKzFbatBxY729uVbPLjj+tJGVqhrO7DaVad17x6746w5uC0AMGJYc7bj5M7JJm78+NciMFsnVwZQBagO+wUmWCipJhrmilbHGuZiOC2eEooI47zmw==;20:ZphaKxIbErXKW+maP5Z+D9mzwpPYBpI1h5ANHG+cThTkUdWStksp/M4WLwANRsNyCy1MTS7bletgo3XEZcgI9ssqSTrJQGs19ssXbNLzbYumSfuOzvMlWSSHkjkserN+p6VXr28bNj2p6ONVl99hs2D3FI/fAWm+47UzQhFHzIo= X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Mar 2017 19:13:29.4219 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR15MB1232 X-OriginatorOrg: fb.com X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-03-21_16:,, signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/21/2017 04:39 AM, Brian Foster wrote: > On Sun, Mar 19, 2017 at 09:54:51PM -0700, Calvin Owens wrote: >> When punching past EOF on XFS, fallocate(mode=PUNCH_HOLE|KEEP_SIZE) will >> round the file size up to the nearest multiple of PAGE_SIZE: >> >> calvinow@vm-disks/generic-xfs-1 ~$ dd if=/dev/urandom of=test bs=2048 count=1 >> calvinow@vm-disks/generic-xfs-1 ~$ stat test >> Size: 2048 Blocks: 8 IO Block: 4096 regular file >> calvinow@vm-disks/generic-xfs-1 ~$ fallocate -n -l 2048 -o 2048 -p test >> calvinow@vm-disks/generic-xfs-1 ~$ stat test >> Size: 4096 Blocks: 8 IO Block: 4096 regular file >> >> Commit 3c2bdc912a1cc050 ("xfs: kill xfs_zero_remaining_bytes") replaced >> xfs_zero_remaining_bytes() with calls to iomap helpers. The new helpers >> don't enforce that [pos,offset) lies strictly on [0,i_size) when being >> called from xfs_free_file_space(), so by "leaking" these ranges into >> xfs_zero_range() we get this buggy behavior. >> >> Fix this by reintroducing the checks xfs_zero_remaining_bytes() did >> against i_size at the bottom of xfs_free_file_space(). >> >> Reported-by: Aaron Gao >> Fixes: 3c2bdc912a1cc050 ("xfs: kill xfs_zero_remaining_bytes") >> Cc: Christoph Hellwig >> Cc: # 4.8+ >> Signed-off-by: Calvin Owens >> --- >> fs/xfs/xfs_bmap_util.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c >> index 8b75dce..0796ebc 100644 >> --- a/fs/xfs/xfs_bmap_util.c >> +++ b/fs/xfs/xfs_bmap_util.c >> @@ -1309,6 +1309,17 @@ xfs_free_file_space( >> } >> >> /* >> + * Avoid doing I/O beyond eof - it's not necessary >> + * since nothing can read beyond eof. The space will >> + * be zeroed when the file is extended anyway. >> + */ > > I'd suggest to update the comment below with this information and move > the following bits down below it as well. Will do. >> + if (offset >= XFS_ISIZE(ip)) >> + return 0; >> + >> + if ((offset + len) >= XFS_ISIZE(ip)) >> + len = XFS_ISIZE(ip) - offset - 1; >> + > > This looks like an off-by-one. Do you mean the following? > > if (offset + len > XFS_ISIZE(ip)) > len = XFS_ISIZE(ip) - offset; It's not an off-by-one (it's self-consistent), but your way makes more sense, I'll fix it ;) Thanks, Calvin > Brian > >> + /* >> * Now that we've unmap all full blocks we'll have to zero out any >> * partial block at the beginning and/or end. xfs_zero_range is >> * smart enough to skip any holes, including those we just created. >> -- >> 2.9.3 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:59315 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757551AbdCUTNl (ORCPT ); Tue, 21 Mar 2017 15:13:41 -0400 From: Calvin Owens Subject: Re: [PATCH v2] xfs: Honor FALLOC_FL_KEEP_SIZE when punching ends of files References: <22a11e65fd5037498a78de61f3ed4dae466ad854.1489791330.git.calvinowens@fb.com> <19504ff40a16efff2e51d85388fce5be578edbc3.1489985397.git.calvinowens@fb.com> <20170321113932.GA58653@bfoster.bfoster> Message-ID: Date: Tue, 21 Mar 2017 12:13:21 -0700 MIME-Version: 1.0 In-Reply-To: <20170321113932.GA58653@bfoster.bfoster> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-xfs-owner@vger.kernel.org List-ID: List-Id: xfs To: Brian Foster Cc: "Darrick J. Wong" , linux-xfs@vger.kernel.org, linux-kernel@vger.kernel.org, Christoph Hellwig , Dave Chinner , kernel-team@fb.com, stable@vger.kernel.org, calvinowens On 03/21/2017 04:39 AM, Brian Foster wrote: > On Sun, Mar 19, 2017 at 09:54:51PM -0700, Calvin Owens wrote: >> When punching past EOF on XFS, fallocate(mode=PUNCH_HOLE|KEEP_SIZE) will >> round the file size up to the nearest multiple of PAGE_SIZE: >> >> calvinow@vm-disks/generic-xfs-1 ~$ dd if=/dev/urandom of=test bs=2048 count=1 >> calvinow@vm-disks/generic-xfs-1 ~$ stat test >> Size: 2048 Blocks: 8 IO Block: 4096 regular file >> calvinow@vm-disks/generic-xfs-1 ~$ fallocate -n -l 2048 -o 2048 -p test >> calvinow@vm-disks/generic-xfs-1 ~$ stat test >> Size: 4096 Blocks: 8 IO Block: 4096 regular file >> >> Commit 3c2bdc912a1cc050 ("xfs: kill xfs_zero_remaining_bytes") replaced >> xfs_zero_remaining_bytes() with calls to iomap helpers. The new helpers >> don't enforce that [pos,offset) lies strictly on [0,i_size) when being >> called from xfs_free_file_space(), so by "leaking" these ranges into >> xfs_zero_range() we get this buggy behavior. >> >> Fix this by reintroducing the checks xfs_zero_remaining_bytes() did >> against i_size at the bottom of xfs_free_file_space(). >> >> Reported-by: Aaron Gao >> Fixes: 3c2bdc912a1cc050 ("xfs: kill xfs_zero_remaining_bytes") >> Cc: Christoph Hellwig >> Cc: # 4.8+ >> Signed-off-by: Calvin Owens >> --- >> fs/xfs/xfs_bmap_util.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c >> index 8b75dce..0796ebc 100644 >> --- a/fs/xfs/xfs_bmap_util.c >> +++ b/fs/xfs/xfs_bmap_util.c >> @@ -1309,6 +1309,17 @@ xfs_free_file_space( >> } >> >> /* >> + * Avoid doing I/O beyond eof - it's not necessary >> + * since nothing can read beyond eof. The space will >> + * be zeroed when the file is extended anyway. >> + */ > > I'd suggest to update the comment below with this information and move > the following bits down below it as well. Will do. >> + if (offset >= XFS_ISIZE(ip)) >> + return 0; >> + >> + if ((offset + len) >= XFS_ISIZE(ip)) >> + len = XFS_ISIZE(ip) - offset - 1; >> + > > This looks like an off-by-one. Do you mean the following? > > if (offset + len > XFS_ISIZE(ip)) > len = XFS_ISIZE(ip) - offset; It's not an off-by-one (it's self-consistent), but your way makes more sense, I'll fix it ;) Thanks, Calvin > Brian > >> + /* >> * Now that we've unmap all full blocks we'll have to zero out any >> * partial block at the beginning and/or end. xfs_zero_range is >> * smart enough to skip any holes, including those we just created. >> -- >> 2.9.3 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html