Re: [PATCH] xfs: fix s_max_bytes to MAX_LFS_FILESIZE if needed

[Dave Chinner <david@fromorbit.com>]

On Wed, Jul 10, 2013 at 02:28:20PM +0800, Jeff Liu wrote:
> Could anyone help to review this patch?

Sorry, I missed it.

> > On 32-bit machine, the s_maxbytes is larger than the MAX_LFS_FILESIZE limits if CONFIG_LBDAF is
> > not enabled.  Hence it's possible to create a huge file via buffered-IO write with a given offset
> > beyond this limitation. e.g.
> > 
> > # block_size=4096
> > # offset=$(((2**32 - 1) * $block_size))
> > # xfs_io -f -c "pwrite $offset $block_size" /storage/test_file
> > 
> > In this case, xfs_io will hang at the page writeback stage soon since the given offset would
> > cause an overflow at xfs_vm_writepage():
> > 
> > end_index = offset >> PAGE_CACHE_SHIFT;
> > last_index = (offset - 1) >> PAGE_CACHE_SHIFT;
> > if (page->index >= end_index) {
> >                 unsigned offset_into_page = offset & (PAGE_CACHE_SIZE - 1);
> > 
> >                 /*
> >                  * Just skip the page if it is fully outside i_size, e.g. due
> >                  * to a truncate operation that is in progress.
> >                  */
> >                 if (page->index >= end_index + 1 || offset_into_page == 0) {
> > 		^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >                         unlock_page(page);
> >                         return 0;
> >                 }
> > end_index is unsigned long so that the max value is '2^32-1 = 4294967295', and it
> > would be evaluated to the max value with the given offset(when writing the page offset
> > up to s_max_bytes) for above test case.  As a result, (page->index >= end_index + 1) is
> > ok as (end_index + 1) is overflowed to ZERO.
> > 
> > Actually, create a file as above on 32-bit machine should be failed with EFBIG error returned
> > because there has strict check up at generic_write_checks() against the given offset with a
> > *correct* s_max_bytes.
> > 
> > This patch fix the s_max_bytes to MAX_LFS_FILESIZE if the pre-calculated value is greater
> > than it.

Isn't MAX_LFS_FILESIZE defined on 32 bit systems to 8TB and the
problem here is that we are overflowing at 16TB? If so, that means
addin gthis patch will potentially cause problems with existing
working setups that have (sparse) files larger than 8TB on 32 bit
systems.

So, can't we simply subtract PAGE_CACHE_SIZE from the offset
being returned to avoid this overflow?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs