[Top] [All Lists]

Re: [PATCH v4 2/10] xfs: Add support FALLOC_FL_INSERT_RANGE for fallocat

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH v4 2/10] xfs: Add support FALLOC_FL_INSERT_RANGE for fallocate
From: Mark Tinguely <tinguely@xxxxxxx>
Date: Fri, 30 May 2014 14:10:32 -0500
Cc: Namjae Jeon <namjae.jeon@xxxxxxxxxxx>, linux-fsdevel@xxxxxxxxxxxxxxx, Ashish Sangwan <a.sangwan@xxxxxxxxxxx>, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140530140551.GC6226@xxxxxxxxxxxxxxx>
References: <001e01cf7b0f$ba43dfd0$2ecb9f70$@samsung.com> <20140530140551.GC6226@xxxxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:9.0) Gecko/20120122 Thunderbird/9.0
On 05/30/14 09:05, Brian Foster wrote:
On Thu, May 29, 2014 at 04:29:29PM +0900, Namjae Jeon wrote:
>  This patch implements fallocate's FALLOC_FL_INSERT_RANGE for XFS.
>  1) Make sure that both offset and len are block size aligned.
>  2) Update the i_size of inode by len bytes.
>  3) Compute the file's logical block number against offset. If the computed
>      block number is not the starting block of the extent, split the extent
>      such that the block number is the starting block of the extent.
>  4) Shift all the extents which are lying bewteen [offset, last allocated 
>      towards right by len bytes. This step will make a hole of len bytes
>      at offset.
>  5) Allocate unwritten extents for the hole created in step 4.
>  Signed-off-by: Namjae Jeon<namjae.jeon@xxxxxxxxxxx>
>  Signed-off-by: Ashish Sangwan<a.sangwan@xxxxxxxxxxx>
>  Reviewed-by: Brian Foster<bfoster@xxxxxxxxxx>
>  ---
>  Changelog
>  v4:
>    - set cur->bc_private.b.allocated to zero before calling 
Other issues not withstanding (sounds like Mark still has a lingering,
potential corruption case), this fixes the warnings I was seeing. I ran
through an xfstest cycle without problem. Thanks for fixing this.

The cursor allocated flag appears to be used to carry bmbt block
allocations over to delayed allocation extent conversions (such that
these allocations are accounted from previous delalloc extent index
block reservations). This is all within the bmbt code, so clearing the
flag seems reasonable as well.

Though I wonder why it falls on the caller to clear the flag as opposed
to fixing up the flag automatically when it becomes accounted for. I
guess that would enable a warning with semantics of "something might
have been unaccounted for" vs. "somebody forgot to reset a flag." That
said, it's likely there are circumstances involved with this mechanism
that I'm not yet familiar with.;)


I did not have this patch included in my tests just the code from Linux 3.15-rc5. I will add and see if it still asserts.


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