lkml.org 
[lkml]   [2018]   [Jun]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH V6 00/30] block: support multipage bvec
On Mon, Jun 11, 2018 at 09:48:06AM -0700, Christoph Hellwig wrote:
> І think the new naming scheme in this series is a nightmare. It
> confuses the heck out of me, and that is despite knowing many bits of
> the block layer inside out, and reviewing previous series.

In V5, there isn't such issue, since bio_for_each_segment* is renamed
into bio_for_each_page* first before doing the change.

>
> I think we need to take a step back and figure out what names what we
> want in the end, and how we get there separately.

Right, I agree, last year I told people that naming may be the biggest
issue for this patchset.

>
> For the end result using bio_for_each_page in some form for the per-page
> iteration seems like the only sensible idea, as that is what it does.

Yeah, I agree, but except for renaming bio_for_each_segment* into
bio_for_each_page* or whatever first, I don't see any way to deal with
it cleanly.

Seems Jens isn't fine with the big renaming, then I follow the suggestion
of taking 'chunk' for representing multipage bvec in V6.

>
> For the bio-vec iteration I'm fine with either bio_for_each_bvec as that
> exactly explains what it does, or bio_for_each_segment to keep the
> change at a minimum.

If bio_for_each_segment() is fine, that is basically what this patch is doing,
then could you share me what the actual naming issue is in V6? And
basically the name of 'chunk' is introduced for multipage bvec.

>
> And in terms of how to get there: maybe we need to move all the drivers
> and file systems to the new names first before the actual changes to
> document all the intent.

That is exactly what I have done in V5, but that way is refused.

Guys, so what can we do to make progress for this naming issue?


Thanks,
Ming

\
 
 \ /
  Last update: 2018-06-12 05:44    [W:0.310 / U:0.280 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site