Date:   Fri, 10 Mar 2023 04:05:08 +0000
From:   Al Viro <viro@zeniv.linux.org.uk>
To:     Yangtao Li <frank.li@vivo.com>
Cc:     xiang@kernel.org, chao@kernel.org, huyue2@coolpad.com,
        jefflexu@linux.alibaba.com, tytso@mit.edu,
        adilger.kernel@dilger.ca, rpeterso@redhat.com, agruenba@redhat.com,
        mark@fasheh.com, jlbec@evilplan.org, joseph.qi@linux.alibaba.com,
        brauner@kernel.org, linux-erofs@lists.ozlabs.org,
        linux-kernel@vger.kernel.org, linux-ext4@vger.kernel.org,
        cluster-devel@redhat.com, ocfs2-devel@oss.oracle.com,
        linux-fsdevel@vger.kernel.org
Subject: Re: erofs: convert to use i_blockmask()
Message-ID: <20230310040508.GN3390869@ZenIV>
References: <20230310031547.GD3390869@ZenIV>
 <20230310035121.56591-1-frank.li@vivo.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230310035121.56591-1-frank.li@vivo.com>
Sender: Al Viro <viro@ftp.linux.org.uk>
Precedence: bulk

On Fri, Mar 10, 2023 at 11:51:21AM +0800, Yangtao Li wrote:
> Hi AI,
> 
> > Umm...  What's the branchpoint for that series?
> > Not the mainline - there we have i_blocksize() open-coded...
> 
> Sorry, I'm based on the latest branch of the erofs repository.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs.git/log/?h=dev-test
> 
> I think I can resend based on mainline.
> 
> > Umm...  That actually asks for DIV_ROUND_UP(i_size_read_inode(), i_blocksize(inode))
> > - compiler should bloody well be able to figure out that division by (1 << n)
> > is shift down by n and it's easier to follow that way...
> 
> So it seems better to change to DIV_ROUND_UP(i_size_read_inode(), i_blocksize(inode))?
> 
> > And the fact that the value will be the same (i.e. that ->i_blkbits is never changed by ocfs2)
> > is worth mentioning in commit message...
> 
> How about the following msg?
> 
> Use i_blockmask() to simplify code. BTW convert ocfs2_is_io_unaligned
> to return bool type and the fact that the value will be the same
> (i.e. that ->i_blkbits is never changed by ocfs2).
> 
> 
> 
> A small question, whether this series of changes will be merged
> into each fs branch or all merged into vfs?

Depends.  The thing to avoid is conflicts between the trees and
convoluted commit graph.

In cases like that the usual approach is
	* put the helper into never-rebased branch - in vfs tree, in this
case; I've no real objections against the helper in question.
	* let other trees convert to the helper at leisure - merging
that never-rebased branch from vfs.git before they use the helper, of
course.  Or wait until the next cycle, for that matter...

I can pick the stuff in the areas that don't have active development,
but doing that for e.g. ext4 won't help anybody - it would only cause
headache for everyone involved down the road.  And I'd expect the gfs2
to be in the same situation...