linux-ext4 - Re: e2fsprogs: f_eofblocks and f_pgsize_gt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <1C455884-3FC3-4B47-AC7B-9C2B908F00EB@dilger.ca>
Date:   Thu, 28 Jun 2018 12:36:16 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     Lukas Czerner <lczerner@...hat.com>
Cc:     Ext4 Developers List <linux-ext4@...r.kernel.org>
Subject: Re: e2fsprogs: f_eofblocks and f_pgsize_gt_blksize fails on
 1.44.3-rc1

On Jun 26, 2018, at 10:44 AM, Lukas Czerner <lczerner@...hat.com> wrote:
> 
> Hi all,
> 
> I've noticed that f_eofblocks fails on systems with page size > 4k
> (like ppc46) due to this commit.
> 
> commit 8d541f641ef3861276f7138b2f3e601036f03110
> Author: Andreas Dilger <andreas.dilger@...el.com>
> Date:   Sat Jun 23 14:30:52 2018 -0400
> 
>    e2fsck: handle preallocation for large PAGE_SIZE
> 
>    Fix handling of block preallocation support in cases where the kernel
>    PAGE_SIZE is larger than the filesystem blocksize.
> 
> Test f_pgsize_gt_blksize was added right after this commit and it also fails
> on ppc64

Sorry for the delay in reply - I was travelling for most of the week.

This patch is fixing the handling for files with allocations larger than
i_size being rounded up to a full PAGE_SIZE.  In hindsight, the test will
only work for PAGE_SIZE=4096, so it should probably be skipped if PAGE_SIZE
is different, or it could be changed to always hard-code 4096 in the test.

The reason for this patch is that the Lustre servers will always allocate a
full PAGE_SIZE of blocks to avoid complexity in the IO path, since the data
is written via RDMA into the buffers, so the whole page is written even if
it doesn't contain data.

I'll submit a patch to fix the test so that it works on PPC and others with
large PAGE_SIZE.

Cheers, Andreas

> I belive that the reason is this
> 
> 		if ((pb.last_init_lblock >= 0) &&
> 		    /* if size is smaller than expected by the block count,
> 		     * allow allocated blocks to end of PAGE_SIZE.
> 		     * last_init_lblock is the last in-use block, so it is
> 		     * the minimum expected file size, but +1 because it is
> 		     * the base-zero block number and not the block count. */
> 		    (size < (__u64)pb.last_init_lblock * fs->blocksize) &&
>>>>> 		    ((pb.last_init_lblock + 1) / blkpg * blkpg !=
>>>>> 		     (pb.last_init_lblock + 1) ||
> 		     size < (__u64)(pb.last_init_lblock & ~(blkpg - 1)) *
> 		     fs->blocksize))
> 			bad_size = 3;
> 
> while the fix seems correct in context I do not understand the point
> of the check.  Seems like the reason for it is to make sure that if we do
> have any initialized blocks after i_size, it's always up the the page size
> boundary.
> 
> This was added with commit
> 
> commit 9f0288d3bbbf47bb05e5abf1e570df368476a8cd
> Author: Theodore Ts'o <tytso@....edu>
> Date:   Fri Aug 3 20:43:37 2007 -0400
> 
>    e2fsck: Allow i_size to be rounded up to the size of a VM page
> 
>    Allow files to be preallocated on-disk up to the next multiple of the
>    system's page size without complaining about extra blocks.
> 
> 
> It's not explained why we need this. Do we ever want to allow initialized
> extents (blocks) past i_size ? I though we do not, what am I missing ?
> Shouldn't we only be doing this ?
> 
> 	if ((pb.last_block >= 0) &&
> 	    (size < (__u64) pb.last_block * fs->blocksize))
> 		bad_size = 3;
> 
> 
> Thanks!
> -Lukas
> 


Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)