Re: [PATCH 23/24] iomap: add support for sub-pagesize buffered I/O without buffer heads

[Brian Foster <bfoster@redhat.com>]

On Mon, Jul 02, 2018 at 02:50:36PM +0200, Christoph Hellwig wrote:
> The problem is that we need to split extents at the eof block
> so that the existing zeroing actually takes effect.  The patch below
> fixes the test case for me:
> 

Looks sane at a glance. I'll take a closer look at v7 and run some more
testing..

Brian

> diff --git a/fs/iomap.c b/fs/iomap.c
> index e8f1bcdc95cf..9c88b8736de0 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -143,13 +143,20 @@ static void
>  iomap_adjust_read_range(struct inode *inode, struct iomap_page *iop,
>  		loff_t *pos, loff_t length, unsigned *offp, unsigned *lenp)
>  {
> +	unsigned block_bits = inode->i_blkbits;
> +	unsigned block_size = (1 << block_bits);
>  	unsigned poff = *pos & (PAGE_SIZE - 1);
>  	unsigned plen = min_t(loff_t, PAGE_SIZE - poff, length);
> +	unsigned first = poff >> block_bits;
> +	unsigned last = (poff + plen - 1) >> block_bits;
> +	unsigned end = (i_size_read(inode) & (PAGE_SIZE - 1)) >> block_bits;
>  
> +	/*
> +	 * If the block size is smaller than the page size we need to check the
> +	 * per-block uptodate status and adjust the offset and length if needed
> +	 * to avoid reading in already uptodate ranges.
> +	 */
>  	if (iop) {
> -		unsigned block_size = i_blocksize(inode);
> -		unsigned first = poff >> inode->i_blkbits;
> -		unsigned last = (poff + plen - 1) >> inode->i_blkbits;
>  		unsigned int i;
>  
>  		/* move forward for each leading block marked uptodate */
> @@ -159,17 +166,27 @@ iomap_adjust_read_range(struct inode *inode, struct iomap_page *iop,
>  			*pos += block_size;
>  			poff += block_size;
>  			plen -= block_size;
> +			first++;
>  		}
>  
>  		/* truncate len if we find any trailing uptodate block(s) */
>  		for ( ; i <= last; i++) {
>  			if (test_bit(i, iop->uptodate)) {
>  				plen -= (last - i + 1) * block_size;
> +				last = i - 1;
>  				break;
>  			}
>  		}
>  	}
>  
> +	/*
> +	 * If the extent spans the block that contains the i_size we need to
> +	 * handle both halves separately so that we properly zero data in the
> +	 * page cache for blocks that are entirely outside of i_size.
> +	 */
> +	if (first <= end && last > end)
> +		plen -= (last - end) * block_size;
> +
>  	*offp = poff;
>  	*lenp = plen;
>  }
> --
> 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