On 10/16/2012 10:48 AM, Carlos Maiolino wrote:
> On Mon, Oct 15, 2012 at 02:30:32PM -0400, Brian Foster wrote:
>> On 10/15/2012 01:19 PM, Carlos Maiolino wrote:
>>> On Fri, Oct 12, 2012 at 11:31:50AM -0400, Brian Foster wrote:
>>>> On 10/11/2012 03:52 PM, Carlos Maiolino wrote:
>>>>> Hi,
>>>>>
...
>> I'm not quite sure I follow what you mean by catching file changes, so I
>> certainly could be missing something...
>
> An example: If you extend a file, you need to check if the file still fits in
> the inode. I'm not sure that only i_size_read(inode) will check for this, that's
> why I added end_offset + i_size_read(), but, I believe it's wrong (end_offset +
> i_size_read).
>
FWIW, there is an i_size_write() in generic_write_end() (via
xfs_vm_write_end()) that looks like it handles the file extension case.
>> but it seems like the code
>> earlier in xfs_vm_writepage() already handles the truncate case. If the
>> file size increased in that time, would this code really care? Wouldn't
>> that mean you'd have to convert the file at that point, presumably when
>> you get a writepage on one of the pages that falls outside the available
>> space?
>>
>> Perhaps this gets more into detection of the current inode format when
>> you have to convert back and forth and this logic naturally becomes more
>> complicated (and thus I'm just reading too far ahead :P).
>>
> Yes, as the patch description says, it doesn't care about file conversion yet :)
> this only cares about write/read new files into inodes, I want to make sure I'm
> doing this part properly, before move on the the next step, which is take care
> of these conversions, if I take care of everything at the same time I'll get
> crazy and have nothing done at the same time :-)
>
I hear you. ;)
Brian
>> Brian
>>
>>>
>>>> Brian
>>>>
>>>>> + err = xfs_inline_write(ip, page, bh, end_offset);
>>>>> +
>>>>> + if (err)
>>>>> + goto error;
>>>>> + return 0;
>>>>> + }
>>>>> +
>>>>> offset = page_offset(page);
>>>>> type = XFS_IO_OVERWRITE;
>>>>>
>>>>> @@ -1624,20 +1723,43 @@ xfs_vm_bmap(
>>>>>
>>>>> STATIC int
>>>>> xfs_vm_readpage(
>>>>> - struct file *unused,
>>>>> + struct file *filp,
>>>>> struct page *page)
>>>>> {
>>>>> - return mpage_readpage(page, xfs_get_blocks);
>>>>> + struct inode *inode = filp->f_mapping->host;
>>>>> + struct xfs_inode *ip = XFS_I(inode);
>>>>> +
>>>>> + if (ip->i_d.di_format == XFS_DINODE_FMT_LOCAL)
>>>>> + return xfs_inline_read(ip, page, page->mapping);
>>>>> + else
>>>>> + return mpage_readpage(page, xfs_get_blocks);
>>>>> }
>>>>>
>>>>> STATIC int
>>>>> xfs_vm_readpages(
>>>>> - struct file *unused,
>>>>> + struct file *filp,
>>>>> struct address_space *mapping,
>>>>> struct list_head *pages,
>>>>> unsigned nr_pages)
>>>>> {
>>>>> - return mpage_readpages(mapping, pages, nr_pages, xfs_get_blocks);
>>>>> + struct inode *inode = filp->f_mapping->host;
>>>>> + struct xfs_inode *ip = XFS_I(inode);
>>>>> +
>>>>> + if (ip->i_d.di_format == XFS_DINODE_FMT_LOCAL) {
>>>>> + struct page *page = list_first_entry(pages, struct page, lru);
>>>>> +
>>>>> + ASSERT(i_size_read(VFS_I(ip) <= PAGE_CACHE_SIZE));
>>>>> +
>>>>> + list_del(&page->lru);
>>>>> + if(!(add_to_page_cache_lru(page, mapping,
>>>>> + page->index, GFP_KERNEL)))
>>>>> + return xfs_inline_read(ip, page, page->mapping);
>>>>> +
>>>>> + page_cache_release(page);
>>>>> + return 0;
>>>>> + } else {
>>>>> + return mpage_readpages(mapping, pages, nr_pages, xfs_get_blocks);
>>>>> + }
>>>>> }
>>>>>
>>>>> const struct address_space_operations xfs_address_space_operations = {
>>>>> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
>>>>> index 2778258..5e56e5c 100644
>>>>> --- a/fs/xfs/xfs_inode.c
>>>>> +++ b/fs/xfs/xfs_inode.c
>>>>> @@ -287,18 +287,6 @@ xfs_iformat(
>>>>> case S_IFDIR:
>>>>> switch (dip->di_format) {
>>>>> case XFS_DINODE_FMT_LOCAL:
>>>>> - /*
>>>>> - * no local regular files yet
>>>>> - */
>>>>> - if (unlikely(S_ISREG(be16_to_cpu(dip->di_mode)))) {
>>>>> - xfs_warn(ip->i_mount,
>>>>> - "corrupt inode %Lu (local format for regular file).",
>>>>> - (unsigned long long) ip->i_ino);
>>>>> - XFS_CORRUPTION_ERROR("xfs_iformat(4)",
>>>>> - XFS_ERRLEVEL_LOW,
>>>>> - ip->i_mount, dip);
>>>>> - return XFS_ERROR(EFSCORRUPTED);
>>>>> - }
>>>>>
>>>>> di_size = be64_to_cpu(dip->di_size);
>>>>> if (unlikely(di_size > XFS_DFORK_DSIZE(dip, ip->i_mount))) {
>>>>> @@ -2471,7 +2459,8 @@ xfs_iflush_int(
>>>>> if (S_ISREG(ip->i_d.di_mode)) {
>>>>> if (XFS_TEST_ERROR(
>>>>> (ip->i_d.di_format != XFS_DINODE_FMT_EXTENTS) &&
>>>>> - (ip->i_d.di_format != XFS_DINODE_FMT_BTREE),
>>>>> + (ip->i_d.di_format != XFS_DINODE_FMT_BTREE) &&
>>>>> + (ip->i_d.di_format != XFS_DINODE_FMT_LOCAL),
>>>>> mp, XFS_ERRTAG_IFLUSH_3, XFS_RANDOM_IFLUSH_3)) {
>>>>> xfs_alert_tag(mp, XFS_PTAG_IFLUSH,
>>>>> "%s: Bad regular inode %Lu, ptr 0x%p",
>>>>>
>>>>
>>>> _______________________________________________
>>>> xfs mailing list
>>>> xfs@xxxxxxxxxxx
>>>> http://oss.sgi.com/mailman/listinfo/xfs
>>>
>>
>> _______________________________________________
>> xfs mailing list
>> xfs@xxxxxxxxxxx
>> http://oss.sgi.com/mailman/listinfo/xfs
>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
