* [RFC PATCH v2 1/4] ocfs2: Add new ocfs2_map_blocks() to introduce iomap feature
2026-07-27 6:17 [RFC PATCH v2 0/4] migrates ocfs2 DIO from buffer_head to iomap Heming Zhao
@ 2026-07-27 6:17 ` Heming Zhao
2026-07-28 5:41 ` Joseph Qi
2026-07-27 6:17 ` [RFC PATCH v2 2/4] ocfs2: switch dio read path from buffer_head to iomap Heming Zhao
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Heming Zhao @ 2026-07-27 6:17 UTC (permalink / raw)
To: joseph.qi, mark, jlbec, hch; +Cc: Heming Zhao, ocfs2-devel, linux-kernel
As part of migrating OCFS2 DIO read/write code paths towards the modern
and high-performant iomap framework, this patch introduces an iomap API
to replace old high-overhead VFS buffer_head structure paths.
This patch establishes the foundational block mapping routines required by
subsequent iomap integration patches. The implementation draws
inspiration from ext4_map_blocks().
Signed-off-by: Heming Zhao <heming.zhao@suse.com>
---
fs/ocfs2/aops.c | 94 +++++++++++++++++++++++++++++++++++++++
fs/ocfs2/aops.h | 2 +
fs/ocfs2/buffer_head_io.c | 12 -----
fs/ocfs2/ocfs2.h | 45 ++++++++++++++++++-
4 files changed, 140 insertions(+), 13 deletions(-)
diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
index 4acdbb70882c..08df5e3b5196 100644
--- a/fs/ocfs2/aops.c
+++ b/fs/ocfs2/aops.c
@@ -126,6 +126,100 @@ static int ocfs2_lock_get_block(struct inode *inode, sector_t iblock,
return ret;
}
+int ocfs2_map_blocks(struct inode *inode, struct ocfs2_map_block *map,
+ int flags)
+{
+ int err = 0;
+ unsigned int ext_flags;
+ u64 max_blocks = map->len;
+ u64 p_blkno, count, past_eof;
+ struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+ int create = flags & OCFS2_GET_BLOCKS_CREATE;
+
+ if (OCFS2_I(inode)->ip_flags & OCFS2_INODE_SYSTEM_FILE)
+ mlog(ML_NOTICE, "map_block on system inode 0x%p (%llu)\n",
+ inode, inode->i_ino);
+
+ if (S_ISLNK(inode->i_mode)) {
+ /*
+ * TODO: refer ocfs2_get_block() to handle
+ * ocfs2_read_folio in the future
+ */
+ mlog(ML_NOTICE, "map_block on S_ISLNK file, node 0x%p (%llu)\n",
+ inode, inode->i_ino);
+ dump_stack();
+ goto bail;
+ }
+
+ err = ocfs2_extent_map_get_blocks(inode, map->lblk, &p_blkno, &count,
+ &ext_flags);
+ if (err) {
+ mlog(ML_ERROR, "get_blocks() failed, inode: 0x%p, "
+ "block: %llu\n", inode, map->lblk);
+ goto bail;
+ }
+
+ if (max_blocks < count)
+ count = max_blocks;
+
+ map->pblk = p_blkno;
+ map->len = count;
+
+ /*
+ * ocfs2 never allocates in this function - the only time we
+ * need to use MAP_NEW is when we're extending i_size on a file
+ * system which doesn't support holes, in which case MAP_NEW
+ * allows __block_write_begin() to zero.
+ *
+ * If we see this on a sparse file system, then a truncate has
+ * raced us and removed the cluster. In this case, we clear
+ * the buffers dirty and uptodate bits and let the buffer code
+ * ignore it as a hole.
+ */
+ if (create && map->pblk == 0 && ocfs2_sparse_alloc(osb)) {
+ map->flags &= ~(OCFS2_MAP_DIRTY | OCFS2_MAP_UPTODATE);
+ goto bail;
+ }
+
+ if (p_blkno) {
+ if (ext_flags & OCFS2_EXT_UNWRITTEN) {
+ map->flags |= OCFS2_MAP_UNWRITTEN;
+ } else if (!(ext_flags & OCFS2_EXT_UNWRITTEN)) {
+ /* Treat the unwritten extent as a hole for zeroing purposes. */
+ map->flags |= OCFS2_MAP_MAPPED;
+ } else {
+ /* nothing to do */
+ }
+ }
+
+ if (!ocfs2_sparse_alloc(osb)) {
+ if (map->pblk == 0) {
+ err = -EIO;
+ mlog(ML_ERROR,
+ "iblock = %llu p_blkno = %llu blkno=(%llu)\n",
+ (unsigned long long)map->lblk,
+ (unsigned long long)map->pblk,
+ (unsigned long long)OCFS2_I(inode)->ip_blkno);
+ mlog(ML_ERROR, "Size %llu, clusters %u\n",
+ (unsigned long long)i_size_read(inode),
+ OCFS2_I(inode)->ip_clusters);
+ dump_stack();
+ goto bail;
+ }
+ }
+
+ past_eof = ocfs2_blocks_for_bytes(inode->i_sb, i_size_read(inode));
+
+ if (create && (map->lblk >= past_eof))
+ map->flags |= OCFS2_MAP_NEW;
+
+bail:
+ if (err < 0)
+ return -EIO;
+ else
+ return map->len;
+}
+
int ocfs2_get_block(struct inode *inode, sector_t iblock,
struct buffer_head *bh_result, int create)
{
diff --git a/fs/ocfs2/aops.h b/fs/ocfs2/aops.h
index 114efc9111e4..8dd6edd7c1a1 100644
--- a/fs/ocfs2/aops.h
+++ b/fs/ocfs2/aops.h
@@ -42,6 +42,8 @@ int ocfs2_size_fits_inline_data(struct buffer_head *di_bh, u64 new_size);
int ocfs2_get_block(struct inode *inode, sector_t iblock,
struct buffer_head *bh_result, int create);
+int ocfs2_map_blocks(struct inode *inode, struct ocfs2_map_block *map,
+ int flags);
/* all ocfs2_dio_end_io()'s fault */
#define ocfs2_iocb_is_rw_locked(iocb) \
test_bit(0, (unsigned long *)&iocb->private)
diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
index 7bfe377af2df..493f2209cca5 100644
--- a/fs/ocfs2/buffer_head_io.c
+++ b/fs/ocfs2/buffer_head_io.c
@@ -23,18 +23,6 @@
#include "buffer_head_io.h"
#include "ocfs2_trace.h"
-/*
- * Bits on bh->b_state used by ocfs2.
- *
- * These MUST be after the JBD2 bits. Hence, we use BH_JBDPrivateStart.
- */
-enum ocfs2_state_bits {
- BH_NeedsValidate = BH_JBDPrivateStart,
-};
-
-/* Expand the magic b_state functions */
-BUFFER_FNS(NeedsValidate, needs_validate);
-
int ocfs2_write_block(struct ocfs2_super *osb, struct buffer_head *bh,
struct ocfs2_caching_info *ci)
{
diff --git a/fs/ocfs2/ocfs2.h b/fs/ocfs2/ocfs2.h
index 62cad6522c7a..095f7ae5dded 100644
--- a/fs/ocfs2/ocfs2.h
+++ b/fs/ocfs2/ocfs2.h
@@ -509,7 +509,50 @@ struct ocfs2_super
struct ocfs2_filecheck_sysfs_entry osb_fc_ent;
};
-#define OCFS2_SB(sb) ((struct ocfs2_super *)(sb)->s_fs_info)
+/*
+ * Bits on bh->b_state used by ocfs2.
+ *
+ * These MUST be after the JBD2 bits. Hence, we use BH_JBDPrivateStart.
+ */
+enum ocfs2_state_bits {
+ BH_NeedsValidate = BH_JBDPrivateStart,
+};
+
+/* Expand the magic b_state functions */
+BUFFER_FNS(NeedsValidate, needs_validate);
+
+/*
+ * Logical to physical block mapping, used by ocfs2_map_blocks()
+ *
+ * This structure is used to pass requests into ocfs2_map_blocks() as
+ * well as to store the information returned by ocfs2_map_blocks(). It
+ * takes less room on the stack than a struct buffer_head.
+ */
+#define OCFS2_MAP_NEW BIT(BH_New)
+#define OCFS2_MAP_MAPPED BIT(BH_Mapped)
+#define OCFS2_MAP_UNWRITTEN BIT(BH_Unwritten)
+/* useless? #define OCFS2_MAP_BOUNDARY BIT(BH_Boundary) */
+/* useless? #define OCFS2_MAP_DELAYED BIT(BH_Delay) */
+#define OCFS2_MAP_DIRTY BIT(BH_Dirty)
+#define OCFS2_MAP_UPTODATE BIT(BH_Uptodate)
+#define OCFS2_MAP_NEEDS_VALIDATE BIT(BH_NeedsValidate)
+#define OCFS2_MAP_DEFER_COMPLETION BIT(BH_Defer_Completion)
+#define OCFS2_MAP_FLAGS (OCFS2_MAP_NEW | OCFS2_MAP_MAPPED |\
+ OCFS2_MAP_DIRTY | OCFS2_MAP_UPTODATE |\
+ OCFS2_MAP_NEEDS_VALIDATE |\
+ OCFS2_MAP_DEFER_COMPLETION)
+
+struct ocfs2_map_block {
+ u64 pblk; /* physical block# */
+ u64 lblk; /* logical block# */
+ u64 len; /* number of block */
+ unsigned int flags;
+};
+
+/* Flags used by ocfs2_map_blocks() */
+#define OCFS2_GET_BLOCKS_CREATE (0x0001)
+
+#define OCFS2_SB(sb) ((struct ocfs2_super *)(sb)->s_fs_info)
/* Useful typedef for passing around journal access functions */
typedef int (*ocfs2_journal_access_func)(handle_t *handle,
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [RFC PATCH v2 1/4] ocfs2: Add new ocfs2_map_blocks() to introduce iomap feature
2026-07-27 6:17 ` [RFC PATCH v2 1/4] ocfs2: Add new ocfs2_map_blocks() to introduce iomap feature Heming Zhao
@ 2026-07-28 5:41 ` Joseph Qi
0 siblings, 0 replies; 14+ messages in thread
From: Joseph Qi @ 2026-07-28 5:41 UTC (permalink / raw)
To: Heming Zhao; +Cc: mark, jlbec, hch, ocfs2-devel, linux-kernel
On 7/27/26 2:17 PM, Heming Zhao wrote:
> As part of migrating OCFS2 DIO read/write code paths towards the modern
> and high-performant iomap framework, this patch introduces an iomap API
> to replace old high-overhead VFS buffer_head structure paths.
>
> This patch establishes the foundational block mapping routines required by
> subsequent iomap integration patches. The implementation draws
> inspiration from ext4_map_blocks().
>
> Signed-off-by: Heming Zhao <heming.zhao@suse.com>
> ---
> fs/ocfs2/aops.c | 94 +++++++++++++++++++++++++++++++++++++++
> fs/ocfs2/aops.h | 2 +
> fs/ocfs2/buffer_head_io.c | 12 -----
> fs/ocfs2/ocfs2.h | 45 ++++++++++++++++++-
> 4 files changed, 140 insertions(+), 13 deletions(-)
>
> diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
> index 4acdbb70882c..08df5e3b5196 100644
> --- a/fs/ocfs2/aops.c
> +++ b/fs/ocfs2/aops.c
> @@ -126,6 +126,100 @@ static int ocfs2_lock_get_block(struct inode *inode, sector_t iblock,
> return ret;
> }
>
> +int ocfs2_map_blocks(struct inode *inode, struct ocfs2_map_block *map,
> + int flags)
> +{
> + int err = 0;
> + unsigned int ext_flags;
> + u64 max_blocks = map->len;
> + u64 p_blkno, count, past_eof;
> + struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
> + int create = flags & OCFS2_GET_BLOCKS_CREATE;
> +
> + if (OCFS2_I(inode)->ip_flags & OCFS2_INODE_SYSTEM_FILE)
> + mlog(ML_NOTICE, "map_block on system inode 0x%p (%llu)\n",
> + inode, inode->i_ino);
> +
> + if (S_ISLNK(inode->i_mode)) {
> + /*
> + * TODO: refer ocfs2_get_block() to handle
> + * ocfs2_read_folio in the future
> + */
> + mlog(ML_NOTICE, "map_block on S_ISLNK file, node 0x%p (%llu)\n",
> + inode, inode->i_ino);
> + dump_stack();
Here mlog and dump_stack() for debug?
Why not gate behind ML_DEBUG and WARN_ON_ONCE()?
> + goto bail;
> + }
> +
> + err = ocfs2_extent_map_get_blocks(inode, map->lblk, &p_blkno, &count,
> + &ext_flags);
> + if (err) {
> + mlog(ML_ERROR, "get_blocks() failed, inode: 0x%p, "
> + "block: %llu\n", inode, map->lblk);
> + goto bail;
> + }
> +
> + if (max_blocks < count)
> + count = max_blocks;
> +
> + map->pblk = p_blkno;
> + map->len = count;
> +
> + /*
> + * ocfs2 never allocates in this function - the only time we
> + * need to use MAP_NEW is when we're extending i_size on a file
> + * system which doesn't support holes, in which case MAP_NEW
> + * allows __block_write_begin() to zero.
> + *
> + * If we see this on a sparse file system, then a truncate has
> + * raced us and removed the cluster. In this case, we clear
> + * the buffers dirty and uptodate bits and let the buffer code
> + * ignore it as a hole.
> + */
> + if (create && map->pblk == 0 && ocfs2_sparse_alloc(osb)) {
> + map->flags &= ~(OCFS2_MAP_DIRTY | OCFS2_MAP_UPTODATE);
> + goto bail;
> + }
> +
> + if (p_blkno) {
> + if (ext_flags & OCFS2_EXT_UNWRITTEN) {
> + map->flags |= OCFS2_MAP_UNWRITTEN;
> + } else if (!(ext_flags & OCFS2_EXT_UNWRITTEN)) {
> + /* Treat the unwritten extent as a hole for zeroing purposes. */
> + map->flags |= OCFS2_MAP_MAPPED;
> + } else {
> + /* nothing to do */
> + }
> + }
It looks odd here. Can simplify to:
if (ext_flags & OCFS2_EXT_UNWRITTEN)
map->flags |= OCFS2_MAP_UNWRITTEN;
else
map->flags |= OCFS2_MAP_MAPPED;
> +
> + if (!ocfs2_sparse_alloc(osb)) {
> + if (map->pblk == 0) {
> + err = -EIO;
> + mlog(ML_ERROR,
> + "iblock = %llu p_blkno = %llu blkno=(%llu)\n",
> + (unsigned long long)map->lblk,
> + (unsigned long long)map->pblk,
> + (unsigned long long)OCFS2_I(inode)->ip_blkno);
> + mlog(ML_ERROR, "Size %llu, clusters %u\n",
> + (unsigned long long)i_size_read(inode),
> + OCFS2_I(inode)->ip_clusters);
> + dump_stack();
> + goto bail;
> + }
> + }
> +
> + past_eof = ocfs2_blocks_for_bytes(inode->i_sb, i_size_read(inode));
> +
> + if (create && (map->lblk >= past_eof))
> + map->flags |= OCFS2_MAP_NEW;
> +
> +bail:
> + if (err < 0)
> + return -EIO;
Why swallow the error code?
I'd rather propagate the actual error.
> + else
> + return map->len;
> +}
> +
> int ocfs2_get_block(struct inode *inode, sector_t iblock,
> struct buffer_head *bh_result, int create)
> {
> diff --git a/fs/ocfs2/aops.h b/fs/ocfs2/aops.h
> index 114efc9111e4..8dd6edd7c1a1 100644
> --- a/fs/ocfs2/aops.h
> +++ b/fs/ocfs2/aops.h
> @@ -42,6 +42,8 @@ int ocfs2_size_fits_inline_data(struct buffer_head *di_bh, u64 new_size);
>
> int ocfs2_get_block(struct inode *inode, sector_t iblock,
> struct buffer_head *bh_result, int create);
> +int ocfs2_map_blocks(struct inode *inode, struct ocfs2_map_block *map,
> + int flags);
> /* all ocfs2_dio_end_io()'s fault */
> #define ocfs2_iocb_is_rw_locked(iocb) \
> test_bit(0, (unsigned long *)&iocb->private)
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index 7bfe377af2df..493f2209cca5 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -23,18 +23,6 @@
> #include "buffer_head_io.h"
> #include "ocfs2_trace.h"
>
> -/*
> - * Bits on bh->b_state used by ocfs2.
> - *
> - * These MUST be after the JBD2 bits. Hence, we use BH_JBDPrivateStart.
> - */
> -enum ocfs2_state_bits {
> - BH_NeedsValidate = BH_JBDPrivateStart,
> -};
> -
> -/* Expand the magic b_state functions */
> -BUFFER_FNS(NeedsValidate, needs_validate);
> -
> int ocfs2_write_block(struct ocfs2_super *osb, struct buffer_head *bh,
> struct ocfs2_caching_info *ci)
> {
> diff --git a/fs/ocfs2/ocfs2.h b/fs/ocfs2/ocfs2.h
> index 62cad6522c7a..095f7ae5dded 100644
> --- a/fs/ocfs2/ocfs2.h
> +++ b/fs/ocfs2/ocfs2.h
> @@ -509,7 +509,50 @@ struct ocfs2_super
> struct ocfs2_filecheck_sysfs_entry osb_fc_ent;
> };
>
> -#define OCFS2_SB(sb) ((struct ocfs2_super *)(sb)->s_fs_info)
> +/*
> + * Bits on bh->b_state used by ocfs2.
> + *
> + * These MUST be after the JBD2 bits. Hence, we use BH_JBDPrivateStart.
> + */
> +enum ocfs2_state_bits {
> + BH_NeedsValidate = BH_JBDPrivateStart,
> +};
> +
> +/* Expand the magic b_state functions */
> +BUFFER_FNS(NeedsValidate, needs_validate);
> +
> +/*
> + * Logical to physical block mapping, used by ocfs2_map_blocks()
> + *
> + * This structure is used to pass requests into ocfs2_map_blocks() as
> + * well as to store the information returned by ocfs2_map_blocks(). It
> + * takes less room on the stack than a struct buffer_head.
> + */
> +#define OCFS2_MAP_NEW BIT(BH_New)
> +#define OCFS2_MAP_MAPPED BIT(BH_Mapped)
> +#define OCFS2_MAP_UNWRITTEN BIT(BH_Unwritten)
> +/* useless? #define OCFS2_MAP_BOUNDARY BIT(BH_Boundary) */
> +/* useless? #define OCFS2_MAP_DELAYED BIT(BH_Delay) */
> +#define OCFS2_MAP_DIRTY BIT(BH_Dirty)
> +#define OCFS2_MAP_UPTODATE BIT(BH_Uptodate)
> +#define OCFS2_MAP_NEEDS_VALIDATE BIT(BH_NeedsValidate)
> +#define OCFS2_MAP_DEFER_COMPLETION BIT(BH_Defer_Completion)
> +#define OCFS2_MAP_FLAGS (OCFS2_MAP_NEW | OCFS2_MAP_MAPPED |\
> + OCFS2_MAP_DIRTY | OCFS2_MAP_UPTODATE |\
> + OCFS2_MAP_NEEDS_VALIDATE |\
> + OCFS2_MAP_DEFER_COMPLETION)
> +
> +struct ocfs2_map_block {
> + u64 pblk; /* physical block# */
> + u64 lblk; /* logical block# */
> + u64 len; /* number of block */
> + unsigned int flags;
> +};
> +
> +/* Flags used by ocfs2_map_blocks() */
> +#define OCFS2_GET_BLOCKS_CREATE (0x0001)
> +
> +#define OCFS2_SB(sb) ((struct ocfs2_super *)(sb)->s_fs_info)
>
> /* Useful typedef for passing around journal access functions */
> typedef int (*ocfs2_journal_access_func)(handle_t *handle,
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC PATCH v2 2/4] ocfs2: switch dio read path from buffer_head to iomap
2026-07-27 6:17 [RFC PATCH v2 0/4] migrates ocfs2 DIO from buffer_head to iomap Heming Zhao
2026-07-27 6:17 ` [RFC PATCH v2 1/4] ocfs2: Add new ocfs2_map_blocks() to introduce iomap feature Heming Zhao
@ 2026-07-27 6:17 ` Heming Zhao
2026-07-28 4:16 ` Christoph Hellwig
2026-07-28 5:50 ` Joseph Qi
2026-07-27 6:17 ` [RFC PATCH v2 3/4] ocfs2: switch dio write " Heming Zhao
` (2 subsequent siblings)
4 siblings, 2 replies; 14+ messages in thread
From: Heming Zhao @ 2026-07-27 6:17 UTC (permalink / raw)
To: joseph.qi, mark, jlbec, hch; +Cc: Heming Zhao, ocfs2-devel, linux-kernel
This patch migrates the DIO read path from the legacy buffer_head
infrastructure to the modern and more efficient iomap framework.
The rw cluster-lock level is not stashed in iocb->private: iomap DIO owns
that field (it stores a bio there for polled I/O and unconditionally
clears it before calling ->end_io), so any side-channel packed into
iocb->private is corrupted and would trip the end_io lock-state check.
Instead the unlock level is encoded by which iomap_dio_ops is passed -
ocfs2_iomap_dio_ops_pr (PRMODE) or ocfs2_iomap_dio_ops_ex (EXMODE) - and
the *_iter() functions use the split __iomap_dio_rw() + iomap_dio_complete()
API so they can determine unambiguously, from the return value, whether the
completion handler already dropped the lock (real I/O, sync or -EIOCBQUEUED)
or whether the caller must drop it (no I/O issued, or buffered fallback).
Co-developed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Signed-off-by: Heming Zhao <heming.zhao@suse.com>
---
fs/ocfs2/Kconfig | 1 +
fs/ocfs2/aops.c | 113 ++++++++++++++++++++++++++++++++++++++++++++
fs/ocfs2/file.c | 57 ++++++++++++++++++----
fs/ocfs2/ocfs2.h | 3 ++
fs/ocfs2/ocfs2_fs.h | 3 ++
5 files changed, 167 insertions(+), 10 deletions(-)
diff --git a/fs/ocfs2/Kconfig b/fs/ocfs2/Kconfig
index 2514d36cbe01..bf1678a5eb01 100644
--- a/fs/ocfs2/Kconfig
+++ b/fs/ocfs2/Kconfig
@@ -7,6 +7,7 @@ config OCFS2_FS
select CRC32
select QUOTA
select QUOTA_TREE
+ select FS_IOMAP
select FS_POSIX_ACL
select LEGACY_DIRECT_IO
help
diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
index 08df5e3b5196..12f5f2e3530a 100644
--- a/fs/ocfs2/aops.c
+++ b/fs/ocfs2/aops.c
@@ -4,6 +4,7 @@
*/
#include <linux/fs.h>
+#include <linux/iomap.h>
#include <linux/slab.h>
#include <linux/highmem.h>
#include <linux/pagemap.h>
@@ -2556,6 +2557,118 @@ static ssize_t ocfs2_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
ocfs2_dio_end_io, 0);
}
+static int ocfs2_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
+ unsigned int flags, struct iomap *iomap, struct iomap *srcmap)
+{
+ int ret;
+ struct ocfs2_map_block map;
+ struct ocfs2_inode_info *oi = OCFS2_I(inode);
+ u8 blkbits = inode->i_blkbits;
+
+ if ((offset >> blkbits) > OCFS2_MAX_LOGICAL_BLOCK)
+ return -EINVAL;
+
+ /*
+ * Calculate the first and last logical blocks respectively.
+ */
+ map.lblk = offset >> blkbits;
+ map.len = min_t(loff_t, (offset + length - 1) >> blkbits,
+ OCFS2_MAX_LOGICAL_BLOCK) - map.lblk + 1;
+ map.flags = 0;
+
+ if (flags & IOMAP_WRITE) {
+ /* todo */
+ } else {
+ down_read(&oi->ip_alloc_sem);
+ ret = ocfs2_map_blocks(inode, &map, 0);
+ up_read(&oi->ip_alloc_sem);
+ }
+
+ if (ret < 0)
+ return ret;
+
+ /*
+ * Before returning to iomap, let's ensure the allocated mapping
+ * covers the entire requested length for atomic writes.
+ */
+ if (flags & IOMAP_ATOMIC) {
+ if (map.len < (length >> blkbits)) {
+ WARN_ON_ONCE(1);
+ return -EINVAL;
+ }
+ }
+
+ iomap->bdev = inode->i_sb->s_bdev;
+ iomap->offset = (u64)map.lblk << blkbits;
+ iomap->length = (u64)map.len << blkbits;
+
+ if (map.pblk == 0) {
+ iomap->type = IOMAP_HOLE;
+ iomap->addr = IOMAP_NULL_ADDR;
+ } else {
+ if (map.flags & OCFS2_MAP_UNWRITTEN) {
+ iomap->type = IOMAP_UNWRITTEN;
+ } else if (map.flags & OCFS2_MAP_MAPPED) {
+ iomap->type = IOMAP_MAPPED;
+ } else {
+ WARN_ON_ONCE(1);
+ return -EIO;
+ }
+ iomap->addr = (sector_t)map.pblk << blkbits;
+ }
+
+ if (map.flags & OCFS2_MAP_NEW)
+ iomap->flags |= IOMAP_F_NEW;
+
+ return 0;
+}
+
+const struct iomap_ops ocfs2_iomap_ops = {
+ .iomap_begin = ocfs2_iomap_begin,
+};
+
+/*
+ * Direct I/O completion. Like the old ocfs2_dio_end_io(), we use the rw_lock
+ * DLM lock to protect io on one node from truncation on another, and drop it
+ * here once the io has completed. iomap_dio_complete() always calls this,
+ * including on the buffered-fallback (-ENOTBLK) path, so the rw_lock taken in
+ * ocfs2_file_{read,write}_iter() is released exactly once.
+ */
+static int ocfs2_iomap_dio_end_io(struct kiocb *iocb, ssize_t size, int error,
+ int level)
+{
+ struct inode *inode = file_inode(iocb->ki_filp);
+
+ /*
+ * iomap_dio_complete() calls us even when no I/O was issued because the
+ * mapping bounced us back to buffered I/O (reported as size == 0 and
+ * error == 0). In that case leave the rw_lock held so
+ * ocfs2_file_{read,write}_iter() keeps it for the buffered retry and
+ * releases it in its own cleanup path.
+ */
+ if (size == 0 && error == 0)
+ return 0;
+
+ ocfs2_rw_unlock(inode, level);
+ return error;
+}
+
+static int ocfs2_dio_end_io_r_pr(struct kiocb *iocb, ssize_t size,
+ int error, unsigned int flags)
+{
+ if (error)
+ mlog_ratelimited(ML_ERROR, "Direct IO failed, bytes = %lld errno:%d",
+ (long long)size, error);
+
+ error = ocfs2_iomap_dio_end_io(iocb, size, error, 0);
+
+bail:
+ return error;
+}
+
+const struct iomap_dio_ops ocfs2_iomap_dio_ops_r_pr = {
+ .end_io = ocfs2_dio_end_io_r_pr,
+};
const struct address_space_operations ocfs2_aops = {
.dirty_folio = block_dirty_folio,
.read_folio = ocfs2_read_folio,
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index d6e977ba6565..a0b3883216bc 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -9,6 +9,7 @@
#include <linux/capability.h>
#include <linux/fs.h>
+#include <linux/iomap.h>
#include <linux/types.h>
#include <linux/slab.h>
#include <linux/highmem.h>
@@ -2374,6 +2375,19 @@ static int ocfs2_prepare_inode_for_write(struct file *file,
return ret;
}
+static bool ocfs2_should_use_dio(struct kiocb *iocb, struct iov_iter *iter,
+ struct inode *inode)
+{
+ /*
+ * Fallback to buffered I/O if we see an inode without
+ * extents.
+ */
+ if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL)
+ return false;
+
+ return true;
+}
+
static ssize_t ocfs2_file_write_iter(struct kiocb *iocb,
struct iov_iter *from)
{
@@ -2548,7 +2562,6 @@ static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
filp->f_path.dentry->d_name.name,
to->nr_segs); /* GRRRRR */
-
if (!inode) {
ret = -EINVAL;
mlog_errno(ret);
@@ -2558,7 +2571,8 @@ static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
if (!direct_io && nowait)
return -EOPNOTSUPP;
- ocfs2_iocb_init_rw_locked(iocb);
+ if (!iov_iter_count(to))
+ return 0; /* skip atime */
/*
* buffered reads protect themselves in ->read_folio(). O_DIRECT reads
@@ -2576,8 +2590,6 @@ static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
goto bail;
}
rw_level = 0;
- /* communicate with ocfs2_dio_end_io */
- ocfs2_iocb_set_rw_locked(iocb, rw_level);
}
/*
@@ -2598,17 +2610,42 @@ static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
}
ocfs2_inode_unlock(inode, lock_level);
- ret = generic_file_read_iter(iocb, to);
+ if (direct_io && ocfs2_should_use_dio(iocb, to, inode)) {
+ struct iomap_dio *dio;
+
+ dio = __iomap_dio_rw(iocb, to, &ocfs2_iomap_ops,
+ &ocfs2_iomap_dio_ops_r_pr, 0, NULL, 0);
+ if (dio == NULL) {
+ /* No I/O issued; rw_lock still held. */
+ ret = 0;
+ } else if (IS_ERR(dio)) {
+ ret = PTR_ERR(dio);
+ if (ret == -EIOCBQUEUED)
+ rw_level = -1;
+ } else {
+ ret = iomap_dio_complete(dio);
+ if (ret != 0)
+ rw_level = -1;
+ }
+ /*
+ * A 0 result means the mapping bounced us back to buffered I/O
+ * (e.g. inline data); the rw_lock is still held. Clear
+ * IOCB_DIRECT so generic_file_read_iter() takes the buffered
+ * path rather than re-entering direct I/O.
+ */
+ if (ret == 0) {
+ iocb->ki_flags &= ~IOCB_DIRECT;
+ ret = generic_file_read_iter(iocb, to);
+ }
+ } else {
+ iocb->ki_flags &= ~IOCB_DIRECT;
+ ret = generic_file_read_iter(iocb, to);
+ }
trace_generic_file_read_iter_ret(ret);
/* buffered aio wouldn't have proper lock coverage today */
BUG_ON(ret == -EIOCBQUEUED && !direct_io);
- /* see ocfs2_file_write_iter */
- if (ret == -EIOCBQUEUED || !ocfs2_iocb_is_rw_locked(iocb)) {
- rw_level = -1;
- }
-
bail:
if (rw_level != -1)
ocfs2_rw_unlock(inode, rw_level);
diff --git a/fs/ocfs2/ocfs2.h b/fs/ocfs2/ocfs2.h
index 095f7ae5dded..a775c1869293 100644
--- a/fs/ocfs2/ocfs2.h
+++ b/fs/ocfs2/ocfs2.h
@@ -549,6 +549,9 @@ struct ocfs2_map_block {
unsigned int flags;
};
+extern const struct iomap_ops ocfs2_iomap_ops;
+extern const struct iomap_dio_ops ocfs2_iomap_dio_ops_r_pr;
+
/* Flags used by ocfs2_map_blocks() */
#define OCFS2_GET_BLOCKS_CREATE (0x0001)
diff --git a/fs/ocfs2/ocfs2_fs.h b/fs/ocfs2/ocfs2_fs.h
index c501eb3cdcda..000bd014acbd 100644
--- a/fs/ocfs2/ocfs2_fs.h
+++ b/fs/ocfs2/ocfs2_fs.h
@@ -314,6 +314,9 @@
*/
#define OCFS2_CLUSTER_O2CB_GLOBAL_HEARTBEAT (0x01)
+/* Max logical block we can support */
+#define OCFS2_MAX_LOGICAL_BLOCK (0xFFFFFFFE)
+
struct ocfs2_system_inode_info {
char *si_name;
int si_iflags;
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [RFC PATCH v2 2/4] ocfs2: switch dio read path from buffer_head to iomap
2026-07-27 6:17 ` [RFC PATCH v2 2/4] ocfs2: switch dio read path from buffer_head to iomap Heming Zhao
@ 2026-07-28 4:16 ` Christoph Hellwig
2026-07-28 5:50 ` Joseph Qi
1 sibling, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2026-07-28 4:16 UTC (permalink / raw)
To: Heming Zhao; +Cc: joseph.qi, mark, jlbec, hch, ocfs2-devel, linux-kernel
On Mon, Jul 27, 2026 at 02:17:58PM +0800, Heming Zhao wrote:
> This patch migrates the DIO read path from the legacy buffer_head
Please never start commit messages with "This patch", that context
is implied already.
> The rw cluster-lock level is not stashed in iocb->private: iomap DIO owns
Explaining what you don't do is always a bit odd, I'd start with what
you do, and only really mention what not later if really needed.
> that field (it stores a bio there for polled I/O and unconditionally
> clears it before calling ->end_io), so any side-channel packed into
> iocb->private is corrupted and would trip the end_io lock-state check.
> Instead the unlock level is encoded by which iomap_dio_ops is passed -
> ocfs2_iomap_dio_ops_pr (PRMODE) or ocfs2_iomap_dio_ops_ex (EXMODE) - and
> the *_iter() functions use the split __iomap_dio_rw() + iomap_dio_complete()
> API so they can determine unambiguously, from the return value, whether the
> completion handler already dropped the lock (real I/O, sync or -EIOCBQUEUED)
> or whether the caller must drop it (no I/O issued, or buffered fallback).
.. and a lot of this just seems to be about the locking, but very little
about the iomap changes.
> + if (flags & IOMAP_WRITE) {
> + /* todo */
should this be a WARN_ON for now?
> + if (direct_io && ocfs2_should_use_dio(iocb, to, inode)) {
> + struct iomap_dio *dio;
> +
> + dio = __iomap_dio_rw(iocb, to, &ocfs2_iomap_ops,
> + &ocfs2_iomap_dio_ops_r_pr, 0, NULL, 0);
> + if (dio == NULL) {
> + /* No I/O issued; rw_lock still held. */
> + ret = 0;
> + } else if (IS_ERR(dio)) {
> + ret = PTR_ERR(dio);
> + if (ret == -EIOCBQUEUED)
> + rw_level = -1;
> + } else {
> + ret = iomap_dio_complete(dio);
> + if (ret != 0)
> + rw_level = -1;
> + }
What information is missing if you use plain iomap_dio_rw here?
> + /*
> + * A 0 result means the mapping bounced us back to buffered I/O
> + * (e.g. inline data); the rw_lock is still held. Clear
> + * IOCB_DIRECT so generic_file_read_iter() takes the buffered
> + * path rather than re-entering direct I/O.
> + */
> + if (ret == 0) {
> + iocb->ki_flags &= ~IOCB_DIRECT;
> + ret = generic_file_read_iter(iocb, to);
> + }
> + } else {
> + iocb->ki_flags &= ~IOCB_DIRECT;
> + ret = generic_file_read_iter(iocb, to);
> + }
Can you share this code somehow?
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC PATCH v2 2/4] ocfs2: switch dio read path from buffer_head to iomap
2026-07-27 6:17 ` [RFC PATCH v2 2/4] ocfs2: switch dio read path from buffer_head to iomap Heming Zhao
2026-07-28 4:16 ` Christoph Hellwig
@ 2026-07-28 5:50 ` Joseph Qi
1 sibling, 0 replies; 14+ messages in thread
From: Joseph Qi @ 2026-07-28 5:50 UTC (permalink / raw)
To: Heming Zhao; +Cc: mark, jlbec, hch, ocfs2-devel, linux-kernel
On 7/27/26 2:17 PM, Heming Zhao wrote:
> This patch migrates the DIO read path from the legacy buffer_head
> infrastructure to the modern and more efficient iomap framework.
>
> The rw cluster-lock level is not stashed in iocb->private: iomap DIO owns
> that field (it stores a bio there for polled I/O and unconditionally
> clears it before calling ->end_io), so any side-channel packed into
> iocb->private is corrupted and would trip the end_io lock-state check.
> Instead the unlock level is encoded by which iomap_dio_ops is passed -
> ocfs2_iomap_dio_ops_pr (PRMODE) or ocfs2_iomap_dio_ops_ex (EXMODE) - and
> the *_iter() functions use the split __iomap_dio_rw() + iomap_dio_complete()
> API so they can determine unambiguously, from the return value, whether the
> completion handler already dropped the lock (real I/O, sync or -EIOCBQUEUED)
> or whether the caller must drop it (no I/O issued, or buffered fallback).
>
> Co-developed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Signed-off-by: Heming Zhao <heming.zhao@suse.com>
> ---
> fs/ocfs2/Kconfig | 1 +
> fs/ocfs2/aops.c | 113 ++++++++++++++++++++++++++++++++++++++++++++
> fs/ocfs2/file.c | 57 ++++++++++++++++++----
> fs/ocfs2/ocfs2.h | 3 ++
> fs/ocfs2/ocfs2_fs.h | 3 ++
> 5 files changed, 167 insertions(+), 10 deletions(-)
>
> diff --git a/fs/ocfs2/Kconfig b/fs/ocfs2/Kconfig
> index 2514d36cbe01..bf1678a5eb01 100644
> --- a/fs/ocfs2/Kconfig
> +++ b/fs/ocfs2/Kconfig
> @@ -7,6 +7,7 @@ config OCFS2_FS
> select CRC32
> select QUOTA
> select QUOTA_TREE
> + select FS_IOMAP
> select FS_POSIX_ACL
> select LEGACY_DIRECT_IO
> help
> diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
> index 08df5e3b5196..12f5f2e3530a 100644
> --- a/fs/ocfs2/aops.c
> +++ b/fs/ocfs2/aops.c
> @@ -4,6 +4,7 @@
> */
>
> #include <linux/fs.h>
> +#include <linux/iomap.h>
> #include <linux/slab.h>
> #include <linux/highmem.h>
> #include <linux/pagemap.h>
> @@ -2556,6 +2557,118 @@ static ssize_t ocfs2_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
> ocfs2_dio_end_io, 0);
> }
>
> +static int ocfs2_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
> + unsigned int flags, struct iomap *iomap, struct iomap *srcmap)
> +{
> + int ret;
> + struct ocfs2_map_block map;
> + struct ocfs2_inode_info *oi = OCFS2_I(inode);
> + u8 blkbits = inode->i_blkbits;
> +
> + if ((offset >> blkbits) > OCFS2_MAX_LOGICAL_BLOCK)
> + return -EINVAL;
> +
> + /*
> + * Calculate the first and last logical blocks respectively.
> + */
> + map.lblk = offset >> blkbits;
> + map.len = min_t(loff_t, (offset + length - 1) >> blkbits,
> + OCFS2_MAX_LOGICAL_BLOCK) - map.lblk + 1;
> + map.flags = 0;
> +
> + if (flags & IOMAP_WRITE) {
> + /* todo */
This makes the series non-bisectable since ret is uninitialized.
So return -EOPNOTSUPP instead.
BTW, 'TODO' is preferred.
> + } else {
> + down_read(&oi->ip_alloc_sem);
> + ret = ocfs2_map_blocks(inode, &map, 0);
> + up_read(&oi->ip_alloc_sem);
> + }
> +
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * Before returning to iomap, let's ensure the allocated mapping
> + * covers the entire requested length for atomic writes.
> + */
> + if (flags & IOMAP_ATOMIC) {
> + if (map.len < (length >> blkbits)) {
> + WARN_ON_ONCE(1);
> + return -EINVAL;
> + }
> + }
> +
> + iomap->bdev = inode->i_sb->s_bdev;
> + iomap->offset = (u64)map.lblk << blkbits;
> + iomap->length = (u64)map.len << blkbits;
> +
> + if (map.pblk == 0) {
> + iomap->type = IOMAP_HOLE;
> + iomap->addr = IOMAP_NULL_ADDR;
> + } else {
> + if (map.flags & OCFS2_MAP_UNWRITTEN) {
> + iomap->type = IOMAP_UNWRITTEN;
> + } else if (map.flags & OCFS2_MAP_MAPPED) {
> + iomap->type = IOMAP_MAPPED;
> + } else {
> + WARN_ON_ONCE(1);
> + return -EIO;
> + }
> + iomap->addr = (sector_t)map.pblk << blkbits;
> + }
> +
> + if (map.flags & OCFS2_MAP_NEW)
> + iomap->flags |= IOMAP_F_NEW;
> +
> + return 0;
> +}
> +
> +const struct iomap_ops ocfs2_iomap_ops = {
> + .iomap_begin = ocfs2_iomap_begin,
> +};
> +
> +/*
> + * Direct I/O completion. Like the old ocfs2_dio_end_io(), we use the rw_lock
> + * DLM lock to protect io on one node from truncation on another, and drop it
> + * here once the io has completed. iomap_dio_complete() always calls this,
> + * including on the buffered-fallback (-ENOTBLK) path, so the rw_lock taken in
> + * ocfs2_file_{read,write}_iter() is released exactly once.
> + */
> +static int ocfs2_iomap_dio_end_io(struct kiocb *iocb, ssize_t size, int error,
> + int level)
> +{
> + struct inode *inode = file_inode(iocb->ki_filp);
> +
> + /*
> + * iomap_dio_complete() calls us even when no I/O was issued because the
This looks incorrect.
If __iomap_dio_rw() returns NULL, iomap_dio_complete() is never called.
> + * mapping bounced us back to buffered I/O (reported as size == 0 and
> + * error == 0). In that case leave the rw_lock held so
> + * ocfs2_file_{read,write}_iter() keeps it for the buffered retry and
> + * releases it in its own cleanup path.
> + */
> + if (size == 0 && error == 0)
> + return 0;
> +
> + ocfs2_rw_unlock(inode, level);
> + return error;
> +}
> +
> +static int ocfs2_dio_end_io_r_pr(struct kiocb *iocb, ssize_t size,
> + int error, unsigned int flags)
> +{
> + if (error)
> + mlog_ratelimited(ML_ERROR, "Direct IO failed, bytes = %lld errno:%d",
> + (long long)size, error);
> +
> + error = ocfs2_iomap_dio_end_io(iocb, size, error, 0);
> +
> +bail:
Seems it is a dead label.
> + return error;
> +}
> +
> +const struct iomap_dio_ops ocfs2_iomap_dio_ops_r_pr = {
> + .end_io = ocfs2_dio_end_io_r_pr,
> +};
> const struct address_space_operations ocfs2_aops = {
> .dirty_folio = block_dirty_folio,
> .read_folio = ocfs2_read_folio,
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index d6e977ba6565..a0b3883216bc 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -9,6 +9,7 @@
>
> #include <linux/capability.h>
> #include <linux/fs.h>
> +#include <linux/iomap.h>
> #include <linux/types.h>
> #include <linux/slab.h>
> #include <linux/highmem.h>
> @@ -2374,6 +2375,19 @@ static int ocfs2_prepare_inode_for_write(struct file *file,
> return ret;
> }
>
> +static bool ocfs2_should_use_dio(struct kiocb *iocb, struct iov_iter *iter,
> + struct inode *inode)
> +{
> + /*
> + * Fallback to buffered I/O if we see an inode without
> + * extents.
> + */
> + if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL)
> + return false;
> +
> + return true;
> +}
> +
> static ssize_t ocfs2_file_write_iter(struct kiocb *iocb,
> struct iov_iter *from)
> {
> @@ -2548,7 +2562,6 @@ static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
> filp->f_path.dentry->d_name.name,
> to->nr_segs); /* GRRRRR */
>
> -
> if (!inode) {
> ret = -EINVAL;
> mlog_errno(ret);
> @@ -2558,7 +2571,8 @@ static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
> if (!direct_io && nowait)
> return -EOPNOTSUPP;
>
> - ocfs2_iocb_init_rw_locked(iocb);
> + if (!iov_iter_count(to))
> + return 0; /* skip atime */
>
> /*
> * buffered reads protect themselves in ->read_folio(). O_DIRECT reads
> @@ -2576,8 +2590,6 @@ static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
> goto bail;
> }
> rw_level = 0;
> - /* communicate with ocfs2_dio_end_io */
> - ocfs2_iocb_set_rw_locked(iocb, rw_level);
> }
>
> /*
> @@ -2598,17 +2610,42 @@ static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
> }
> ocfs2_inode_unlock(inode, lock_level);
>
> - ret = generic_file_read_iter(iocb, to);
> + if (direct_io && ocfs2_should_use_dio(iocb, to, inode)) {
> + struct iomap_dio *dio;
> +
> + dio = __iomap_dio_rw(iocb, to, &ocfs2_iomap_ops,
> + &ocfs2_iomap_dio_ops_r_pr, 0, NULL, 0);
> + if (dio == NULL) {
> + /* No I/O issued; rw_lock still held. */
> + ret = 0;
> + } else if (IS_ERR(dio)) {
> + ret = PTR_ERR(dio);
> + if (ret == -EIOCBQUEUED)
> + rw_level = -1;
> + } else {
> + ret = iomap_dio_complete(dio);
> + if (ret != 0)
> + rw_level = -1;
> + }
> + /*
> + * A 0 result means the mapping bounced us back to buffered I/O
> + * (e.g. inline data); the rw_lock is still held. Clear
> + * IOCB_DIRECT so generic_file_read_iter() takes the buffered
> + * path rather than re-entering direct I/O.
> + */
> + if (ret == 0) {
> + iocb->ki_flags &= ~IOCB_DIRECT;
> + ret = generic_file_read_iter(iocb, to);
> + }
> + } else {
> + iocb->ki_flags &= ~IOCB_DIRECT;
> + ret = generic_file_read_iter(iocb, to);
> + }
> trace_generic_file_read_iter_ret(ret);
>
> /* buffered aio wouldn't have proper lock coverage today */
> BUG_ON(ret == -EIOCBQUEUED && !direct_io);
>
> - /* see ocfs2_file_write_iter */
> - if (ret == -EIOCBQUEUED || !ocfs2_iocb_is_rw_locked(iocb)) {
> - rw_level = -1;
> - }
> -
> bail:
> if (rw_level != -1)
> ocfs2_rw_unlock(inode, rw_level);
> diff --git a/fs/ocfs2/ocfs2.h b/fs/ocfs2/ocfs2.h
> index 095f7ae5dded..a775c1869293 100644
> --- a/fs/ocfs2/ocfs2.h
> +++ b/fs/ocfs2/ocfs2.h
> @@ -549,6 +549,9 @@ struct ocfs2_map_block {
> unsigned int flags;
> };
>
> +extern const struct iomap_ops ocfs2_iomap_ops;
> +extern const struct iomap_dio_ops ocfs2_iomap_dio_ops_r_pr;
> +
> /* Flags used by ocfs2_map_blocks() */
> #define OCFS2_GET_BLOCKS_CREATE (0x0001)
>
> diff --git a/fs/ocfs2/ocfs2_fs.h b/fs/ocfs2/ocfs2_fs.h
> index c501eb3cdcda..000bd014acbd 100644
> --- a/fs/ocfs2/ocfs2_fs.h
> +++ b/fs/ocfs2/ocfs2_fs.h
> @@ -314,6 +314,9 @@
> */
> #define OCFS2_CLUSTER_O2CB_GLOBAL_HEARTBEAT (0x01)
>
> +/* Max logical block we can support */
> +#define OCFS2_MAX_LOGICAL_BLOCK (0xFFFFFFFE)
> +
> struct ocfs2_system_inode_info {
> char *si_name;
> int si_iflags;
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC PATCH v2 3/4] ocfs2: switch dio write path from buffer_head to iomap
2026-07-27 6:17 [RFC PATCH v2 0/4] migrates ocfs2 DIO from buffer_head to iomap Heming Zhao
2026-07-27 6:17 ` [RFC PATCH v2 1/4] ocfs2: Add new ocfs2_map_blocks() to introduce iomap feature Heming Zhao
2026-07-27 6:17 ` [RFC PATCH v2 2/4] ocfs2: switch dio read path from buffer_head to iomap Heming Zhao
@ 2026-07-27 6:17 ` Heming Zhao
2026-07-28 4:18 ` Christoph Hellwig
2026-07-28 6:07 ` Joseph Qi
2026-07-27 6:18 ` [RFC PATCH v2 4/4] ocfs2: remove legacy blockdev direct-IO path APIs Heming Zhao
2026-07-28 4:11 ` [RFC PATCH v2 0/4] migrates ocfs2 DIO from buffer_head to iomap Christoph Hellwig
4 siblings, 2 replies; 14+ messages in thread
From: Heming Zhao @ 2026-07-27 6:17 UTC (permalink / raw)
To: joseph.qi, mark, jlbec, hch; +Cc: Heming Zhao, ocfs2-devel, linux-kernel
This patch converts OCFS2's DIO write path from the legacy
buffer_head infrastructure to the modern iomap framework.
Key modifications and designs are as follows:
1. Dynamic Context Allocation:
Refactor 'struct ocfs2_write_ctxt' to use a flexible array 'w_desc[]'
instead of a fixed-size array. Dynamically allocate the context based on
the write length ('w_clen') in 'ocfs2_alloc_write_ctxt()'. This prevents
static limits overflow and optimizes kernel heap memory utilization.
2. Robust Mapping and Limits:
Introduce 'ocfs2_dio_wr_map_blocks()' to allocate and map direct write
blocks. Implement a 1 MiB cap ('OCFS2_DIO_WR_MAX_MAX_BYTES') per mapping
call to restrict allocation granularity, preventing JBD2 transaction
credit exhaustion during huge asynchronous sequential writes.
3. Reliable Completion Work and Fallback:
- Implement 'ocfs2_iomap_dio_end_io_write()' to handle metadata completion.
It converts UNWRITTEN extents, updates inode size (EOF), and deletes the
inode from the orphan directory if it was appended.
- Implement 'ocfs2_dio_write_end_io()' to finalize the dio lifecycle and
release cluster locks safely.
- Intercept '-ENOTBLK' errors from 'iomap_dio_rw()' caused by page cache
invalidation failures (due to mmap/buffered collisions). Gracefully clear
the error, strip the IOCB_DIRECT flag, and fall back to buffered write
4. Moved ocfs2_add_inode_to_orphan():
- moved ocfs2_add_inode_to_orphan() from ocfs2_dio_wr_map_blocks() to
ocfs2_file_write_iter().
5. Uncertain logic in code
For the following code block in ocfs2_dio_wr_map_blocks():
```
if (extend) {
if (ocfs2_sparse_alloc(osb))
ret = ocfs2_zero_tail(inode, di_bh, pos);
else
ret = ocfs2_expand_nonsparse_inode(inode, di_bh, pos,
map_len, NULL);
if (ret < 0) {
mlog_errno(ret);
goto unlock;
}
}
```
I am not completely certain whether it is called only once per
ocfs2_file_write_iter(), but I believe calling it multiple times will
not introduce any side effects. Furthermore, testing across various
scenarios showed no instances of multiple calls.
Assisted-by: Gemini:gemini-3.5-flash
Assisted-by: Claude:claude-sonnet-4-5
Co-developed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Signed-off-by: Heming Zhao <heming.zhao@suse.com>
---
fs/ocfs2/aops.c | 394 ++++++++++++++++++++++++++++++++++++--
fs/ocfs2/buffer_head_io.c | 7 +-
fs/ocfs2/file.c | 91 +++++++--
fs/ocfs2/ocfs2.h | 2 +
4 files changed, 459 insertions(+), 35 deletions(-)
diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
index 12f5f2e3530a..9a079436c9c0 100644
--- a/fs/ocfs2/aops.c
+++ b/fs/ocfs2/aops.c
@@ -764,6 +764,8 @@ int ocfs2_map_folio_blocks(struct folio *folio, u64 *p_blkno,
#define OCFS2_MAX_CTXT_PAGES (OCFS2_MAX_CLUSTERSIZE / PAGE_SIZE)
#endif
+#define OCFS2_DIO_WR_MAX_MAP_BYTES (1U << 20) /* 1 MiB */
+
#define OCFS2_MAX_CLUSTERS_PER_PAGE (PAGE_SIZE / OCFS2_MIN_CLUSTERSIZE)
struct ocfs2_unwritten_extent {
@@ -799,8 +801,6 @@ struct ocfs2_write_ctxt {
/* Type of caller. Must be one of buffer, mmap, direct. */
ocfs2_write_type_t w_type;
- struct ocfs2_write_cluster_desc w_desc[OCFS2_MAX_CLUSTERS_PER_PAGE];
-
/*
* This is true if page_size > cluster_size.
*
@@ -850,6 +850,8 @@ struct ocfs2_write_ctxt {
struct list_head w_unwritten_list;
unsigned int w_unwritten_count;
+
+ struct ocfs2_write_cluster_desc w_desc[];
};
void ocfs2_unlock_and_free_folios(struct folio **folios, int num_folios)
@@ -917,17 +919,24 @@ static int ocfs2_alloc_write_ctxt(struct ocfs2_write_ctxt **wcp,
unsigned len, ocfs2_write_type_t type,
struct buffer_head *di_bh)
{
- u32 cend;
+ u32 w_cpos, cend, w_clen;
struct ocfs2_write_ctxt *wc;
+ size_t alloc_size;
+
+ w_cpos = pos >> osb->s_clustersize_bits;
+ cend = (pos + len - 1) >> osb->s_clustersize_bits;
+ w_clen = cend - w_cpos + 1;
+
+ alloc_size = sizeof(struct ocfs2_write_ctxt) +
+ w_clen * sizeof(struct ocfs2_write_cluster_desc);
- wc = kzalloc_obj(struct ocfs2_write_ctxt, GFP_NOFS);
+ wc = kzalloc(alloc_size, GFP_NOFS);
if (!wc)
return -ENOMEM;
- wc->w_cpos = pos >> osb->s_clustersize_bits;
+ wc->w_cpos = w_cpos;
wc->w_first_new_cpos = UINT_MAX;
- cend = (pos + len - 1) >> osb->s_clustersize_bits;
- wc->w_clen = cend - wc->w_cpos + 1;
+ wc->w_clen = w_clen;
get_bh(di_bh);
wc->w_di_bh = di_bh;
wc->w_type = type;
@@ -2218,7 +2227,7 @@ static int ocfs2_dio_wr_get_block(struct inode *inode, sector_t iblock,
struct ocfs2_write_cluster_desc *desc = NULL;
struct ocfs2_dio_write_ctxt *dwc = NULL;
struct buffer_head *di_bh = NULL;
- u64 p_blkno;
+ u64 p_blkno = 0;
unsigned int i_blkbits = inode->i_sb->s_blocksize_bits;
loff_t pos = iblock << i_blkbits;
sector_t endblk = (i_size_read(inode) - 1) >> i_blkbits;
@@ -2358,6 +2367,148 @@ static int ocfs2_dio_wr_get_block(struct inode *inode, sector_t iblock,
return ret;
}
+/* copy from ocfs2_dio_wr_get_block */
+static int ocfs2_dio_wr_map_blocks(struct inode *inode,
+ struct ocfs2_map_block *map, int create)
+{
+ struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+ struct ocfs2_inode_info *oi = OCFS2_I(inode);
+ struct ocfs2_write_ctxt *wc;
+ struct ocfs2_write_cluster_desc *desc = NULL;
+ struct buffer_head *di_bh = NULL;
+ u64 p_blkno;
+ sector_t iblock = map->lblk;
+ unsigned int i_blkbits = inode->i_sb->s_blocksize_bits;
+ loff_t pos = iblock << i_blkbits;
+ sector_t endblk = (i_size_read(inode) - 1) >> i_blkbits;
+ unsigned int total_len = map->len << i_blkbits;
+ unsigned int map_len;
+ unsigned int cont_clusters = 1;
+ int ret = 0, extend = 0, idx;
+
+ /*
+ * map->len is count in ocfs2_iomap_begin according to write
+ * "pos" and "end", we need map twice to return different buffer state:
+ * 1. area in file size, not set NEW;
+ * 2. area out file size, set NEW.
+ *
+ * iblock endblk
+ * |--------|---------|---------|---------
+ * |<-------area in file------->|
+ */
+ if ((iblock <= endblk) &&
+ ((iblock + ((total_len - 1) >> i_blkbits)) > endblk))
+ total_len = (endblk - iblock + 1) << i_blkbits;
+
+ /*
+ * Bound how much we allocate/prepare (and journal) per call. map->len
+ * below still reports only the physically-contiguous prefix to iomap;
+ * iomap calls us again for the remainder.
+ */
+ map_len = min(total_len, OCFS2_DIO_WR_MAX_MAP_BYTES);
+
+ mlog(0, "get block of %llu at %llu:%u req %u\n",
+ inode->i_ino, pos, map_len, total_len);
+
+ /*
+ * Because we need to change file size in ocfs2_iomap_dio_end_io_write(),
+ * or we may need to add it to orphan dir. So can not fall to fast path
+ * while file size will be changed.
+ */
+ if (pos + total_len <= i_size_read(inode)) {
+ /* This is the fast path for re-write. */
+ ret = ocfs2_map_blocks(inode, map, OCFS2_GET_BLOCKS_CREATE);
+ if ((map->flags & OCFS2_MAP_MAPPED) &&
+ !(map->flags & OCFS2_MAP_NEW) && ret == 0)
+ goto out;
+
+ /* Clear state set by ocfs2_map_blocks. */
+ map->flags = 0;
+ } else {
+ extend = 1;
+ }
+
+ ret = ocfs2_inode_lock(inode, &di_bh, 1);
+ if (ret) {
+ mlog_errno(ret);
+ goto out;
+ }
+
+ down_write(&oi->ip_alloc_sem);
+
+ if (extend) {
+ if (ocfs2_sparse_alloc(osb))
+ ret = ocfs2_zero_tail(inode, di_bh, pos);
+ else
+ ret = ocfs2_expand_nonsparse_inode(inode, di_bh, pos,
+ map_len, NULL);
+ if (ret < 0) {
+ mlog_errno(ret);
+ goto unlock;
+ }
+ }
+
+ ret = ocfs2_write_begin_nolock(inode->i_mapping, pos, map_len,
+ OCFS2_WRITE_DIRECT, NULL,
+ (void **)&wc, di_bh, NULL);
+ if (ret) {
+ mlog_errno(ret);
+ goto unlock;
+ }
+
+ desc = &wc->w_desc[0];
+
+ p_blkno = ocfs2_clusters_to_blocks(inode->i_sb, desc->c_phys);
+ BUG_ON(p_blkno == 0);
+ p_blkno += iblock & (u64)(ocfs2_clusters_to_blocks(inode->i_sb, 1) - 1);
+
+ /*
+ * Calculate physically contiguous clusters by checking descriptor arrays.
+ * This strictly prevents map->len from expanding past non-contiguous physical bounds.
+ */
+ for (idx = 1; idx < wc->w_clen; idx++) {
+ if (wc->w_desc[idx].c_phys == wc->w_desc[idx - 1].c_phys + 1 &&
+ wc->w_desc[idx].c_needs_zero == wc->w_desc[idx - 1].c_needs_zero) {
+ cont_clusters++;
+ } else {
+ break;
+ }
+ }
+ map->pblk = p_blkno;
+ /*
+ * Only report the mapping as UNWRITTEN when the cluster is newly
+ * allocated or was already unwritten (desc->c_needs_zero). Reporting
+ * an already-written cluster as UNWRITTEN makes the iomap direct-io
+ * core zero the sub-block head/tail padding, which destroys existing
+ * data when a sub-block write lands inside a written cluster.
+ */
+ map->flags |= OCFS2_MAP_MAPPED;
+ if (desc->c_needs_zero)
+ map->flags |= OCFS2_MAP_UNWRITTEN;
+ map->len = cont_clusters * ocfs2_clusters_to_blocks(inode->i_sb, 1);
+
+ if (desc->c_needs_zero)
+ map->flags |= OCFS2_MAP_NEW;
+
+ if (iblock > endblk)
+ map->flags |= OCFS2_MAP_NEW;
+
+ map->flags |= OCFS2_MAP_DEFER_COMPLETION;
+
+ ocfs2_free_unwritten_list(inode, &wc->w_unwritten_list);
+ ret = ocfs2_write_end_nolock(inode->i_mapping, pos, map_len, map_len, wc);
+ BUG_ON(ret != map_len);
+ ret = 0;
+
+unlock:
+ up_write(&oi->ip_alloc_sem);
+ ocfs2_inode_unlock(inode, 1);
+ brelse(di_bh);
+
+out:
+ return ret;
+}
+
static int ocfs2_dio_end_io_write(struct inode *inode,
struct ocfs2_dio_write_ctxt *dwc,
loff_t offset,
@@ -2557,6 +2708,12 @@ static ssize_t ocfs2_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
ocfs2_dio_end_io, 0);
}
+static int ocfs2_iomap_alloc(struct inode *inode, struct ocfs2_map_block *map,
+ unsigned int flags)
+{
+ return ocfs2_dio_wr_map_blocks(inode, map, 1);
+}
+
static int ocfs2_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
unsigned int flags, struct iomap *iomap, struct iomap *srcmap)
{
@@ -2564,6 +2721,7 @@ static int ocfs2_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
struct ocfs2_map_block map;
struct ocfs2_inode_info *oi = OCFS2_I(inode);
u8 blkbits = inode->i_blkbits;
+ unsigned int orig_mlen;
if ((offset >> blkbits) > OCFS2_MAX_LOGICAL_BLOCK)
return -EINVAL;
@@ -2575,9 +2733,32 @@ static int ocfs2_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
map.len = min_t(loff_t, (offset + length - 1) >> blkbits,
OCFS2_MAX_LOGICAL_BLOCK) - map.lblk + 1;
map.flags = 0;
+ orig_mlen = map.len;
if (flags & IOMAP_WRITE) {
- /* todo */
+ /*
+ * We check here if the blocks are already allocated, then we
+ * don't need to start a journal txn and we can directly return
+ * the mapping information. This could boost performance
+ * especially in multi-threaded overwrite requests.
+ */
+ if (offset + length <= i_size_read(inode)) {
+ down_read(&oi->ip_alloc_sem);
+ ret = ocfs2_map_blocks(inode, &map, 0);
+ up_read(&oi->ip_alloc_sem);
+ /*
+ * For atomic writes the entire requested length should
+ * be mapped.
+ */
+ if (map.flags & OCFS2_MAP_MAPPED) {
+ if ((!(flags & IOMAP_ATOMIC) && ret > 0) ||
+ (flags & IOMAP_ATOMIC && ret >= orig_mlen)) {
+ goto out;
+ }
+ }
+ map.len = orig_mlen;
+ }
+ ret = ocfs2_iomap_alloc(inode, &map, flags);
} else {
down_read(&oi->ip_alloc_sem);
ret = ocfs2_map_blocks(inode, &map, 0);
@@ -2587,6 +2768,7 @@ static int ocfs2_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
if (ret < 0)
return ret;
+out:
/*
* Before returning to iomap, let's ensure the allocated mapping
* covers the entire requested length for atomic writes.
@@ -2653,8 +2835,8 @@ static int ocfs2_iomap_dio_end_io(struct kiocb *iocb, ssize_t size, int error,
return error;
}
-static int ocfs2_dio_end_io_r_pr(struct kiocb *iocb, ssize_t size,
- int error, unsigned int flags)
+static int ocfs2_iomap_dio_end_io_r_pr(struct kiocb *iocb, ssize_t size,
+ int error, unsigned int flags)
{
if (error)
mlog_ratelimited(ML_ERROR, "Direct IO failed, bytes = %lld errno:%d",
@@ -2662,13 +2844,199 @@ static int ocfs2_dio_end_io_r_pr(struct kiocb *iocb, ssize_t size,
error = ocfs2_iomap_dio_end_io(iocb, size, error, 0);
-bail:
return error;
}
const struct iomap_dio_ops ocfs2_iomap_dio_ops_r_pr = {
- .end_io = ocfs2_dio_end_io_r_pr,
+ .end_io = ocfs2_iomap_dio_end_io_r_pr,
+};
+
+/* copy from ocfs2_dio_end_io_write */
+static int ocfs2_iomap_dio_end_io_write(struct inode *inode,
+ loff_t offset,
+ ssize_t bytes)
+{
+ struct ocfs2_cached_dealloc_ctxt dealloc;
+ struct ocfs2_extent_tree et;
+ struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+ struct ocfs2_inode_info *oi = OCFS2_I(inode);
+ struct buffer_head *di_bh = NULL;
+ struct ocfs2_dinode *di;
+ struct ocfs2_alloc_context *data_ac = NULL;
+ struct ocfs2_alloc_context *meta_ac = NULL;
+ handle_t *handle = NULL;
+ loff_t end = offset + bytes;
+ int ret = 0, credits = 0;
+ struct ocfs2_map_block map;
+ unsigned int blkbits = inode->i_blkbits;
+ unsigned int max_blocks;
+ unsigned int ue_cpos = 0, ue_phys = 0, ue_len = 0;
+ unsigned int curr_lblk, end_lblk;
+
+ map.lblk = offset >> blkbits;
+ max_blocks = (bytes + offset) >> osb->s_clustersize_bits;
+
+ ocfs2_init_dealloc_ctxt(&dealloc);
+
+ ret = ocfs2_inode_lock(inode, &di_bh, 1);
+ if (ret < 0) {
+ mlog_errno(ret);
+ goto out;
+ }
+
+ down_write(&oi->ip_alloc_sem);
+
+ di = (struct ocfs2_dinode *)di_bh->b_data;
+
+ ocfs2_init_dinode_extent_tree(&et, INODE_CACHE(inode), di_bh);
+
+ /* Attach dealloc with extent tree in case that we may reuse extents
+ * which are already unlinked from current extent tree due to extent
+ * rotation and merging.
+ */
+ et.et_dealloc = &dealloc;
+
+ ret = ocfs2_lock_allocators(inode, &et, 0, (bytes >> osb->s_clustersize_bits)*2,
+ &data_ac, &meta_ac);
+ if (ret) {
+ mlog_errno(ret);
+ goto unlock;
+ }
+
+ credits = ocfs2_calc_extend_credits(inode->i_sb, &di->id2.i_list);
+
+ handle = ocfs2_start_trans(osb, credits);
+ if (IS_ERR(handle)) {
+ ret = PTR_ERR(handle);
+ mlog_errno(ret);
+ goto unlock;
+ }
+ ret = ocfs2_journal_access_di(handle, INODE_CACHE(inode), di_bh,
+ OCFS2_JOURNAL_ACCESS_WRITE);
+ if (ret) {
+ mlog_errno(ret);
+ goto commit;
+ }
+
+ curr_lblk = offset >> blkbits;
+ /*
+ * Round the end up so the final partial block (sub-block direct I/O)
+ * is included; otherwise the last, partially-written cluster is left
+ * unwritten and reads back as zero.
+ */
+ end_lblk = (offset + bytes + (1 << blkbits) - 1) >> blkbits;
+ while (ret >= 0 && curr_lblk < end_lblk) {
+ memset(&map, 0, sizeof(map));
+ map.lblk += curr_lblk;
+ map.len = end_lblk - curr_lblk;
+
+ ret = ocfs2_assure_trans_credits(handle, credits);
+ if (ret < 0) {
+ mlog_errno(ret);
+ break;
+ }
+
+ ret = ocfs2_map_blocks(inode, &map, 0);
+ if (ret < 0) {
+ mlog_errno(ret);
+ break;
+ }
+ if (ret == 0)
+ break;
+
+ if (map.flags & OCFS2_MAP_UNWRITTEN) {
+ ue_cpos = ocfs2_blocks_to_clusters(inode->i_sb, map.lblk);
+ ue_phys = ocfs2_blocks_to_clusters(inode->i_sb, map.pblk);
+ ue_len = ocfs2_clusters_for_blocks(inode->i_sb,
+ map.lblk + map.len) - ue_cpos;
+
+ ret = ocfs2_mark_extent_written(inode, &et, handle,
+ ue_cpos, ue_len,
+ ue_phys,
+ meta_ac, &dealloc);
+ if (ret < 0) {
+ mlog_errno(ret);
+ break;
+ }
+ }
+ curr_lblk += map.len;
+ }
+
+ if (end > i_size_read(inode)) {
+ ret = ocfs2_set_inode_size(handle, inode, di_bh, end);
+ if (ret < 0)
+ mlog_errno(ret);
+ }
+
+commit:
+ ocfs2_commit_trans(osb, handle);
+
+unlock:
+ up_write(&oi->ip_alloc_sem);
+
+ if (data_ac)
+ ocfs2_free_alloc_context(data_ac);
+ if (meta_ac)
+ ocfs2_free_alloc_context(meta_ac);
+
+ down_write(&oi->ip_alloc_sem);
+ if (!ret && (di->i_flags & cpu_to_le32(OCFS2_DIO_ORPHANED_FL))) {
+ ret = ocfs2_del_inode_from_orphan(osb, inode, di_bh, 0, 0);
+ if (ret < 0)
+ mlog_errno(ret);
+ }
+ up_write(&oi->ip_alloc_sem);
+ ocfs2_inode_unlock(inode, 1);
+ brelse(di_bh);
+
+out:
+ ocfs2_run_deallocs(osb, &dealloc);
+ return ret;
+}
+
+static int ocfs2_dio_write_end_io(struct kiocb *iocb, ssize_t size,
+ int error, unsigned int flags, int level)
+{
+ struct inode *inode = file_inode(iocb->ki_filp);
+ loff_t offset = iocb->ki_pos;
+ int ret = 0;
+
+ if (error)
+ mlog_ratelimited(ML_ERROR, "Direct IO failed, bytes = %lld errno:%d",
+ (long long)size, error);
+
+ if (size && ((flags & IOMAP_DIO_UNWRITTEN) ||
+ (offset + size > i_size_read(inode)))) {
+ ret = ocfs2_iomap_dio_end_io_write(inode, offset, size);
+ if (ret < 0)
+ mlog_errno(ret);
+ }
+
+ error = ocfs2_iomap_dio_end_io(iocb, size, error, level);
+
+ return error;
+}
+
+static int ocfs2_iomap_dio_end_io_w_pr(struct kiocb *iocb, ssize_t size,
+ int error, unsigned int flags)
+{
+ return ocfs2_dio_write_end_io(iocb, size, error, flags, 0);
+}
+
+static int ocfs2_iomap_dio_end_io_w_ex(struct kiocb *iocb, ssize_t size,
+ int error, unsigned int flags)
+{
+ return ocfs2_dio_write_end_io(iocb, size, error, flags, 1);
+}
+
+const struct iomap_dio_ops ocfs2_iomap_dio_ops_w_pr = {
+ .end_io = ocfs2_iomap_dio_end_io_w_pr,
+};
+
+const struct iomap_dio_ops ocfs2_iomap_dio_ops_w_ex = {
+ .end_io = ocfs2_iomap_dio_end_io_w_ex,
};
+
const struct address_space_operations ocfs2_aops = {
.dirty_folio = block_dirty_folio,
.read_folio = ocfs2_read_folio,
diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
index 493f2209cca5..03906ab737ec 100644
--- a/fs/ocfs2/buffer_head_io.c
+++ b/fs/ocfs2/buffer_head_io.c
@@ -332,6 +332,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
wait_on_buffer(bh);
put_bh(bh);
bhs[i] = NULL;
+ } else if (bh && buffer_uptodate(bh)) {
+ clear_buffer_uptodate(bh);
}
continue;
}
@@ -360,11 +362,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
BUG_ON(buffer_jbd(bh));
clear_buffer_needs_validate(bh);
status = validate(sb, bh);
- if (status) {
- if (buffer_uptodate(bh))
- clear_buffer_uptodate(bh);
+ if (status)
goto read_failure;
- }
}
}
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index a0b3883216bc..2d78f2863acf 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -48,6 +48,7 @@
#include "quota.h"
#include "refcounttree.h"
#include "ocfs2_trace.h"
+#include "namei.h"
#include "buffer_head_io.h"
@@ -2378,6 +2379,8 @@ static int ocfs2_prepare_inode_for_write(struct file *file,
static bool ocfs2_should_use_dio(struct kiocb *iocb, struct iov_iter *iter,
struct inode *inode)
{
+ struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+
/*
* Fallback to buffered I/O if we see an inode without
* extents.
@@ -2385,6 +2388,11 @@ static bool ocfs2_should_use_dio(struct kiocb *iocb, struct iov_iter *iter,
if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL)
return false;
+ /* Fallback to buffered I/O if we do not support append dio. */
+ if (iocb->ki_pos + iter->count > i_size_read(inode) &&
+ !ocfs2_supports_append_dio(osb))
+ return false;
+
return true;
}
@@ -2405,6 +2413,8 @@ static ssize_t ocfs2_file_write_iter(struct kiocb *iocb,
i_size_read(inode) ? 1 : 0);
int direct_io = iocb->ki_flags & IOCB_DIRECT ? 1 : 0;
int nowait = iocb->ki_flags & IOCB_NOWAIT ? 1 : 0;
+ int dio_flags = 0;
+ ssize_t buffered;
trace_ocfs2_file_write_iter(inode, file, file->f_path.dentry,
(unsigned long long)OCFS2_I(inode)->ip_blkno,
@@ -2424,8 +2434,6 @@ static ssize_t ocfs2_file_write_iter(struct kiocb *iocb,
} else
inode_lock(inode);
- ocfs2_iocb_init_rw_locked(iocb);
-
/*
* Concurrent O_DIRECT writes are allowed with
* mount_option "coherency=buffered".
@@ -2489,26 +2497,73 @@ static ssize_t ocfs2_file_write_iter(struct kiocb *iocb,
saved_ki_complete = xchg(&iocb->ki_complete, NULL);
}
- /* communicate with ocfs2_dio_end_io */
- ocfs2_iocb_set_rw_locked(iocb, rw_level);
+ if (direct_io && ocfs2_should_use_dio(iocb, from, inode)) {
+ if (ocfs2_clusters_for_bytes(inode->i_sb, iocb->ki_pos + count) >
+ ocfs2_clusters_for_bytes(inode->i_sb, i_size_read(inode))) {
+ /*
+ * when we are going to alloc extents beyond file size, add the
+ * inode to orphan dir, so we can recall those spaces when
+ * system crashed during write.
+ */
+ ret = ocfs2_add_inode_to_orphan(osb, inode);
+ if (ret < 0) {
+ mlog_errno(ret);
+ goto out;
+ }
+ dio_flags = IOMAP_DIO_FORCE_WAIT;
+ }
+ const struct iomap_dio_ops *dops = rw_level ?
+ &ocfs2_iomap_dio_ops_w_ex : &ocfs2_iomap_dio_ops_w_pr;
+ struct iomap_dio *dio;
+
+ dio = __iomap_dio_rw(iocb, from, &ocfs2_iomap_ops,
+ dops, dio_flags, NULL, 0);
+ if (dio == NULL) {
+ /* No I/O issued; rw_lock still held. */
+ written = 0;
+ } else if (IS_ERR(dio)) {
+ written = PTR_ERR(dio);
+ /*
+ * -EIOCBQUEUED: async I/O; end_io will drop the lock on
+ * completion. Any other error means no I/O started and
+ * end_io was not called, so the cleanup path below drops
+ * the lock.
+ */
+ if (written == -EIOCBQUEUED)
+ rw_level = -1;
+ } else {
+ /*
+ * Synchronous completion. iomap_dio_complete() runs
+ * end_io, which drops the lock unless the mapping
+ * bounced us back to buffered I/O (returns 0, lock
+ * retained).
+ */
+ written = iomap_dio_complete(dio);
+ if (written != 0)
+ rw_level = -1;
+ }
+ /*
+ * iomap_dio_rw() returns -ENOTBLK when it could not invalidate
+ * the page cache for the DIO range (e.g. racing buffered/mmap
+ * I/O on the same file, as exercised by generic/095). Fall back
+ * to a buffered write for the remaining data, the same way
+ * ext4/xfs do, instead of leaking -ENOTBLK to userspace.
+ */
+ if (written == -ENOTBLK)
+ written = 0;
+ if (written >= 0 && iov_iter_count(from)) {
+ iocb->ki_flags &= ~IOCB_DIRECT;
+ buffered = __generic_file_write_iter(iocb, from);
+ written = direct_write_fallback(iocb, from, written, buffered);
+ }
+ } else {
+ iocb->ki_flags &= ~IOCB_DIRECT;
+ written = __generic_file_write_iter(iocb, from);
+ }
- written = __generic_file_write_iter(iocb, from);
/* buffered aio wouldn't have proper lock coverage today */
BUG_ON(written == -EIOCBQUEUED && !direct_io);
- /*
- * deep in g_f_a_w_n()->ocfs2_direct_IO we pass in a ocfs2_dio_end_io
- * function pointer which is called when o_direct io completes so that
- * it can unlock our rw lock.
- * Unfortunately there are error cases which call end_io and others
- * that don't. so we don't have to unlock the rw_lock if either an
- * async dio is going to do it in the future or an end_io after an
- * error has already done it.
- */
- if ((written == -EIOCBQUEUED) || (!ocfs2_iocb_is_rw_locked(iocb))) {
- rw_level = -1;
- }
-
if (unlikely(written <= 0))
goto out;
diff --git a/fs/ocfs2/ocfs2.h b/fs/ocfs2/ocfs2.h
index a775c1869293..b228b10b5e71 100644
--- a/fs/ocfs2/ocfs2.h
+++ b/fs/ocfs2/ocfs2.h
@@ -551,6 +551,8 @@ struct ocfs2_map_block {
extern const struct iomap_ops ocfs2_iomap_ops;
extern const struct iomap_dio_ops ocfs2_iomap_dio_ops_r_pr;
+extern const struct iomap_dio_ops ocfs2_iomap_dio_ops_w_pr;
+extern const struct iomap_dio_ops ocfs2_iomap_dio_ops_w_ex;
/* Flags used by ocfs2_map_blocks() */
#define OCFS2_GET_BLOCKS_CREATE (0x0001)
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [RFC PATCH v2 3/4] ocfs2: switch dio write path from buffer_head to iomap
2026-07-27 6:17 ` [RFC PATCH v2 3/4] ocfs2: switch dio write " Heming Zhao
@ 2026-07-28 4:18 ` Christoph Hellwig
2026-07-28 6:07 ` Joseph Qi
1 sibling, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2026-07-28 4:18 UTC (permalink / raw)
To: Heming Zhao; +Cc: joseph.qi, mark, jlbec, hch, ocfs2-devel, linux-kernel
On Mon, Jul 27, 2026 at 02:17:59PM +0800, Heming Zhao wrote:
> This patch converts OCFS2's DIO write path from the legacy
> buffer_head infrastructure to the modern iomap framework.
>
> Key modifications and designs are as follows:
>
> 1. Dynamic Context Allocation:
This reads like complete AI slop. Please turn this into a coherent
commit message without the lists and exaggerations.
I'll try to read that before reading the commit, because it kinda
implies a lack of understing by anyone what actually happens.
So please try to understand what you're doing or had your tool do,
then write it down and re-review if the code makes sense based
on that.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v2 3/4] ocfs2: switch dio write path from buffer_head to iomap
2026-07-27 6:17 ` [RFC PATCH v2 3/4] ocfs2: switch dio write " Heming Zhao
2026-07-28 4:18 ` Christoph Hellwig
@ 2026-07-28 6:07 ` Joseph Qi
1 sibling, 0 replies; 14+ messages in thread
From: Joseph Qi @ 2026-07-28 6:07 UTC (permalink / raw)
To: Heming Zhao; +Cc: mark, jlbec, hch, ocfs2-devel, linux-kernel
On 7/27/26 2:17 PM, Heming Zhao wrote:
> This patch converts OCFS2's DIO write path from the legacy
> buffer_head infrastructure to the modern iomap framework.
>
> Key modifications and designs are as follows:
>
> 1. Dynamic Context Allocation:
> Refactor 'struct ocfs2_write_ctxt' to use a flexible array 'w_desc[]'
> instead of a fixed-size array. Dynamically allocate the context based on
> the write length ('w_clen') in 'ocfs2_alloc_write_ctxt()'. This prevents
> static limits overflow and optimizes kernel heap memory utilization.
>
> 2. Robust Mapping and Limits:
> Introduce 'ocfs2_dio_wr_map_blocks()' to allocate and map direct write
> blocks. Implement a 1 MiB cap ('OCFS2_DIO_WR_MAX_MAX_BYTES') per mapping
> call to restrict allocation granularity, preventing JBD2 transaction
> credit exhaustion during huge asynchronous sequential writes.
>
> 3. Reliable Completion Work and Fallback:
> - Implement 'ocfs2_iomap_dio_end_io_write()' to handle metadata completion.
> It converts UNWRITTEN extents, updates inode size (EOF), and deletes the
> inode from the orphan directory if it was appended.
> - Implement 'ocfs2_dio_write_end_io()' to finalize the dio lifecycle and
> release cluster locks safely.
> - Intercept '-ENOTBLK' errors from 'iomap_dio_rw()' caused by page cache
> invalidation failures (due to mmap/buffered collisions). Gracefully clear
> the error, strip the IOCB_DIRECT flag, and fall back to buffered write
>
> 4. Moved ocfs2_add_inode_to_orphan():
> - moved ocfs2_add_inode_to_orphan() from ocfs2_dio_wr_map_blocks() to
> ocfs2_file_write_iter().
>
> 5. Uncertain logic in code
> For the following code block in ocfs2_dio_wr_map_blocks():
> ```
> if (extend) {
> if (ocfs2_sparse_alloc(osb))
> ret = ocfs2_zero_tail(inode, di_bh, pos);
> else
> ret = ocfs2_expand_nonsparse_inode(inode, di_bh, pos,
> map_len, NULL);
> if (ret < 0) {
> mlog_errno(ret);
> goto unlock;
> }
> }
> ```
> I am not completely certain whether it is called only once per
> ocfs2_file_write_iter(), but I believe calling it multiple times will
> not introduce any side effects. Furthermore, testing across various
> scenarios showed no instances of multiple calls.
>
> Assisted-by: Gemini:gemini-3.5-flash
> Assisted-by: Claude:claude-sonnet-4-5
> Co-developed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Signed-off-by: Heming Zhao <heming.zhao@suse.com>
> ---
> fs/ocfs2/aops.c | 394 ++++++++++++++++++++++++++++++++++++--
> fs/ocfs2/buffer_head_io.c | 7 +-
> fs/ocfs2/file.c | 91 +++++++--
> fs/ocfs2/ocfs2.h | 2 +
> 4 files changed, 459 insertions(+), 35 deletions(-)
>
> diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
> index 12f5f2e3530a..9a079436c9c0 100644
> --- a/fs/ocfs2/aops.c
> +++ b/fs/ocfs2/aops.c
...
> +
> + ocfs2_free_unwritten_list(inode, &wc->w_unwritten_list);
> + ret = ocfs2_write_end_nolock(inode->i_mapping, pos, map_len, map_len, wc);
> + BUG_ON(ret != map_len);
Under memory pressure, folio allocation may fail. In this case,
ocfs2_write_end_nolock() can return a short count.
So we must handle this case gracefully.
> + ret = 0;
> +
> +unlock:
> + up_write(&oi->ip_alloc_sem);
> + ocfs2_inode_unlock(inode, 1);
> + brelse(di_bh);
> +
> +out:
> + return ret;
> +}
> +
> static int ocfs2_dio_end_io_write(struct inode *inode,
> struct ocfs2_dio_write_ctxt *dwc,
> loff_t offset,
...
> +static int ocfs2_iomap_dio_end_io_write(struct inode *inode,
> + loff_t offset,
> + ssize_t bytes)
> +{
> + struct ocfs2_cached_dealloc_ctxt dealloc;
> + struct ocfs2_extent_tree et;
> + struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
> + struct ocfs2_inode_info *oi = OCFS2_I(inode);
> + struct buffer_head *di_bh = NULL;
> + struct ocfs2_dinode *di;
> + struct ocfs2_alloc_context *data_ac = NULL;
> + struct ocfs2_alloc_context *meta_ac = NULL;
> + handle_t *handle = NULL;
> + loff_t end = offset + bytes;
> + int ret = 0, credits = 0;
> + struct ocfs2_map_block map;
> + unsigned int blkbits = inode->i_blkbits;
> + unsigned int max_blocks;
> + unsigned int ue_cpos = 0, ue_phys = 0, ue_len = 0;
> + unsigned int curr_lblk, end_lblk;
> +
> + map.lblk = offset >> blkbits;
> + max_blocks = (bytes + offset) >> osb->s_clustersize_bits;
Seems unused.
...
> + curr_lblk = offset >> blkbits;
> + /*
> + * Round the end up so the final partial block (sub-block direct I/O)
> + * is included; otherwise the last, partially-written cluster is left
> + * unwritten and reads back as zero.
> + */
> + end_lblk = (offset + bytes + (1 << blkbits) - 1) >> blkbits;
> + while (ret >= 0 && curr_lblk < end_lblk) {
> + memset(&map, 0, sizeof(map));
> + map.lblk += curr_lblk;
Since map is memset just now, so here we can use "map.lblk = curr_lblk"
directly.
> + map.len = end_lblk - curr_lblk;
> +
> + ret = ocfs2_assure_trans_credits(handle, credits);
> + if (ret < 0) {
> + mlog_errno(ret);
> + break;
> + }
> +
...
> +
> +static int ocfs2_dio_write_end_io(struct kiocb *iocb, ssize_t size,
> + int error, unsigned int flags, int level)
> +{
> + struct inode *inode = file_inode(iocb->ki_filp);
> + loff_t offset = iocb->ki_pos;
> + int ret = 0;
> +
> + if (error)
> + mlog_ratelimited(ML_ERROR, "Direct IO failed, bytes = %lld errno:%d",
> + (long long)size, error);
> +
> + if (size && ((flags & IOMAP_DIO_UNWRITTEN) ||
> + (offset + size > i_size_read(inode)))) {
Is it safe to do i_size_read() in case async dio completion?
...
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC PATCH v2 4/4] ocfs2: remove legacy blockdev direct-IO path APIs
2026-07-27 6:17 [RFC PATCH v2 0/4] migrates ocfs2 DIO from buffer_head to iomap Heming Zhao
` (2 preceding siblings ...)
2026-07-27 6:17 ` [RFC PATCH v2 3/4] ocfs2: switch dio write " Heming Zhao
@ 2026-07-27 6:18 ` Heming Zhao
2026-07-28 4:19 ` Christoph Hellwig
2026-07-28 6:11 ` Joseph Qi
2026-07-28 4:11 ` [RFC PATCH v2 0/4] migrates ocfs2 DIO from buffer_head to iomap Christoph Hellwig
4 siblings, 2 replies; 14+ messages in thread
From: Heming Zhao @ 2026-07-27 6:18 UTC (permalink / raw)
To: joseph.qi, mark, jlbec, hch; +Cc: Heming Zhao, ocfs2-devel, linux-kernel
Since the DIO path already migrated from buffer_head mode (by using
a_ops->direct_IO method) to iomap mode, the VFS no longer sets
FMODE_CAN_ODIRECT automatically, so ocfs2_file_open() sets it
explicitly.
The now-unused blockdev get_block/end_io machinery
(ocfs2_lock_get_block(), ocfs2_dio_wr_get_block(), ocfs2_dio_end_io(),
ocfs2_dio_end_io_write(), and the ocfs2_dio_write_ctxt helpers) is removed,
and OCFS2_FS Kconfig removes LEGACY_DIRECT_IO.
Co-developed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Signed-off-by: Heming Zhao <heming.zhao@suse.com>
---
fs/ocfs2/Kconfig | 1 -
fs/ocfs2/aops.c | 411 -----------------------------------------------
fs/ocfs2/aops.h | 35 +---
fs/ocfs2/file.c | 13 +-
fs/ocfs2/ocfs2.h | 5 -
5 files changed, 16 insertions(+), 449 deletions(-)
diff --git a/fs/ocfs2/Kconfig b/fs/ocfs2/Kconfig
index bf1678a5eb01..74bc2008ef0b 100644
--- a/fs/ocfs2/Kconfig
+++ b/fs/ocfs2/Kconfig
@@ -9,7 +9,6 @@ config OCFS2_FS
select QUOTA_TREE
select FS_IOMAP
select FS_POSIX_ACL
- select LEGACY_DIRECT_IO
help
OCFS2 is a general purpose extent based shared disk cluster file
system with many similarities to ext3. It supports 64 bit inode
diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
index 9a079436c9c0..3e4a7043074a 100644
--- a/fs/ocfs2/aops.c
+++ b/fs/ocfs2/aops.c
@@ -114,19 +114,6 @@ static int ocfs2_symlink_get_block(struct inode *inode, sector_t iblock,
return err;
}
-static int ocfs2_lock_get_block(struct inode *inode, sector_t iblock,
- struct buffer_head *bh_result, int create)
-{
- int ret = 0;
- struct ocfs2_inode_info *oi = OCFS2_I(inode);
-
- down_read(&oi->ip_alloc_sem);
- ret = ocfs2_get_block(inode, iblock, bh_result, create);
- up_read(&oi->ip_alloc_sem);
-
- return ret;
-}
-
int ocfs2_map_blocks(struct inode *inode, struct ocfs2_map_block *map,
int flags)
{
@@ -2170,204 +2157,6 @@ static int ocfs2_write_end(const struct kiocb *iocb,
return ret;
}
-struct ocfs2_dio_write_ctxt {
- struct list_head dw_zero_list;
- unsigned dw_zero_count;
- int dw_orphaned;
- pid_t dw_writer_pid;
-};
-
-static struct ocfs2_dio_write_ctxt *
-ocfs2_dio_alloc_write_ctx(struct buffer_head *bh, int *alloc)
-{
- struct ocfs2_dio_write_ctxt *dwc = NULL;
-
- if (bh->b_private)
- return bh->b_private;
-
- dwc = kmalloc_obj(struct ocfs2_dio_write_ctxt, GFP_NOFS);
- if (dwc == NULL)
- return NULL;
- INIT_LIST_HEAD(&dwc->dw_zero_list);
- dwc->dw_zero_count = 0;
- dwc->dw_orphaned = 0;
- dwc->dw_writer_pid = task_pid_nr(current);
- bh->b_private = dwc;
- *alloc = 1;
-
- return dwc;
-}
-
-static void ocfs2_dio_free_write_ctx(struct inode *inode,
- struct ocfs2_dio_write_ctxt *dwc)
-{
- ocfs2_free_unwritten_list(inode, &dwc->dw_zero_list);
- kfree(dwc);
-}
-
-/*
- * TODO: Make this into a generic get_blocks function.
- *
- * From do_direct_io in direct-io.c:
- * "So what we do is to permit the ->get_blocks function to populate
- * bh.b_size with the size of IO which is permitted at this offset and
- * this i_blkbits."
- *
- * This function is called directly from get_more_blocks in direct-io.c.
- *
- * called like this: dio->get_blocks(dio->inode, fs_startblk,
- * fs_count, map_bh, dio->rw == WRITE);
- */
-static int ocfs2_dio_wr_get_block(struct inode *inode, sector_t iblock,
- struct buffer_head *bh_result, int create)
-{
- struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
- struct ocfs2_inode_info *oi = OCFS2_I(inode);
- struct ocfs2_write_ctxt *wc;
- struct ocfs2_write_cluster_desc *desc = NULL;
- struct ocfs2_dio_write_ctxt *dwc = NULL;
- struct buffer_head *di_bh = NULL;
- u64 p_blkno = 0;
- unsigned int i_blkbits = inode->i_sb->s_blocksize_bits;
- loff_t pos = iblock << i_blkbits;
- sector_t endblk = (i_size_read(inode) - 1) >> i_blkbits;
- unsigned len, total_len = bh_result->b_size;
- int ret = 0, first_get_block = 0;
-
- len = osb->s_clustersize - (pos & (osb->s_clustersize - 1));
- len = min(total_len, len);
-
- /*
- * bh_result->b_size is count in get_more_blocks according to write
- * "pos" and "end", we need map twice to return different buffer state:
- * 1. area in file size, not set NEW;
- * 2. area out file size, set NEW.
- *
- * iblock endblk
- * |--------|---------|---------|---------
- * |<-------area in file------->|
- */
-
- if ((iblock <= endblk) &&
- ((iblock + ((len - 1) >> i_blkbits)) > endblk))
- len = (endblk - iblock + 1) << i_blkbits;
-
- mlog(0, "get block of %llu at %llu:%u req %u\n",
- inode->i_ino, pos, len, total_len);
-
- /*
- * Because we need to change file size in ocfs2_dio_end_io_write(), or
- * we may need to add it to orphan dir. So can not fall to fast path
- * while file size will be changed.
- */
- if (pos + total_len <= i_size_read(inode)) {
-
- /* This is the fast path for re-write. */
- ret = ocfs2_lock_get_block(inode, iblock, bh_result, create);
- if (buffer_mapped(bh_result) &&
- !buffer_new(bh_result) &&
- ret == 0)
- goto out;
-
- /* Clear state set by ocfs2_get_block. */
- bh_result->b_state = 0;
- }
-
- dwc = ocfs2_dio_alloc_write_ctx(bh_result, &first_get_block);
- if (unlikely(dwc == NULL)) {
- ret = -ENOMEM;
- mlog_errno(ret);
- goto out;
- }
-
- if (ocfs2_clusters_for_bytes(inode->i_sb, pos + total_len) >
- ocfs2_clusters_for_bytes(inode->i_sb, i_size_read(inode)) &&
- !dwc->dw_orphaned) {
- /*
- * when we are going to alloc extents beyond file size, add the
- * inode to orphan dir, so we can recall those spaces when
- * system crashed during write.
- */
- ret = ocfs2_add_inode_to_orphan(osb, inode);
- if (ret < 0) {
- mlog_errno(ret);
- goto out;
- }
- dwc->dw_orphaned = 1;
- }
-
- ret = ocfs2_inode_lock(inode, &di_bh, 1);
- if (ret) {
- mlog_errno(ret);
- goto out;
- }
-
- down_write(&oi->ip_alloc_sem);
-
- if (first_get_block) {
- if (ocfs2_sparse_alloc(osb))
- ret = ocfs2_zero_tail(inode, di_bh, pos);
- else
- ret = ocfs2_expand_nonsparse_inode(inode, di_bh, pos,
- total_len, NULL);
- if (ret < 0) {
- mlog_errno(ret);
- goto unlock;
- }
- }
-
- ret = ocfs2_write_begin_nolock(inode->i_mapping, pos, len,
- OCFS2_WRITE_DIRECT, NULL,
- (void **)&wc, di_bh, NULL);
- if (ret) {
- mlog_errno(ret);
- goto unlock;
- }
-
- desc = &wc->w_desc[0];
-
- p_blkno = ocfs2_clusters_to_blocks(inode->i_sb, desc->c_phys);
- BUG_ON(p_blkno == 0);
- p_blkno += iblock & (u64)(ocfs2_clusters_to_blocks(inode->i_sb, 1) - 1);
-
- map_bh(bh_result, inode->i_sb, p_blkno);
- bh_result->b_size = len;
- if (desc->c_needs_zero)
- set_buffer_new(bh_result);
-
- if (iblock > endblk)
- set_buffer_new(bh_result);
-
- /* May sleep in end_io. It should not happen in a irq context. So defer
- * it to dio work queue. */
- set_buffer_defer_completion(bh_result);
-
- if (!list_empty(&wc->w_unwritten_list)) {
- struct ocfs2_unwritten_extent *ue = NULL;
-
- ue = list_first_entry(&wc->w_unwritten_list,
- struct ocfs2_unwritten_extent,
- ue_node);
- BUG_ON(ue->ue_cpos != desc->c_cpos);
- /* The physical address may be 0, fill it. */
- ue->ue_phys = desc->c_phys;
-
- list_splice_tail_init(&wc->w_unwritten_list, &dwc->dw_zero_list);
- dwc->dw_zero_count += wc->w_unwritten_count;
- }
-
- ret = ocfs2_write_end_nolock(inode->i_mapping, pos, len, len, wc);
- BUG_ON(ret != len);
- ret = 0;
-unlock:
- up_write(&oi->ip_alloc_sem);
- ocfs2_inode_unlock(inode, 1);
- brelse(di_bh);
-out:
- return ret;
-}
-
-/* copy from ocfs2_dio_wr_get_block */
static int ocfs2_dio_wr_map_blocks(struct inode *inode,
struct ocfs2_map_block *map, int create)
{
@@ -2509,205 +2298,6 @@ static int ocfs2_dio_wr_map_blocks(struct inode *inode,
return ret;
}
-static int ocfs2_dio_end_io_write(struct inode *inode,
- struct ocfs2_dio_write_ctxt *dwc,
- loff_t offset,
- ssize_t bytes)
-{
- struct ocfs2_cached_dealloc_ctxt dealloc;
- struct ocfs2_extent_tree et;
- struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
- struct ocfs2_inode_info *oi = OCFS2_I(inode);
- struct ocfs2_unwritten_extent *ue = NULL;
- struct buffer_head *di_bh = NULL;
- struct ocfs2_dinode *di;
- struct ocfs2_alloc_context *data_ac = NULL;
- struct ocfs2_alloc_context *meta_ac = NULL;
- handle_t *handle = NULL;
- loff_t end = offset + bytes;
- int ret = 0, credits = 0, batch = 0;
-
- ocfs2_init_dealloc_ctxt(&dealloc);
-
- /* We do clear unwritten, delete orphan, change i_size here. If neither
- * of these happen, we can skip all this. */
- if (list_empty(&dwc->dw_zero_list) &&
- end <= i_size_read(inode) &&
- !dwc->dw_orphaned)
- goto out;
-
- ret = ocfs2_inode_lock(inode, &di_bh, 1);
- if (ret < 0) {
- mlog_errno(ret);
- goto out;
- }
-
- down_write(&oi->ip_alloc_sem);
- di = (struct ocfs2_dinode *)di_bh->b_data;
-
- ocfs2_init_dinode_extent_tree(&et, INODE_CACHE(inode), di_bh);
-
- /* Attach dealloc with extent tree in case that we may reuse extents
- * which are already unlinked from current extent tree due to extent
- * rotation and merging.
- */
- et.et_dealloc = &dealloc;
-
- ret = ocfs2_lock_allocators(inode, &et, 0, dwc->dw_zero_count*2,
- &data_ac, &meta_ac);
- if (ret) {
- mlog_errno(ret);
- goto unlock;
- }
-
- credits = ocfs2_calc_extend_credits(inode->i_sb, &di->id2.i_list);
-
- list_for_each_entry(ue, &dwc->dw_zero_list, ue_node) {
- if (!handle) {
- handle = ocfs2_start_trans(osb, credits);
- if (IS_ERR(handle)) {
- ret = PTR_ERR(handle);
- mlog_errno(ret);
- goto unlock;
- }
- ret = ocfs2_journal_access_di(handle, INODE_CACHE(inode), di_bh,
- OCFS2_JOURNAL_ACCESS_WRITE);
- if (ret) {
- mlog_errno(ret);
- goto commit;
- }
- }
- ret = ocfs2_assure_trans_credits(handle, credits);
- if (ret < 0) {
- mlog_errno(ret);
- goto commit;
- }
- ret = ocfs2_mark_extent_written(inode, &et, handle,
- ue->ue_cpos, 1,
- ue->ue_phys,
- meta_ac, &dealloc);
- if (ret < 0) {
- mlog_errno(ret);
- goto commit;
- }
-
- if (++batch == OCFS2_DIO_MARK_EXTENT_BATCH) {
- ocfs2_commit_trans(osb, handle);
- handle = NULL;
- batch = 0;
- }
- }
-
- if (end > i_size_read(inode)) {
- if (!handle) {
- handle = ocfs2_start_trans(osb, credits);
- if (IS_ERR(handle)) {
- ret = PTR_ERR(handle);
- mlog_errno(ret);
- goto unlock;
- }
- }
- ret = ocfs2_set_inode_size(handle, inode, di_bh, end);
- if (ret < 0)
- mlog_errno(ret);
- }
-
-commit:
- if (handle)
- ocfs2_commit_trans(osb, handle);
-unlock:
- up_write(&oi->ip_alloc_sem);
-
- if (data_ac) {
- ocfs2_free_alloc_context(data_ac);
- data_ac = NULL;
- }
- if (meta_ac) {
- ocfs2_free_alloc_context(meta_ac);
- meta_ac = NULL;
- }
-
- /* everything looks good, let's start the cleanup */
- if (!ret && dwc->dw_orphaned) {
- BUG_ON(dwc->dw_writer_pid != task_pid_nr(current));
-
- ret = ocfs2_del_inode_from_orphan(osb, inode, di_bh, 0, 0);
- if (ret < 0)
- mlog_errno(ret);
- }
- ocfs2_inode_unlock(inode, 1);
- brelse(di_bh);
-out:
- ocfs2_run_deallocs(osb, &dealloc);
- ocfs2_dio_free_write_ctx(inode, dwc);
-
- return ret;
-}
-
-/*
- * ocfs2_dio_end_io is called by the dio core when a dio is finished. We're
- * particularly interested in the aio/dio case. We use the rw_lock DLM lock
- * to protect io on one node from truncation on another.
- */
-static int ocfs2_dio_end_io(struct kiocb *iocb,
- loff_t offset,
- ssize_t bytes,
- void *private)
-{
- struct inode *inode = file_inode(iocb->ki_filp);
- int level;
- int ret = 0;
-
- /* this io's submitter should not have unlocked this before we could */
- BUG_ON(!ocfs2_iocb_is_rw_locked(iocb));
-
- if (bytes <= 0)
- mlog_ratelimited(ML_ERROR, "Direct IO failed, bytes = %lld",
- (long long)bytes);
- if (private) {
- if (bytes > 0)
- ret = ocfs2_dio_end_io_write(inode, private, offset,
- bytes);
- else
- ocfs2_dio_free_write_ctx(inode, private);
- }
-
- ocfs2_iocb_clear_rw_locked(iocb);
-
- level = ocfs2_iocb_rw_locked_level(iocb);
- ocfs2_rw_unlock(inode, level);
- return ret;
-}
-
-static ssize_t ocfs2_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
-{
- struct file *file = iocb->ki_filp;
- struct inode *inode = file->f_mapping->host;
- struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
- get_block_t *get_block;
-
- /*
- * Fallback to buffered I/O if we see an inode without
- * extents.
- */
- if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL)
- return 0;
-
- /* Fallback to buffered I/O if we do not support append dio. */
- if (iocb->ki_pos + iter->count > i_size_read(inode) &&
- !ocfs2_supports_append_dio(osb))
- return 0;
-
- if (iov_iter_rw(iter) == READ)
- get_block = ocfs2_lock_get_block;
- else
- get_block = ocfs2_dio_wr_get_block;
-
- return __blockdev_direct_IO(iocb, inode, inode->i_sb->s_bdev,
- iter, get_block,
- ocfs2_dio_end_io, 0);
-}
-
static int ocfs2_iomap_alloc(struct inode *inode, struct ocfs2_map_block *map,
unsigned int flags)
{
@@ -3045,7 +2635,6 @@ const struct address_space_operations ocfs2_aops = {
.write_begin = ocfs2_write_begin,
.write_end = ocfs2_write_end,
.bmap = ocfs2_bmap,
- .direct_IO = ocfs2_direct_IO,
.invalidate_folio = block_invalidate_folio,
.release_folio = ocfs2_release_folio,
.migrate_folio = buffer_migrate_folio,
diff --git a/fs/ocfs2/aops.h b/fs/ocfs2/aops.h
index 8dd6edd7c1a1..484b25d749d7 100644
--- a/fs/ocfs2/aops.h
+++ b/fs/ocfs2/aops.h
@@ -7,6 +7,12 @@
#define OCFS2_AOPS_H
#include <linux/fs.h>
+#include <linux/iomap.h>
+
+extern const struct iomap_ops ocfs2_iomap_ops;
+extern const struct iomap_dio_ops ocfs2_iomap_dio_ops_r_pr;
+extern const struct iomap_dio_ops ocfs2_iomap_dio_ops_w_pr;
+extern const struct iomap_dio_ops ocfs2_iomap_dio_ops_w_ex;
int ocfs2_map_folio_blocks(struct folio *folio, u64 *p_blkno,
struct inode *inode, unsigned int from,
@@ -44,34 +50,5 @@ int ocfs2_get_block(struct inode *inode, sector_t iblock,
struct buffer_head *bh_result, int create);
int ocfs2_map_blocks(struct inode *inode, struct ocfs2_map_block *map,
int flags);
-/* all ocfs2_dio_end_io()'s fault */
-#define ocfs2_iocb_is_rw_locked(iocb) \
- test_bit(0, (unsigned long *)&iocb->private)
-static inline void ocfs2_iocb_set_rw_locked(struct kiocb *iocb, int level)
-{
- set_bit(0, (unsigned long *)&iocb->private);
- if (level)
- set_bit(1, (unsigned long *)&iocb->private);
- else
- clear_bit(1, (unsigned long *)&iocb->private);
-}
-
-/*
- * Using a named enum representing lock types in terms of #N bit stored in
- * iocb->private, which is going to be used for communication between
- * ocfs2_dio_end_io() and ocfs2_file_write/read_iter().
- */
-enum ocfs2_iocb_lock_bits {
- OCFS2_IOCB_RW_LOCK = 0,
- OCFS2_IOCB_RW_LOCK_LEVEL,
- OCFS2_IOCB_NUM_LOCKS
-};
-
-#define ocfs2_iocb_init_rw_locked(iocb) \
- (iocb->private = NULL)
-#define ocfs2_iocb_clear_rw_locked(iocb) \
- clear_bit(OCFS2_IOCB_RW_LOCK, (unsigned long *)&iocb->private)
-#define ocfs2_iocb_rw_locked_level(iocb) \
- test_bit(OCFS2_IOCB_RW_LOCK_LEVEL, (unsigned long *)&iocb->private)
#endif /* OCFS2_FILE_H */
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 2d78f2863acf..a54f92633582 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -92,6 +92,13 @@ static int ocfs2_file_open(struct inode *inode, struct file *file)
file->f_path.dentry->d_name.len,
file->f_path.dentry->d_name.name, mode);
+ /*
+ * Direct I/O is served through iomap_dio_rw() from
+ * ocfs2_file_{read,write}_iter() rather than an a_ops->direct_IO
+ * method, so advertise O_DIRECT capability explicitly here.
+ */
+ file->f_mode |= FMODE_CAN_ODIRECT;
+
if (file->f_mode & FMODE_WRITE) {
status = dquot_initialize(inode);
if (status)
@@ -1171,9 +1178,9 @@ int ocfs2_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
size_change = S_ISREG(inode->i_mode) && attr->ia_valid & ATTR_SIZE;
if (size_change) {
/*
- * Here we should wait dio to finish before inode lock
- * to avoid a deadlock between ocfs2_setattr() and
- * ocfs2_dio_end_io_write()
+ * Here we should wait for in-flight direct I/O to finish
+ * before taking the inode lock, to avoid a deadlock between
+ * ocfs2_setattr() and direct I/O completion.
*/
inode_dio_wait(inode);
diff --git a/fs/ocfs2/ocfs2.h b/fs/ocfs2/ocfs2.h
index b228b10b5e71..095f7ae5dded 100644
--- a/fs/ocfs2/ocfs2.h
+++ b/fs/ocfs2/ocfs2.h
@@ -549,11 +549,6 @@ struct ocfs2_map_block {
unsigned int flags;
};
-extern const struct iomap_ops ocfs2_iomap_ops;
-extern const struct iomap_dio_ops ocfs2_iomap_dio_ops_r_pr;
-extern const struct iomap_dio_ops ocfs2_iomap_dio_ops_w_pr;
-extern const struct iomap_dio_ops ocfs2_iomap_dio_ops_w_ex;
-
/* Flags used by ocfs2_map_blocks() */
#define OCFS2_GET_BLOCKS_CREATE (0x0001)
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [RFC PATCH v2 4/4] ocfs2: remove legacy blockdev direct-IO path APIs
2026-07-27 6:18 ` [RFC PATCH v2 4/4] ocfs2: remove legacy blockdev direct-IO path APIs Heming Zhao
@ 2026-07-28 4:19 ` Christoph Hellwig
2026-07-28 4:25 ` Heming Zhao
2026-07-28 6:11 ` Joseph Qi
1 sibling, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2026-07-28 4:19 UTC (permalink / raw)
To: Heming Zhao; +Cc: joseph.qi, mark, jlbec, hch, ocfs2-devel, linux-kernel
After this __blockdev_direct_IO is unused except to implement
blockdev_direct_IO. So please fold blockdev_direct_IO into
__blockdev_direct_IO and remove all the code that is now unused
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v2 4/4] ocfs2: remove legacy blockdev direct-IO path APIs
2026-07-28 4:19 ` Christoph Hellwig
@ 2026-07-28 4:25 ` Heming Zhao
0 siblings, 0 replies; 14+ messages in thread
From: Heming Zhao @ 2026-07-28 4:25 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: joseph.qi, mark, jlbec, ocfs2-devel, linux-kernel
On 7/28/26 12:19, Christoph Hellwig wrote:
> After this __blockdev_direct_IO is unused except to implement
> blockdev_direct_IO. So please fold blockdev_direct_IO into
> __blockdev_direct_IO and remove all the code that is now unused
Thanks for all your kind review comments.
I will carefully revise the patch set before sending v3.
- Heming
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v2 4/4] ocfs2: remove legacy blockdev direct-IO path APIs
2026-07-27 6:18 ` [RFC PATCH v2 4/4] ocfs2: remove legacy blockdev direct-IO path APIs Heming Zhao
2026-07-28 4:19 ` Christoph Hellwig
@ 2026-07-28 6:11 ` Joseph Qi
1 sibling, 0 replies; 14+ messages in thread
From: Joseph Qi @ 2026-07-28 6:11 UTC (permalink / raw)
To: Heming Zhao; +Cc: mark, jlbec, hch, ocfs2-devel, linux-kernel
On 7/27/26 2:18 PM, Heming Zhao wrote:
> Since the DIO path already migrated from buffer_head mode (by using
> a_ops->direct_IO method) to iomap mode, the VFS no longer sets
> FMODE_CAN_ODIRECT automatically, so ocfs2_file_open() sets it
> explicitly.
>
> The now-unused blockdev get_block/end_io machinery
> (ocfs2_lock_get_block(), ocfs2_dio_wr_get_block(), ocfs2_dio_end_io(),
> ocfs2_dio_end_io_write(), and the ocfs2_dio_write_ctxt helpers) is removed,
> and OCFS2_FS Kconfig removes LEGACY_DIRECT_IO.
>
> Co-developed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Signed-off-by: Heming Zhao <heming.zhao@suse.com>
> ---
> fs/ocfs2/Kconfig | 1 -
> fs/ocfs2/aops.c | 411 -----------------------------------------------
> fs/ocfs2/aops.h | 35 +---
> fs/ocfs2/file.c | 13 +-
> fs/ocfs2/ocfs2.h | 5 -
> 5 files changed, 16 insertions(+), 449 deletions(-)
>
> diff --git a/fs/ocfs2/Kconfig b/fs/ocfs2/Kconfig
> index bf1678a5eb01..74bc2008ef0b 100644
> --- a/fs/ocfs2/Kconfig
> +++ b/fs/ocfs2/Kconfig
> @@ -9,7 +9,6 @@ config OCFS2_FS
> select QUOTA_TREE
> select FS_IOMAP
> select FS_POSIX_ACL
> - select LEGACY_DIRECT_IO
> help
> OCFS2 is a general purpose extent based shared disk cluster file
> system with many similarities to ext3. It supports 64 bit inode
> diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
> index 9a079436c9c0..3e4a7043074a 100644
> --- a/fs/ocfs2/aops.c
> +++ b/fs/ocfs2/aops.c
> @@ -114,19 +114,6 @@ static int ocfs2_symlink_get_block(struct inode *inode, sector_t iblock,
> return err;
> }
>
> -static int ocfs2_lock_get_block(struct inode *inode, sector_t iblock,
> - struct buffer_head *bh_result, int create)
> -{
> - int ret = 0;
> - struct ocfs2_inode_info *oi = OCFS2_I(inode);
> -
> - down_read(&oi->ip_alloc_sem);
> - ret = ocfs2_get_block(inode, iblock, bh_result, create);
> - up_read(&oi->ip_alloc_sem);
> -
> - return ret;
> -}
> -
> int ocfs2_map_blocks(struct inode *inode, struct ocfs2_map_block *map,
> int flags)
> {
> @@ -2170,204 +2157,6 @@ static int ocfs2_write_end(const struct kiocb *iocb,
> return ret;
> }
>
> -struct ocfs2_dio_write_ctxt {
> - struct list_head dw_zero_list;
> - unsigned dw_zero_count;
> - int dw_orphaned;
> - pid_t dw_writer_pid;
> -};
> -
> -static struct ocfs2_dio_write_ctxt *
> -ocfs2_dio_alloc_write_ctx(struct buffer_head *bh, int *alloc)
> -{
> - struct ocfs2_dio_write_ctxt *dwc = NULL;
> -
> - if (bh->b_private)
> - return bh->b_private;
> -
> - dwc = kmalloc_obj(struct ocfs2_dio_write_ctxt, GFP_NOFS);
> - if (dwc == NULL)
> - return NULL;
> - INIT_LIST_HEAD(&dwc->dw_zero_list);
> - dwc->dw_zero_count = 0;
> - dwc->dw_orphaned = 0;
> - dwc->dw_writer_pid = task_pid_nr(current);
> - bh->b_private = dwc;
> - *alloc = 1;
> -
> - return dwc;
> -}
> -
> -static void ocfs2_dio_free_write_ctx(struct inode *inode,
> - struct ocfs2_dio_write_ctxt *dwc)
> -{
> - ocfs2_free_unwritten_list(inode, &dwc->dw_zero_list);
> - kfree(dwc);
> -}
> -
> -/*
> - * TODO: Make this into a generic get_blocks function.
> - *
> - * From do_direct_io in direct-io.c:
> - * "So what we do is to permit the ->get_blocks function to populate
> - * bh.b_size with the size of IO which is permitted at this offset and
> - * this i_blkbits."
> - *
> - * This function is called directly from get_more_blocks in direct-io.c.
> - *
> - * called like this: dio->get_blocks(dio->inode, fs_startblk,
> - * fs_count, map_bh, dio->rw == WRITE);
> - */
> -static int ocfs2_dio_wr_get_block(struct inode *inode, sector_t iblock,
> - struct buffer_head *bh_result, int create)
> -{
> - struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
> - struct ocfs2_inode_info *oi = OCFS2_I(inode);
> - struct ocfs2_write_ctxt *wc;
> - struct ocfs2_write_cluster_desc *desc = NULL;
> - struct ocfs2_dio_write_ctxt *dwc = NULL;
> - struct buffer_head *di_bh = NULL;
> - u64 p_blkno = 0;
> - unsigned int i_blkbits = inode->i_sb->s_blocksize_bits;
> - loff_t pos = iblock << i_blkbits;
> - sector_t endblk = (i_size_read(inode) - 1) >> i_blkbits;
> - unsigned len, total_len = bh_result->b_size;
> - int ret = 0, first_get_block = 0;
> -
> - len = osb->s_clustersize - (pos & (osb->s_clustersize - 1));
> - len = min(total_len, len);
> -
> - /*
> - * bh_result->b_size is count in get_more_blocks according to write
> - * "pos" and "end", we need map twice to return different buffer state:
> - * 1. area in file size, not set NEW;
> - * 2. area out file size, set NEW.
> - *
> - * iblock endblk
> - * |--------|---------|---------|---------
> - * |<-------area in file------->|
> - */
> -
> - if ((iblock <= endblk) &&
> - ((iblock + ((len - 1) >> i_blkbits)) > endblk))
> - len = (endblk - iblock + 1) << i_blkbits;
> -
> - mlog(0, "get block of %llu at %llu:%u req %u\n",
> - inode->i_ino, pos, len, total_len);
> -
> - /*
> - * Because we need to change file size in ocfs2_dio_end_io_write(), or
> - * we may need to add it to orphan dir. So can not fall to fast path
> - * while file size will be changed.
> - */
> - if (pos + total_len <= i_size_read(inode)) {
> -
> - /* This is the fast path for re-write. */
> - ret = ocfs2_lock_get_block(inode, iblock, bh_result, create);
> - if (buffer_mapped(bh_result) &&
> - !buffer_new(bh_result) &&
> - ret == 0)
> - goto out;
> -
> - /* Clear state set by ocfs2_get_block. */
> - bh_result->b_state = 0;
> - }
> -
> - dwc = ocfs2_dio_alloc_write_ctx(bh_result, &first_get_block);
> - if (unlikely(dwc == NULL)) {
> - ret = -ENOMEM;
> - mlog_errno(ret);
> - goto out;
> - }
> -
> - if (ocfs2_clusters_for_bytes(inode->i_sb, pos + total_len) >
> - ocfs2_clusters_for_bytes(inode->i_sb, i_size_read(inode)) &&
> - !dwc->dw_orphaned) {
> - /*
> - * when we are going to alloc extents beyond file size, add the
> - * inode to orphan dir, so we can recall those spaces when
> - * system crashed during write.
> - */
> - ret = ocfs2_add_inode_to_orphan(osb, inode);
> - if (ret < 0) {
> - mlog_errno(ret);
> - goto out;
> - }
> - dwc->dw_orphaned = 1;
> - }
> -
> - ret = ocfs2_inode_lock(inode, &di_bh, 1);
> - if (ret) {
> - mlog_errno(ret);
> - goto out;
> - }
> -
> - down_write(&oi->ip_alloc_sem);
> -
> - if (first_get_block) {
> - if (ocfs2_sparse_alloc(osb))
> - ret = ocfs2_zero_tail(inode, di_bh, pos);
> - else
> - ret = ocfs2_expand_nonsparse_inode(inode, di_bh, pos,
> - total_len, NULL);
> - if (ret < 0) {
> - mlog_errno(ret);
> - goto unlock;
> - }
> - }
> -
> - ret = ocfs2_write_begin_nolock(inode->i_mapping, pos, len,
> - OCFS2_WRITE_DIRECT, NULL,
> - (void **)&wc, di_bh, NULL);
> - if (ret) {
> - mlog_errno(ret);
> - goto unlock;
> - }
> -
> - desc = &wc->w_desc[0];
> -
> - p_blkno = ocfs2_clusters_to_blocks(inode->i_sb, desc->c_phys);
> - BUG_ON(p_blkno == 0);
> - p_blkno += iblock & (u64)(ocfs2_clusters_to_blocks(inode->i_sb, 1) - 1);
> -
> - map_bh(bh_result, inode->i_sb, p_blkno);
> - bh_result->b_size = len;
> - if (desc->c_needs_zero)
> - set_buffer_new(bh_result);
> -
> - if (iblock > endblk)
> - set_buffer_new(bh_result);
> -
> - /* May sleep in end_io. It should not happen in a irq context. So defer
> - * it to dio work queue. */
> - set_buffer_defer_completion(bh_result);
> -
> - if (!list_empty(&wc->w_unwritten_list)) {
> - struct ocfs2_unwritten_extent *ue = NULL;
> -
> - ue = list_first_entry(&wc->w_unwritten_list,
> - struct ocfs2_unwritten_extent,
> - ue_node);
> - BUG_ON(ue->ue_cpos != desc->c_cpos);
> - /* The physical address may be 0, fill it. */
> - ue->ue_phys = desc->c_phys;
> -
> - list_splice_tail_init(&wc->w_unwritten_list, &dwc->dw_zero_list);
> - dwc->dw_zero_count += wc->w_unwritten_count;
> - }
> -
> - ret = ocfs2_write_end_nolock(inode->i_mapping, pos, len, len, wc);
> - BUG_ON(ret != len);
> - ret = 0;
> -unlock:
> - up_write(&oi->ip_alloc_sem);
> - ocfs2_inode_unlock(inode, 1);
> - brelse(di_bh);
> -out:
> - return ret;
> -}
> -
> -/* copy from ocfs2_dio_wr_get_block */
> static int ocfs2_dio_wr_map_blocks(struct inode *inode,
> struct ocfs2_map_block *map, int create)
> {
> @@ -2509,205 +2298,6 @@ static int ocfs2_dio_wr_map_blocks(struct inode *inode,
> return ret;
> }
>
> -static int ocfs2_dio_end_io_write(struct inode *inode,
> - struct ocfs2_dio_write_ctxt *dwc,
> - loff_t offset,
> - ssize_t bytes)
> -{
> - struct ocfs2_cached_dealloc_ctxt dealloc;
> - struct ocfs2_extent_tree et;
> - struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
> - struct ocfs2_inode_info *oi = OCFS2_I(inode);
> - struct ocfs2_unwritten_extent *ue = NULL;
> - struct buffer_head *di_bh = NULL;
> - struct ocfs2_dinode *di;
> - struct ocfs2_alloc_context *data_ac = NULL;
> - struct ocfs2_alloc_context *meta_ac = NULL;
> - handle_t *handle = NULL;
> - loff_t end = offset + bytes;
> - int ret = 0, credits = 0, batch = 0;
> -
> - ocfs2_init_dealloc_ctxt(&dealloc);
> -
> - /* We do clear unwritten, delete orphan, change i_size here. If neither
> - * of these happen, we can skip all this. */
> - if (list_empty(&dwc->dw_zero_list) &&
> - end <= i_size_read(inode) &&
> - !dwc->dw_orphaned)
> - goto out;
> -
> - ret = ocfs2_inode_lock(inode, &di_bh, 1);
> - if (ret < 0) {
> - mlog_errno(ret);
> - goto out;
> - }
> -
> - down_write(&oi->ip_alloc_sem);
> - di = (struct ocfs2_dinode *)di_bh->b_data;
> -
> - ocfs2_init_dinode_extent_tree(&et, INODE_CACHE(inode), di_bh);
> -
> - /* Attach dealloc with extent tree in case that we may reuse extents
> - * which are already unlinked from current extent tree due to extent
> - * rotation and merging.
> - */
> - et.et_dealloc = &dealloc;
> -
> - ret = ocfs2_lock_allocators(inode, &et, 0, dwc->dw_zero_count*2,
> - &data_ac, &meta_ac);
> - if (ret) {
> - mlog_errno(ret);
> - goto unlock;
> - }
> -
> - credits = ocfs2_calc_extend_credits(inode->i_sb, &di->id2.i_list);
> -
> - list_for_each_entry(ue, &dwc->dw_zero_list, ue_node) {
> - if (!handle) {
> - handle = ocfs2_start_trans(osb, credits);
> - if (IS_ERR(handle)) {
> - ret = PTR_ERR(handle);
> - mlog_errno(ret);
> - goto unlock;
> - }
> - ret = ocfs2_journal_access_di(handle, INODE_CACHE(inode), di_bh,
> - OCFS2_JOURNAL_ACCESS_WRITE);
> - if (ret) {
> - mlog_errno(ret);
> - goto commit;
> - }
> - }
> - ret = ocfs2_assure_trans_credits(handle, credits);
> - if (ret < 0) {
> - mlog_errno(ret);
> - goto commit;
> - }
> - ret = ocfs2_mark_extent_written(inode, &et, handle,
> - ue->ue_cpos, 1,
> - ue->ue_phys,
> - meta_ac, &dealloc);
> - if (ret < 0) {
> - mlog_errno(ret);
> - goto commit;
> - }
> -
> - if (++batch == OCFS2_DIO_MARK_EXTENT_BATCH) {
> - ocfs2_commit_trans(osb, handle);
> - handle = NULL;
> - batch = 0;
> - }
> - }
> -
> - if (end > i_size_read(inode)) {
> - if (!handle) {
> - handle = ocfs2_start_trans(osb, credits);
> - if (IS_ERR(handle)) {
> - ret = PTR_ERR(handle);
> - mlog_errno(ret);
> - goto unlock;
> - }
> - }
> - ret = ocfs2_set_inode_size(handle, inode, di_bh, end);
> - if (ret < 0)
> - mlog_errno(ret);
> - }
> -
> -commit:
> - if (handle)
> - ocfs2_commit_trans(osb, handle);
> -unlock:
> - up_write(&oi->ip_alloc_sem);
> -
> - if (data_ac) {
> - ocfs2_free_alloc_context(data_ac);
> - data_ac = NULL;
> - }
> - if (meta_ac) {
> - ocfs2_free_alloc_context(meta_ac);
> - meta_ac = NULL;
> - }
> -
> - /* everything looks good, let's start the cleanup */
> - if (!ret && dwc->dw_orphaned) {
> - BUG_ON(dwc->dw_writer_pid != task_pid_nr(current));
> -
> - ret = ocfs2_del_inode_from_orphan(osb, inode, di_bh, 0, 0);
> - if (ret < 0)
> - mlog_errno(ret);
> - }
> - ocfs2_inode_unlock(inode, 1);
> - brelse(di_bh);
> -out:
> - ocfs2_run_deallocs(osb, &dealloc);
> - ocfs2_dio_free_write_ctx(inode, dwc);
> -
> - return ret;
> -}
> -
> -/*
> - * ocfs2_dio_end_io is called by the dio core when a dio is finished. We're
> - * particularly interested in the aio/dio case. We use the rw_lock DLM lock
> - * to protect io on one node from truncation on another.
> - */
> -static int ocfs2_dio_end_io(struct kiocb *iocb,
> - loff_t offset,
> - ssize_t bytes,
> - void *private)
> -{
> - struct inode *inode = file_inode(iocb->ki_filp);
> - int level;
> - int ret = 0;
> -
> - /* this io's submitter should not have unlocked this before we could */
> - BUG_ON(!ocfs2_iocb_is_rw_locked(iocb));
> -
> - if (bytes <= 0)
> - mlog_ratelimited(ML_ERROR, "Direct IO failed, bytes = %lld",
> - (long long)bytes);
> - if (private) {
> - if (bytes > 0)
> - ret = ocfs2_dio_end_io_write(inode, private, offset,
> - bytes);
> - else
> - ocfs2_dio_free_write_ctx(inode, private);
> - }
> -
> - ocfs2_iocb_clear_rw_locked(iocb);
> -
> - level = ocfs2_iocb_rw_locked_level(iocb);
> - ocfs2_rw_unlock(inode, level);
> - return ret;
> -}
> -
> -static ssize_t ocfs2_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
> -{
> - struct file *file = iocb->ki_filp;
> - struct inode *inode = file->f_mapping->host;
> - struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
> - get_block_t *get_block;
> -
> - /*
> - * Fallback to buffered I/O if we see an inode without
> - * extents.
> - */
> - if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL)
> - return 0;
> -
> - /* Fallback to buffered I/O if we do not support append dio. */
> - if (iocb->ki_pos + iter->count > i_size_read(inode) &&
> - !ocfs2_supports_append_dio(osb))
> - return 0;
> -
> - if (iov_iter_rw(iter) == READ)
> - get_block = ocfs2_lock_get_block;
> - else
> - get_block = ocfs2_dio_wr_get_block;
> -
> - return __blockdev_direct_IO(iocb, inode, inode->i_sb->s_bdev,
> - iter, get_block,
> - ocfs2_dio_end_io, 0);
> -}
> -
> static int ocfs2_iomap_alloc(struct inode *inode, struct ocfs2_map_block *map,
> unsigned int flags)
> {
> @@ -3045,7 +2635,6 @@ const struct address_space_operations ocfs2_aops = {
> .write_begin = ocfs2_write_begin,
> .write_end = ocfs2_write_end,
> .bmap = ocfs2_bmap,
> - .direct_IO = ocfs2_direct_IO,
> .invalidate_folio = block_invalidate_folio,
> .release_folio = ocfs2_release_folio,
> .migrate_folio = buffer_migrate_folio,
> diff --git a/fs/ocfs2/aops.h b/fs/ocfs2/aops.h
> index 8dd6edd7c1a1..484b25d749d7 100644
> --- a/fs/ocfs2/aops.h
> +++ b/fs/ocfs2/aops.h
> @@ -7,6 +7,12 @@
> #define OCFS2_AOPS_H
>
> #include <linux/fs.h>
> +#include <linux/iomap.h>
> +
> +extern const struct iomap_ops ocfs2_iomap_ops;
> +extern const struct iomap_dio_ops ocfs2_iomap_dio_ops_r_pr;
> +extern const struct iomap_dio_ops ocfs2_iomap_dio_ops_w_pr;
> +extern const struct iomap_dio_ops ocfs2_iomap_dio_ops_w_ex;
>
> int ocfs2_map_folio_blocks(struct folio *folio, u64 *p_blkno,
> struct inode *inode, unsigned int from,
> @@ -44,34 +50,5 @@ int ocfs2_get_block(struct inode *inode, sector_t iblock,
> struct buffer_head *bh_result, int create);
> int ocfs2_map_blocks(struct inode *inode, struct ocfs2_map_block *map,
> int flags);
> -/* all ocfs2_dio_end_io()'s fault */
> -#define ocfs2_iocb_is_rw_locked(iocb) \
> - test_bit(0, (unsigned long *)&iocb->private)
> -static inline void ocfs2_iocb_set_rw_locked(struct kiocb *iocb, int level)
> -{
> - set_bit(0, (unsigned long *)&iocb->private);
> - if (level)
> - set_bit(1, (unsigned long *)&iocb->private);
> - else
> - clear_bit(1, (unsigned long *)&iocb->private);
> -}
> -
> -/*
> - * Using a named enum representing lock types in terms of #N bit stored in
> - * iocb->private, which is going to be used for communication between
> - * ocfs2_dio_end_io() and ocfs2_file_write/read_iter().
> - */
> -enum ocfs2_iocb_lock_bits {
> - OCFS2_IOCB_RW_LOCK = 0,
> - OCFS2_IOCB_RW_LOCK_LEVEL,
> - OCFS2_IOCB_NUM_LOCKS
> -};
> -
> -#define ocfs2_iocb_init_rw_locked(iocb) \
> - (iocb->private = NULL)
> -#define ocfs2_iocb_clear_rw_locked(iocb) \
> - clear_bit(OCFS2_IOCB_RW_LOCK, (unsigned long *)&iocb->private)
> -#define ocfs2_iocb_rw_locked_level(iocb) \
> - test_bit(OCFS2_IOCB_RW_LOCK_LEVEL, (unsigned long *)&iocb->private)
>
> #endif /* OCFS2_FILE_H */
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index 2d78f2863acf..a54f92633582 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -92,6 +92,13 @@ static int ocfs2_file_open(struct inode *inode, struct file *file)
> file->f_path.dentry->d_name.len,
> file->f_path.dentry->d_name.name, mode);
>
> + /*
> + * Direct I/O is served through iomap_dio_rw() from
> + * ocfs2_file_{read,write}_iter() rather than an a_ops->direct_IO
> + * method, so advertise O_DIRECT capability explicitly here.
> + */
> + file->f_mode |= FMODE_CAN_ODIRECT;
This unconditionally set may include inodes with inline data.
Though it will handle the fallback in ocfs2_should_use_dio(), it looks odd
and slightly misleading to userspace.
> +
> if (file->f_mode & FMODE_WRITE) {
> status = dquot_initialize(inode);
> if (status)
> @@ -1171,9 +1178,9 @@ int ocfs2_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> size_change = S_ISREG(inode->i_mode) && attr->ia_valid & ATTR_SIZE;
> if (size_change) {
> /*
> - * Here we should wait dio to finish before inode lock
> - * to avoid a deadlock between ocfs2_setattr() and
> - * ocfs2_dio_end_io_write()
> + * Here we should wait for in-flight direct I/O to finish
> + * before taking the inode lock, to avoid a deadlock between
> + * ocfs2_setattr() and direct I/O completion.
> */
> inode_dio_wait(inode);
>
> diff --git a/fs/ocfs2/ocfs2.h b/fs/ocfs2/ocfs2.h
> index b228b10b5e71..095f7ae5dded 100644
> --- a/fs/ocfs2/ocfs2.h
> +++ b/fs/ocfs2/ocfs2.h
> @@ -549,11 +549,6 @@ struct ocfs2_map_block {
> unsigned int flags;
> };
>
> -extern const struct iomap_ops ocfs2_iomap_ops;
> -extern const struct iomap_dio_ops ocfs2_iomap_dio_ops_r_pr;
> -extern const struct iomap_dio_ops ocfs2_iomap_dio_ops_w_pr;
> -extern const struct iomap_dio_ops ocfs2_iomap_dio_ops_w_ex;
> -
> /* Flags used by ocfs2_map_blocks() */
> #define OCFS2_GET_BLOCKS_CREATE (0x0001)
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v2 0/4] migrates ocfs2 DIO from buffer_head to iomap
2026-07-27 6:17 [RFC PATCH v2 0/4] migrates ocfs2 DIO from buffer_head to iomap Heming Zhao
` (3 preceding siblings ...)
2026-07-27 6:18 ` [RFC PATCH v2 4/4] ocfs2: remove legacy blockdev direct-IO path APIs Heming Zhao
@ 2026-07-28 4:11 ` Christoph Hellwig
4 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2026-07-28 4:11 UTC (permalink / raw)
To: Heming Zhao; +Cc: joseph.qi, mark, jlbec, hch, ocfs2-devel, linux-kernel
s/migrates/migrate/
^ permalink raw reply [flat|nested] 14+ messages in thread