From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753277AbdHXPXL (ORCPT ); Thu, 24 Aug 2017 11:23:11 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:40012 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752748AbdHXPXK (ORCPT ); Thu, 24 Aug 2017 11:23:10 -0400 Subject: Re: Kernels v4.9+ cause short reads of block devices To: Doug Nazar , Andreas Dilger , Linus Torvalds Cc: Al Viro , Linux Kernel Mailing List , Wei Fang , linux-fsdevel , Mark Fasheh , Joel Becker , Dave Kleikamp References: <1ae53e17-e455-4f17-0280-b0dae183a449@nazar.ca> <75bc3c58-c699-f7f2-4fc2-a8e2aefeddb5@nazar.ca> From: Dave Kleikamp Message-ID: <6f456cb7-53b2-7ae1-07fa-8c33332b9c66@oracle.com> Date: Thu, 24 Aug 2017 10:22:11 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <75bc3c58-c699-f7f2-4fc2-a8e2aefeddb5@nazar.ca> Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/24/2017 05:20 AM, Doug Nazar wrote: > On 8/23/17 5:01 PM, Andreas Dilger wrote: >> Doug, >> I noticed while checking for other implications of changing >> MAX_LFS_FILESIZE >> that fs/jfs/super.c is also working around this limit.  If you are going >> to submit a patch for this, it also makes sense to fix >> jfs_fill_super() to >> use MAX_LFS_FILESIZE instead of JFS rolling its own, something like: >> >>     /* logical blocks are represented by 40 bits in pxd_t, etc. >>      * and page cache is indexed by long. */ >>     sb->s_maxbytes = min((u64)sb->s_blocksize) << 40, >>                               MAX_LFS_FILESIZE); >> >> It also looks like ocfs2_max_file_offset() is trying to avoid overflowing >> the old 31-bit limit, and isn't using MAX_LFS_FILESIZE directly, so it >> will >> now be wrong.  It looks like it could use "bitshift = 32; trim = bytes;", >> but Joel or Mark should confirm. >> >> Finally, there is a check in fs/super.c::mount_fs() that is verifying >> s_maxbytes is not set too large, but this has been present since 2.6.32 >> and should probably be removed at this point, or changed to a BUG_ON() >> (see commit 42cb56ae2ab for details). > > I don't have any issue trying to write patches for those, but I have no > domain knowledge > in the area or any way to test them. If you want to wrap the jfs change into this, I will be happy to test it for you, or I could take care of jfs with a separate patch if you'd prefer. > > From a quick glance, jfs is locked to PSIZE (4096) so should be ok. > OCFS looks a little complex, and since it's a shared fs, little hesitant. > > The check in fs/super.c, maybe that should be: > > sb->s_maxbytes > MAX_LFS_FILESIZE > > Actually, little confused, the comment says unsigned, but loff_t looks > like its long long. > Maybe cast to u64 and check greater than? > > Doug > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from aserp1040.oracle.com ([141.146.126.69]:40012 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752748AbdHXPXK (ORCPT ); Thu, 24 Aug 2017 11:23:10 -0400 Subject: Re: Kernels v4.9+ cause short reads of block devices To: Doug Nazar , Andreas Dilger , Linus Torvalds Cc: Al Viro , Linux Kernel Mailing List , Wei Fang , linux-fsdevel , Mark Fasheh , Joel Becker , Dave Kleikamp References: <1ae53e17-e455-4f17-0280-b0dae183a449@nazar.ca> <75bc3c58-c699-f7f2-4fc2-a8e2aefeddb5@nazar.ca> From: Dave Kleikamp Message-ID: <6f456cb7-53b2-7ae1-07fa-8c33332b9c66@oracle.com> Date: Thu, 24 Aug 2017 10:22:11 -0500 MIME-Version: 1.0 In-Reply-To: <75bc3c58-c699-f7f2-4fc2-a8e2aefeddb5@nazar.ca> Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 08/24/2017 05:20 AM, Doug Nazar wrote: > On 8/23/17 5:01 PM, Andreas Dilger wrote: >> Doug, >> I noticed while checking for other implications of changing >> MAX_LFS_FILESIZE >> that fs/jfs/super.c is also working around this limit.� If you are going >> to submit a patch for this, it also makes sense to fix >> jfs_fill_super() to >> use MAX_LFS_FILESIZE instead of JFS rolling its own, something like: >> >> ����/* logical blocks are represented by 40 bits in pxd_t, etc. >> ���� * and page cache is indexed by long. */ >> ����sb->s_maxbytes = min((u64)sb->s_blocksize) << 40, >> ����������������������������� MAX_LFS_FILESIZE); >> >> It also looks like ocfs2_max_file_offset() is trying to avoid overflowing >> the old 31-bit limit, and isn't using MAX_LFS_FILESIZE directly, so it >> will >> now be wrong.� It looks like it could use "bitshift = 32; trim = bytes;", >> but Joel or Mark should confirm. >> >> Finally, there is a check in fs/super.c::mount_fs() that is verifying >> s_maxbytes is not set too large, but this has been present since 2.6.32 >> and should probably be removed at this point, or changed to a BUG_ON() >> (see commit 42cb56ae2ab for details). > > I don't have any issue trying to write patches for those, but I have no > domain knowledge > in the area or any way to test them. If you want to wrap the jfs change into this, I will be happy to test it for you, or I could take care of jfs with a separate patch if you'd prefer. > > From a quick glance, jfs is locked to PSIZE (4096) so should be ok. > OCFS looks a little complex, and since it's a shared fs, little hesitant. > > The check in fs/super.c, maybe that should be: > > sb->s_maxbytes > MAX_LFS_FILESIZE > > Actually, little confused, the comment says unsigned, but loff_t looks > like its long long. > Maybe cast to u64 and check greater than? > > Doug >