> -----Original Message-----
> From: Namjae Jeon <linkinjeon@xxxxxxxxxx>
> Sent: Monday, August 14, 2023 12:36 PM
> To: Mo, Yuezhang <Yuezhang.Mo@xxxxxxxx>
> Cc: sj1557.seo@xxxxxxxxxxx; linux-fsdevel@xxxxxxxxxxxxxxx; Wu, Andy
> <Andy.Wu@xxxxxxxx>; Aoyama, Wataru (SGC) <Wataru.Aoyama@xxxxxxxx>
> Subject: Re: [PATCH v2 1/2] exfat: change to get file size from DataLength
>
> [snip]
> > +static ssize_t exfat_file_write_iter(struct kiocb *iocb, struct iov_iter
> > *iter)
> > +{
> > + ssize_t ret;
> > + struct file *file = iocb->ki_filp;
> > + struct inode *inode = file_inode(file);
> > + struct exfat_inode_info *ei = EXFAT_I(inode);
> > + loff_t pos = iocb->ki_pos;
> > + loff_t valid_size;
> > +
> > + inode_lock(inode);
> > +
> > + valid_size = ei->valid_size;
> > +
> > + ret = generic_write_checks(iocb, iter);
> > + if (ret < 0)
> > + goto unlock;
> > +
> > + if (pos > valid_size) {
> > + ret = exfat_file_zeroed_range(file, valid_size, pos);
> Can we use block_write_begin instead of cont_write_begin in
> exfat_write_begin?
Yes.
exfat_get_block() should be exported if block_write_begin() is used.
>
> > + if (ret < 0 && ret != -ENOSPC) {
> > + exfat_err(inode->i_sb,
> > + "write: fail to zero from %llu to %llu(%ld)",
> > + valid_size, pos, ret);
> > + }
> > + if (ret < 0)
> > + goto unlock;
> > + }
> > +
> > + ret = __generic_file_write_iter(iocb, iter);
> > + if (ret < 0)
> Probably It should be if (ret <= 0)...
In the case of ret==0, exfat_file_zeroed_range() may have written zeros to page cache, and the
following vfs_fsync_range() is needed to synchronize these data.
>
> > + goto unlock;
> > +
> > + if (pos + ret > i_size_read(inode))
> > + i_size_write(inode, pos + ret);
> > +
> > + if (pos + ret > ei->valid_size)
> > + ei->valid_size = pos + ret;
> > +
> > + /*
> > + * If valid_size is extended with sector-aligned length in
> > + * exfat_get_block(), set to the writren length.
> > + */
> > + if (i_size_read(inode) < ei->valid_size)
> > + ei->valid_size = i_size_read(inode);
> > +
> > + mark_inode_dirty(inode);
> > + inode_unlock(inode);
> > +
> > + if (pos > valid_size && iocb_is_dsync(iocb)) {
> > + ssize_t err = vfs_fsync_range(file, valid_size, pos - 1,
> > + iocb->ki_flags & IOCB_SYNC);
> It should be moved to exfat_file_zeroed_range() ?
The inode lock is locked when exfat_file_zeroed_range() is called, and vfs_fsync_range() may
need to synchronize a lot of data. If vfs_fsync_range() is called in exfat_file_zeroed_range(),
the inode lock will be locked for a long time.
