Re: [Linux-NTFS-Dev] [PATCH v5 6/7] fs: pass iocb to generic_write_sync

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 6, 2014 at 5:52 AM, Anton Altaparmakov <aia21@xxxxxxxxx> wrote:
> Hi,
>
>> On 5 Nov 2014, at 23:14, Milosz Tanski <milosz@xxxxxxxxx> wrote:
>>
>> From: Christoph Hellwig <hch@xxxxxx>
>>
>> Clean up the generic_write_sync by just passing an iocb and a bytes
>> written / negative errno argument.  In addition to simplifying the
>> callers this also prepares for passing a per-operation O_DSYNC
>> flag.  Two callers didn't quite fit that scheme:
>>
>> - dio_complete didn't both to update ki_pos as we don't need it
>>   on a iocb that is about to be freed, so we had to add it. Additionally
>>   it also synced out written data in the error case, which has been
>>   changed to operate like the other callers.
>> - gfs2 also used generic_write_sync to implement a crude version
>>   of fallocate.  It has been switched to use an open coded variant
>>   instead.
>>
>> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
>> ---
>> fs/block_dev.c     |  8 +-------
>> fs/btrfs/file.c    |  7 ++-----
>> fs/cifs/file.c     |  8 +-------
>> fs/direct-io.c     |  8 ++------
>> fs/ext4/file.c     |  8 +-------
>> fs/gfs2/file.c     |  9 +++++++--
>> fs/ntfs/file.c     |  8 ++------
>> fs/udf/file.c      | 11 ++---------
>> fs/xfs/xfs_file.c  |  8 +-------
>> include/linux/fs.h |  8 +-------
>> mm/filemap.c       | 30 ++++++++++++++++++++----------
>> 11 files changed, 40 insertions(+), 73 deletions(-)
>>
>> diff --git a/fs/block_dev.c b/fs/block_dev.c
>> index cc9d411..c529b1c 100644
>> --- a/fs/block_dev.c
>> +++ b/fs/block_dev.c
>> @@ -1568,18 +1568,12 @@ static long block_ioctl(struct file *file, unsigned cmd, unsigned long arg)
>>  */
>> ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
>> {
>> -     struct file *file = iocb->ki_filp;
>>       struct blk_plug plug;
>>       ssize_t ret;
>>
>>       blk_start_plug(&plug);
>>       ret = __generic_file_write_iter(iocb, from);
>> -     if (ret > 0) {
>> -             ssize_t err;
>> -             err = generic_write_sync(file, iocb->ki_pos - ret, ret);
>> -             if (err < 0)
>> -                     ret = err;
>> -     }
>> +     ret = generic_write_sync(iocb, ret);
>>       blk_finish_plug(&plug);
>>       return ret;
>> }
>> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
>> index a18ceab..4f4a6f7 100644
>> --- a/fs/btrfs/file.c
>> +++ b/fs/btrfs/file.c
>> @@ -1820,11 +1820,8 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
>>        */
>>       BTRFS_I(inode)->last_trans = root->fs_info->generation + 1;
>>       BTRFS_I(inode)->last_sub_trans = root->log_transid;
>> -     if (num_written > 0) {
>> -             err = generic_write_sync(file, pos, num_written);
>> -             if (err < 0)
>> -                     num_written = err;
>> -     }
>> +
>> +     num_written = generic_write_sync(iocb, num_written);
>>
>>       if (sync)
>>               atomic_dec(&BTRFS_I(inode)->sync_writers);
>> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
>> index c485afa..32359de 100644
>> --- a/fs/cifs/file.c
>> +++ b/fs/cifs/file.c
>> @@ -2706,13 +2706,7 @@ cifs_writev(struct kiocb *iocb, struct iov_iter *from)
>>               rc = __generic_file_write_iter(iocb, from);
>>               mutex_unlock(&inode->i_mutex);
>>
>> -             if (rc > 0) {
>> -                     ssize_t err;
>> -
>> -                     err = generic_write_sync(file, iocb->ki_pos - rc, rc);
>> -                     if (err < 0)
>> -                             rc = err;
>> -             }
>> +             rc = generic_write_sync(iocb, rc);
>>       } else {
>>               mutex_unlock(&inode->i_mutex);
>>       }
>> diff --git a/fs/direct-io.c b/fs/direct-io.c
>> index e181b6b..b72ac83 100644
>> --- a/fs/direct-io.c
>> +++ b/fs/direct-io.c
>> @@ -257,12 +257,8 @@ static ssize_t dio_complete(struct dio *dio, loff_t offset, ssize_t ret,
>>       inode_dio_done(dio->inode);
>>       if (is_async) {
>>               if (dio->rw & WRITE) {
>> -                     int err;
>> -
>> -                     err = generic_write_sync(dio->iocb->ki_filp, offset,
>> -                                              transferred);
>> -                     if (err < 0 && ret > 0)
>> -                             ret = err;
>> +                     dio->iocb->ki_pos = offset + transferred;
>> +                     ret = generic_write_sync(dio->iocb, ret);
>>               }
>>
>>               aio_complete(dio->iocb, ret, 0);
>> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
>> index aca7b24..79b000c 100644
>> --- a/fs/ext4/file.c
>> +++ b/fs/ext4/file.c
>> @@ -175,13 +175,7 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>>       ret = __generic_file_write_iter(iocb, from);
>>       mutex_unlock(&inode->i_mutex);
>>
>> -     if (ret > 0) {
>> -             ssize_t err;
>> -
>> -             err = generic_write_sync(file, iocb->ki_pos - ret, ret);
>> -             if (err < 0)
>> -                     ret = err;
>> -     }
>> +     ret = generic_write_sync(iocb, ret);
>>       if (o_direct)
>>               blk_finish_plug(&plug);
>>
>> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
>> index 80dd44d..3fafeca 100644
>> --- a/fs/gfs2/file.c
>> +++ b/fs/gfs2/file.c
>> @@ -895,8 +895,13 @@ retry:
>>               gfs2_quota_unlock(ip);
>>       }
>>
>> -     if (error == 0)
>> -             error = generic_write_sync(file, pos, count);
>> +     if (error)
>> +             goto out_unlock;
>> +
>> +     if ((file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host)) {
>> +             error = vfs_fsync_range(file, pos, pos + count - 1,
>> +                            (file->f_flags & __O_SYNC) ? 0 : 1);
>> +     }
>>       goto out_unlock;
>>
>> out_trans_fail:
>> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
>> index 643faa4..4f3d664 100644
>> --- a/fs/ntfs/file.c
>> +++ b/fs/ntfs/file.c
>> @@ -2127,12 +2127,8 @@ static ssize_t ntfs_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
>>       mutex_lock(&inode->i_mutex);
>>       ret = ntfs_file_aio_write_nolock(iocb, iov, nr_segs, &iocb->ki_pos);
>>       mutex_unlock(&inode->i_mutex);
>> -     if (ret > 0) {
>> -             int err = generic_write_sync(file, iocb->ki_pos - ret, ret);
>> -             if (err < 0)
>> -                     ret = err;
>> -     }
>> -     return ret;
>> +
>> +     return generic_write_sync(iocb, ret);
>> }
>>
>> /**
>> diff --git a/fs/udf/file.c b/fs/udf/file.c
>> index bb15771..1cdabd0 100644
>> --- a/fs/udf/file.c
>> +++ b/fs/udf/file.c
>> @@ -155,16 +155,9 @@ static ssize_t udf_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>>       retval = __generic_file_write_iter(iocb, from);
>>       mutex_unlock(&inode->i_mutex);
>>
>> -     if (retval > 0) {
>> -             ssize_t err;
>> -
>> +     if (retval > 0)
>>               mark_inode_dirty(inode);
>> -             err = generic_write_sync(file, iocb->ki_pos - retval, retval);
>> -             if (err < 0)
>> -                     retval = err;
>> -     }
>> -
>> -     return retval;
>> +     return generic_write_sync(iocb, retval);
>> }
>>
>> long udf_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
>> index 0655915..a8cab66 100644
>> --- a/fs/xfs/xfs_file.c
>> +++ b/fs/xfs/xfs_file.c
>> @@ -792,14 +792,8 @@ xfs_file_write_iter(
>>               ret = xfs_file_buffered_aio_write(iocb, from);
>>
>>       if (ret > 0) {
>> -             ssize_t err;
>> -
>>               XFS_STATS_ADD(xs_write_bytes, ret);
>> -
>> -             /* Handle various SYNC-type writes */
>> -             err = generic_write_sync(file, iocb->ki_pos - ret, ret);
>> -             if (err < 0)
>> -                     ret = err;
>> +             ret = generic_write_sync(iocb, ret);
>>       }
>>       return ret;
>> }
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index eaebd99..7d0e116 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -2242,13 +2242,7 @@ extern int filemap_fdatawrite_range(struct address_space *mapping,
>> extern int vfs_fsync_range(struct file *file, loff_t start, loff_t end,
>>                          int datasync);
>> extern int vfs_fsync(struct file *file, int datasync);
>> -static inline int generic_write_sync(struct file *file, loff_t pos, loff_t count)
>> -{
>> -     if (!(file->f_flags & O_DSYNC) && !IS_SYNC(file->f_mapping->host))
>> -             return 0;
>> -     return vfs_fsync_range(file, pos, pos + count - 1,
>> -                            (file->f_flags & __O_SYNC) ? 0 : 1);
>> -}
>> +extern int generic_write_sync(struct kiocb *iocb, loff_t count);
>> extern void emergency_sync(void);
>> extern void emergency_remount(void);
>> #ifdef CONFIG_BLOCK
>> diff --git a/mm/filemap.c b/mm/filemap.c
>> index 09d3af3..6107058 100644
>> --- a/mm/filemap.c
>> +++ b/mm/filemap.c
>> @@ -2664,6 +2664,24 @@ out:
>> }
>> EXPORT_SYMBOL(__generic_file_write_iter);
>>
>> +int generic_write_sync(struct kiocb *iocb, loff_t count)
>> +{
>> +     struct file *file = iocb->ki_filp;
>> +
>> +     if (count > 0 &&
>> +         ((file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host))) {
>> +             bool fdatasync = !(file->f_flags & __O_SYNC);
>> +             ssize_t ret = 0;
>
> That "= 0" is pointless.  "ret" is overwritten unconditionally on the following line...

I have fixed this change; it will be in the next patch series / pull
request. The branch for the pull is at:
https://bitbucket.org/adfin/linux-fs.git  read_call_6

>
> Other than that the NTFS bits are:
>
> Acked-by: Anton Altaparmakov <anton@xxxxxxxxxx>
>
> Best regards,
>
>         Anton
>
>> +
>> +             ret = vfs_fsync_range(file, iocb->ki_pos - count,
>> +                             iocb->ki_pos - 1, fdatasync);
>> +             if (ret < 0)
>> +                     return ret;
>> +     }
>> +     return count;
>> +}
>> +EXPORT_SYMBOL(generic_write_sync);
>> +
>> /**
>>  * generic_file_write_iter - write data to a file
>>  * @iocb:     IO state structure
>> @@ -2675,22 +2693,14 @@ EXPORT_SYMBOL(__generic_file_write_iter);
>>  */
>> ssize_t generic_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>> {
>> -     struct file *file = iocb->ki_filp;
>> -     struct inode *inode = file->f_mapping->host;
>> +     struct inode *inode = iocb->ki_filp->f_mapping->host;
>>       ssize_t ret;
>>
>>       mutex_lock(&inode->i_mutex);
>>       ret = __generic_file_write_iter(iocb, from);
>>       mutex_unlock(&inode->i_mutex);
>>
>> -     if (ret > 0) {
>> -             ssize_t err;
>> -
>> -             err = generic_write_sync(file, iocb->ki_pos - ret, ret);
>> -             if (err < 0)
>> -                     ret = err;
>> -     }
>> -     return ret;
>> +     return generic_write_sync(iocb, ret);
>> }
>> EXPORT_SYMBOL(generic_file_write_iter);
>
> --
> Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
> University of Cambridge Information Services, Roger Needham Building
> 7 JJ Thomson Avenue, Cambridge, CB3 0RB, UK
>



-- 
Milosz Tanski
CTO
16 East 34th Street, 15th floor
New York, NY 10016

p: 646-253-9055
e: milosz@xxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux