Hi Dave,
Thanks for the quick response.
On 07/10/2013 02:48 PM, Dave Chinner wrote:
> 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.
Yes, but maybe I should say end_index is wrapped to zero rather than
overflow in this situation.
>
> So, can't we simply subtract PAGE_CACHE_SIZE from the offset
> being returned to avoid this overflow?
It seems that this change does not works to me because page->index is
greater than end_index in most cases for large files.
IOWs, what I mentioned in comment log is incorrect and it misled you.
I worked out another patch, it looks works for this test but I need to
run some extra tests before posting.
Thanks,
-Jeff
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
