Re: e2fsprogs: f_eofblocks and f_pgsize_gt_blksize fails on 1.44.3-rc1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Jun 26, 2018, at 10:44 AM, Lukas Czerner <lczerner@xxxxxxxxxx> 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@xxxxxxxxx>
> 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@xxxxxxx>
> 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





Attachment: signature.asc
Description: Message signed with OpenPGP


[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux