linux-ext4 - Re: [PATCH v3 3/4] fs: change write_begin/write

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aF68sKzx24P1q54h@casper.infradead.org>
Date: Fri, 27 Jun 2025 16:45:52 +0100
From: Matthew Wilcox <willy@...radead.org>
To: 陈涛涛 Taotao Chen <chentaotao@...iglobal.com>
Cc: "tytso@....edu" <tytso@....edu>,
	"hch@...radead.org" <hch@...radead.org>,
	"adilger.kernel@...ger.ca" <adilger.kernel@...ger.ca>,
	"brauner@...nel.org" <brauner@...nel.org>,
	"jani.nikula@...ux.intel.com" <jani.nikula@...ux.intel.com>,
	"rodrigo.vivi@...el.com" <rodrigo.vivi@...el.com>,
	"tursulin@...ulin.net" <tursulin@...ulin.net>,
	"airlied@...il.com" <airlied@...il.com>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	"linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
	"linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
	"intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"chentao325@...com" <chentao325@...com>,
	"frank.li@...o.com" <frank.li@...o.com>
Subject: Re: [PATCH v3 3/4] fs: change write_begin/write_end interface to
 take struct kiocb *

On Fri, Jun 27, 2025 at 11:03:11AM +0000, 陈涛涛 Taotao Chen wrote:
> diff --git a/fs/exfat/file.c b/fs/exfat/file.c
> index 841a5b18e3df..fdc2fa1e5c41 100644
> --- a/fs/exfat/file.c
> +++ b/fs/exfat/file.c
> @@ -532,10 +532,12 @@ int exfat_file_fsync(struct file *filp, loff_t start, loff_t end, int datasync)
>  	return blkdev_issue_flush(inode->i_sb->s_bdev);
>  }
>  
> -static int exfat_extend_valid_size(struct file *file, loff_t new_valid_size)
> +static int exfat_extend_valid_size(const struct kiocb *iocb,
> +				   loff_t new_valid_size)
>  {
>  	int err;
>  	loff_t pos;
> +	struct file *file = iocb->ki_filp;
>  	struct inode *inode = file_inode(file);
>  	struct exfat_inode_info *ei = EXFAT_I(inode);
>  	struct address_space *mapping = inode->i_mapping;
> @@ -551,14 +553,14 @@ static int exfat_extend_valid_size(struct file *file, loff_t new_valid_size)
>  		if (pos + len > new_valid_size)
>  			len = new_valid_size - pos;
>  
> -		err = ops->write_begin(file, mapping, pos, len, &folio, NULL);
> +		err = ops->write_begin(iocb, mapping, pos, len, &folio, NULL);
>  		if (err)
>  			goto out;
>  
>  		off = offset_in_folio(folio, pos);
>  		folio_zero_new_buffers(folio, off, off + len);
>  
> -		err = ops->write_end(file, mapping, pos, len, len, folio, NULL);
> +		err = ops->write_end(iocb, mapping, pos, len, len, folio, NULL);
>  		if (err < 0)
>  			goto out;
>  		pos += len;
> @@ -604,7 +606,7 @@ static ssize_t exfat_file_write_iter(struct kiocb *iocb, struct iov_iter *iter)
>  	}
>  
>  	if (pos > valid_size) {
> -		ret = exfat_extend_valid_size(file, pos);
> +		ret = exfat_extend_valid_size(iocb, pos);
>  		if (ret < 0 && ret != -ENOSPC) {
>  			exfat_err(inode->i_sb,
>  				"write: fail to zero from %llu to %llu(%zd)",
> @@ -655,8 +657,11 @@ static vm_fault_t exfat_page_mkwrite(struct vm_fault *vmf)
>  	struct file *file = vma->vm_file;
>  	struct inode *inode = file_inode(file);
>  	struct exfat_inode_info *ei = EXFAT_I(inode);
> +	struct kiocb iocb;
>  	loff_t start, end;
>  
> +	init_sync_kiocb(&iocb, file);
> +
>  	if (!inode_trylock(inode))
>  		return VM_FAULT_RETRY;
>  
> @@ -665,7 +670,7 @@ static vm_fault_t exfat_page_mkwrite(struct vm_fault *vmf)
>  			start + vma->vm_end - vma->vm_start);
>  
>  	if (ei->valid_size < end) {
> -		err = exfat_extend_valid_size(file, end);
> +		err = exfat_extend_valid_size(&iocb, end);
>  		if (err < 0) {
>  			inode_unlock(inode);
>  			return vmf_fs_error(err);

This is unnecessary work.  The only ->write_begin/write_end that we'll
see here is exfat_write_begin() / exfat_write_end() which don't actually
need iocb (or file).  Traditionally we pass NULL in these situations,
but the exfat people probably weren't aware of this convention.

exfat_extend_valid_size() only uses the file it's passed to get the
inode, and both callers already have the inode.  So I'd change
exfat_extend_valid_size() to take an inode instead of a file as its
first argument, then you can skip the creation of an iocb in
exfat_page_mkwrite().