On Mon, Jul 15, 2024 at 04:59:36PM +0100, Matthew Wilcox wrote:
> I'm looking at ->write_begin() / ->write_end() again. Here are our
> current callers:
>
> drivers/gpu/drm/i915/gem/i915_gem_shmem.c:
> [1] shmem_pwrite()
> [2] i915_gem_object_create_shmem_from_data()
> fs/affs/file.c:
> [3] affs_truncate()
> fs/buffer.c:
> [4] generic_cont_expand_simple()
> [5] cont_expand_zero()
> [6] cont_expand_zero()
> fs/exfat/file.c:
> [7] exfat_file_zeroed_range()
> fs/ext4/verity.c:
> [8] pagecache_write()
> fs/f2fs/super.c:
> [9] f2fs_quota_write()
> fs/f2fs/verity.c:
> [A] pagecache_write()
> fs/namei.c:
> [B] page_symlink()
> mm/filemap.c:
> [C] generic_perform_write()
I found a few variants of the same pattern:
fs/hfs/extent.c:
[D] hfs_file_truncate()
fs/hfsplus/extents.c:
[E] hfsplus_file_truncate()
fs/ntfs3/file.c:
[F] ntfs_extend_initialized_size()
> There are essentially four things that happen between ->write_begin()
> and ->write_end() in these 12 callers:
>
> - copy_from_user [1]
> - memcpy [289AB]
> - zero [567]
- zero [567F]
> - nothing [34]
- nothing [34DE]
> - copy_from_iter [C]
>
> I suspect that exfat_file_zeroed_range() should be calling
> cont_expand_zero(), which means it would need to be exported, but
> that seems like an improvement over calling write_begin/write_end
> itself.
>
> The copy_from_user() / memcpy() users feel like they should all end
> up calling ->write_iter(). One way they could do this is by calling
> kernel_write() / __kernel_write(), but I'm not sure whether they
> should have the various accounting things (add_wchar(), inc_syscw())
> that happen inside __kernel_write_iter().
>
> So should we add:
>
> ssize_t filemap_write_iter(struct file *file, struct iov_iter *from)
> { ... }
>
> which contains the guts of __kernel_write_iter?
> ext4's verity code needs a minor refactor to pass down the file
> (but note comment about how it's a RO file descriptor)
> f2fs_quota_write doesn't have a struct file and looks generally awkward.
> page_symlink() is also awkward.
>
> I think that means we need something that _doesn't work_ for iomap-based
> filesystems. All of these callers know the filesystem they're working
> on doesn't use iomap, so perhaps filemap_write_iter() just takes a
> struct address_space and assumes the existance of
> ->write_begin/->write_end.
>
> Thoughts?
>
>
