This is an example of what removing the indirect call would look like. With the inlined iomap_buffered_write_inline() that gets passed xfs_buffered_write_iomap_begin() and xfs_buffered_write_iomap_end() as constant callbacks, the compiler is able to call the callbacks directly and drop the indirect call through struct iomap_ops: before: callq iomap_file_buffered_write R_X86_64_32S xfs_buffered_write_iomap_ops after (direct calls, R_X86_64_PLT32 == direct): callq ... R_X86_64_PLT32 xfs_buffered_write_iomap_begin-0x4 callq ... R_X86_64_PLT32 xfs_buffered_write_iomap_end-0x4 Suggested-by: Matthew Wilcox (Oracle) Suggested-by: Christoph Hellwig Signed-off-by: Joanne Koong --- fs/iomap/buffered-io.c | 3 ++- fs/iomap/iter.c | 3 ++- fs/xfs/xfs_file.c | 26 ++++++++++++++++++++--- fs/xfs/xfs_iomap.c | 4 ++-- fs/xfs/xfs_iomap.h | 7 +++++++ include/linux/iomap.h | 47 ++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 83 insertions(+), 7 deletions(-) diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index 5a107d59ae27..12aa42f6497e 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -1153,7 +1153,7 @@ static bool iomap_write_end(struct iomap_iter *iter, size_t len, size_t copied, return __iomap_write_end(iter->inode, pos, len, copied, folio); } -static int iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i, +int iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i, const struct iomap_write_ops *write_ops) { ssize_t total_written = 0; @@ -1259,6 +1259,7 @@ static int iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i, return total_written ? 0 : status; } +EXPORT_SYMBOL_GPL(iomap_write_iter); ssize_t iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *i, diff --git a/fs/iomap/iter.c b/fs/iomap/iter.c index 2d5469996a51..a3edb16d3488 100644 --- a/fs/iomap/iter.c +++ b/fs/iomap/iter.c @@ -25,7 +25,7 @@ int iomap_iter_advance(struct iomap_iter *iter, u64 count) return 0; } -static inline void iomap_iter_done(struct iomap_iter *iter) +void iomap_iter_done(struct iomap_iter *iter) { WARN_ON_ONCE(iter->iomap.offset > iter->pos); WARN_ON_ONCE(iter->iomap.length == 0); @@ -38,6 +38,7 @@ static inline void iomap_iter_done(struct iomap_iter *iter) if (iter->srcmap.type != IOMAP_HOLE) trace_iomap_iter_srcmap(iter->inode, &iter->srcmap); } +EXPORT_SYMBOL_GPL(iomap_iter_done); static int iomap_iter_legacy(struct iomap_iter *iter, const struct iomap_ops *ops) { diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index 845a97c9b063..49f5d7f495dd 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -1038,6 +1038,7 @@ xfs_file_buffered_write( { struct inode *inode = iocb->ki_filp->f_mapping->host; struct xfs_inode *ip = XFS_I(inode); + struct iomap_iter iter; ssize_t ret; bool cleared_space = false; unsigned int iolock; @@ -1053,9 +1054,28 @@ xfs_file_buffered_write( goto out; trace_xfs_file_buffered_write(iocb, from); - ret = iomap_file_buffered_write(iocb, from, - &xfs_buffered_write_iomap_ops, &xfs_iomap_write_ops, - NULL); + + /* + * Call inlined iomap buffered write and pass constant begin/end + * function pointers so the compiler can devirtualize them and avoid the + * indirect call through struct iomap_ops. + */ + iter = (struct iomap_iter){ + .inode = inode, + .pos = iocb->ki_pos, + .len = iov_iter_count(from), + .flags = IOMAP_WRITE, + }; + if (iocb->ki_flags & IOCB_NOWAIT) + iter.flags |= IOMAP_NOWAIT; + if (iocb->ki_flags & IOCB_DONTCACHE) + iter.flags |= IOMAP_DONTCACHE; + + ret = iomap_buffered_write_inline(&iter, from, + xfs_buffered_write_iomap_begin, + xfs_buffered_write_iomap_end, &xfs_iomap_write_ops); + if (ret > 0) + iocb->ki_pos = iter.pos; /* * If we hit a space limit, try to free up some lingering preallocated diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c index 3b212bfe04d7..23b1885a1123 100644 --- a/fs/xfs/xfs_iomap.c +++ b/fs/xfs/xfs_iomap.c @@ -1773,7 +1773,7 @@ xfs_zoned_buffered_write_iomap_begin( return error; } -static int +int xfs_buffered_write_iomap_begin( struct inode *inode, loff_t offset, @@ -2124,7 +2124,7 @@ xfs_buffered_write_delalloc_punch( offset, offset + length, iter->private); } -static int +int xfs_buffered_write_iomap_end( struct inode *inode, loff_t offset, diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h index ebcce7d49446..42183e4c461b 100644 --- a/fs/xfs/xfs_iomap.h +++ b/fs/xfs/xfs_iomap.h @@ -19,6 +19,13 @@ int xfs_iomap_write_unwritten(struct xfs_inode *, xfs_off_t, xfs_off_t, bool); xfs_fileoff_t xfs_iomap_eof_align_last_fsb(struct xfs_inode *ip, xfs_fileoff_t end_fsb); +int xfs_buffered_write_iomap_begin(struct inode *inode, loff_t offset, + loff_t count, unsigned flags, struct iomap *iomap, + struct iomap *srcmap); +int xfs_buffered_write_iomap_end(struct inode *inode, loff_t offset, + loff_t length, ssize_t written, unsigned flags, + struct iomap *iomap); + u64 xfs_iomap_inode_sequence(struct xfs_inode *ip, u16 iomap_flags); int xfs_bmbt_to_iomap(struct xfs_inode *ip, struct iomap *iomap, struct xfs_bmbt_irec *imap, unsigned int mapping_flags, diff --git a/include/linux/iomap.h b/include/linux/iomap.h index 335a3858601c..60dc977dca55 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -367,6 +367,8 @@ static inline bool iomap_want_unshare_iter(const struct iomap_iter *iter) ssize_t iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *from, const struct iomap_ops *ops, const struct iomap_write_ops *write_ops, void *private); +int iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i, + const struct iomap_write_ops *write_ops); int iomap_fsverity_write(struct file *file, loff_t pos, size_t length, const void *buf, const struct iomap_ops *ops, const struct iomap_write_ops *write_ops); @@ -689,4 +691,49 @@ static __always_inline int iomap_process(const struct iomap_iter *iter, return ret < 0 ? ret : 1; } +void iomap_iter_done(struct iomap_iter *iter); + +/* + * Inline version of the ->iomap_next call for performance-critical callers. + * When inlined at a call site that passes constant begin / end function + * pointers, the begin and end calls are called directly and avoid the indirect + * call through struct iomap_ops. + */ +static __always_inline int iomap_iter_inline(struct iomap_iter *iter, + iomap_begin_fn begin, iomap_end_fn end) +{ + int ret = iomap_process(iter, &iter->iomap, &iter->srcmap, begin, end); + + if (ret > 0) { + iter->status = 0; + iomap_iter_done(iter); + } + return ret; +} + +/* + * Inline buffered write loop for callers that can supply constant begin and end + * callbacks. This lets the compiler devirtualize the mapping calls and drop the + * indirect call through struct iomap_ops. The passed-in iter should be + * initialized by the caller. This will advance the iter by the number of bytes + * that get successfully written. + * + * This returns the number of bytes written or a negative error. + */ +static __always_inline ssize_t iomap_buffered_write_inline( + struct iomap_iter *iter, struct iov_iter *i, + iomap_begin_fn begin, iomap_end_fn end, + const struct iomap_write_ops *write_ops) +{ + loff_t start_pos = iter->pos; + ssize_t ret; + + while ((ret = iomap_iter_inline(iter, begin, end)) > 0) + iter->status = iomap_write_iter(iter, i, write_ops); + + if (iter->pos == start_pos) + return ret; + return iter->pos - start_pos; +} + #endif /* LINUX_IOMAP_H */ -- 2.52.0