On Wed, Mar 01, 2017 at 04:48:20PM +0800, Qu Wenruo wrote:
> 
> 
> At 03/01/2017 09:04 AM, Liu Bo wrote:
> > @pos, not aligned @start_pos, should be used to check whether the eof page
> > needs to be marked as readonly, thus @start_pos can be removed.
> > 
> > Signed-off-by: Liu Bo <bo.li....@oracle.com>
> > ---
> >  fs/btrfs/file.c | 7 +------
> >  1 file changed, 1 insertion(+), 6 deletions(-)
> > 
> > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> > index 0be837b..ef88e6d 100644
> > --- a/fs/btrfs/file.c
> > +++ b/fs/btrfs/file.c
> > @@ -1814,7 +1814,6 @@ static ssize_t btrfs_file_write_iter(struct kiocb 
> > *iocb,
> >     struct inode *inode = file_inode(file);
> >     struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
> >     struct btrfs_root *root = BTRFS_I(inode)->root;
> > -   u64 start_pos;
> >     u64 end_pos;
> >     ssize_t num_written = 0;
> >     bool sync = (file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host);
> > @@ -1822,7 +1821,6 @@ static ssize_t btrfs_file_write_iter(struct kiocb 
> > *iocb,
> >     loff_t pos;
> >     size_t count;
> >     loff_t oldsize;
> > -   int clean_page = 0;
> > 
> >     inode_lock(inode);
> >     err = generic_write_checks(iocb, from);
> > @@ -1860,7 +1858,6 @@ static ssize_t btrfs_file_write_iter(struct kiocb 
> > *iocb,
> > 
> >     pos = iocb->ki_pos;
> >     count = iov_iter_count(from);
> > -   start_pos = round_down(pos, fs_info->sectorsize);
> >     end_pos = round_up(pos + count, fs_info->sectorsize);
> >     oldsize = i_size_read(inode);
> >     if (end_pos > oldsize) {
> > @@ -1870,8 +1867,6 @@ static ssize_t btrfs_file_write_iter(struct kiocb 
> > *iocb,
> >                     inode_unlock(inode);
> >                     goto out;
> >             }
> > -           if (start_pos > round_up(oldsize, fs_info->sectorsize))
> > -                   clean_page = 1;
> >     }
> > 
> >     if (sync)
> > @@ -1883,7 +1878,7 @@ static ssize_t btrfs_file_write_iter(struct kiocb 
> > *iocb,
> >             num_written = __btrfs_buffered_write(file, from, pos);
> >             if (num_written > 0)
> >                     iocb->ki_pos = pos + num_written;
> > -           if (clean_page)
> > +           if (oldsize < pos)
> >                     pagecache_isize_extended(inode, oldsize,
> >                                             i_size_read(inode));
> 
> Not familiar with page cache, so I can be totally wrong here.
> 
> But what will happen if @oldsize and @pos are in the same page?
> 
> For example:
> Page start                                    Page start + 4K
> |             |                |              |
>               old size         pos
> 
> Do we still need to call pagecache_iszie_extented() since we will dirty that
> page anyway?

Yes, isize has changed, if blocksize < pagesize, so it's still possible that the
next write access to the new isize doesn't own an block since no page_mkwrite()
has been called to allocate it, then a following writepage() may fail silently
from userspace's view (unless they run fsync and check its ret).

Thanks,

-liubo
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to