All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Heming Zhao <heming.zhao@suse.com>
Cc: joseph.qi@linux.alibaba.com, mark@fasheh.com, jlbec@evilplan.org,
	hch@lst.de, ocfs2-devel@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH v5 3/4] ocfs2: switch dio write path from buffer_head to iomap
Date: Fri, 6 Nov 2026 11:52:12 +0100	[thread overview]
Message-ID: <20261106105212.GC5746@lst.de> (raw)
In-Reply-To: <20261105071930.16399-4-heming.zhao@suse.com>

On Thu, Nov 05, 2026 at 03:19:27PM +0800, Heming Zhao wrote:
> Converts OCFS2's DIO write path from the legacy buffer_head
> infrastructure to the modern iomap framework.
> 
> Key modifications and designs are as follows:

That's a not very useful style of usually AI generated text.

If a change is not key it should not be mentioned here.

As should "modifications" in general - what is changed is pretty
obvious in the diff itself (and if it isn't it should be made more
clear in the code/comments).  So what should go into the commit
log really is the high-level considerations, pitfals and not quite
clear bits.  The rest is just unhelpful noise.


> +static int ocfs2_lock_map_blocks(struct inode *inode,
> +				 struct ocfs2_map_block *map, int flags)
> +{
> +	int ret = 0;
> +	struct ocfs2_inode_info *oi = OCFS2_I(inode);
> +
> +	down_read(&oi->ip_alloc_sem);
> +	ret = ocfs2_map_blocks(inode, map, flags);
> +	up_read(&oi->ip_alloc_sem);
> +
> +	return ret;
> +}

This pattern already showed up in the previous patch, why isn't the
helper added there and used for reads as well?

Also this and the next one have a very similar diffstat.  It would
help reviewers a lot if you merged the two patches and did any pure
code movements in a separate patch, so that the actual changes stick
out instead of just adding a wall of new code here, and removing
lots of old code in the next one.


  reply	other threads:[~2026-11-06 10:52 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-05  7:19 [RFC PATCH v5 0/4] migrates ocfs2 DIO from buffer_head to iomap Heming Zhao
2026-11-05  7:19 ` [RFC PATCH v5 1/4] ocfs2: Add new ocfs2_map_blocks() to introduce iomap feature Heming Zhao
2026-11-05  7:19 ` [RFC PATCH v5 2/4] ocfs2: switch DIO read path from buffer_head to iomap Heming Zhao
2026-11-06 10:48   ` Christoph Hellwig
2026-11-06 14:11     ` Heming Zhao
2026-11-05  7:19 ` [RFC PATCH v5 3/4] ocfs2: switch dio write " Heming Zhao
2026-11-06 10:52   ` Christoph Hellwig [this message]
2026-11-06 14:25     ` Heming Zhao
2026-11-05  7:19 ` [RFC PATCH v5 4/4] ocfs2: remove legacy blockdev direct-IO path APIs Heming Zhao
2026-11-06 10:46 ` [RFC PATCH v5 0/4] migrates ocfs2 DIO from buffer_head to iomap Christoph Hellwig
2026-11-06 14:09   ` Heming Zhao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261106105212.GC5746@lst.de \
    --to=hch@lst.de \
    --cc=heming.zhao@suse.com \
    --cc=jlbec@evilplan.org \
    --cc=joseph.qi@linux.alibaba.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark@fasheh.com \
    --cc=ocfs2-devel@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.