All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Jeremy Bingham <jbingham@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	brauner@kernel.org, jkoolstra@xs4all.nl, jack@suse.cz,
	djwong@kernel.org, viro@zeniv.linux.org.uk
Subject: Re: [PATCH v4 2/3] minix: add iomap functions and definitions
Date: Sun, 20 Sep 2026 23:50:06 -0700	[thread overview]
Message-ID: <arDTnjK84EGsIeYE@infradead.org> (raw)
In-Reply-To: <CAMyBmMA5bOaGK0TfPMGpmfR6oD0k9EdS4_R3RoJ7d5QTjm0bqg@mail.gmail.com>

On Sat, Sep 19, 2026 at 10:51:32AM -0700, Jeremy Bingham wrote:
> On Fri, Sep 18, 2026 at 7:12 AM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Wed, Aug 26, 2026 at 02:41:56PM -0700, Jeremy Bingham wrote:
> > > This patch also wires iomap.c into itree_v1.c and itree_v2.c, similarly
> > > to how itree_common.c is included in those files, and exports version
> > > specific versions of minix_iomap_begin and iomap_ops.
> >
> > Can we keep the iomap code out of this double build mess?  Just define
> > some low-level helper in it, but keep everything else in an only
> > once build iomap.c
> 
> OK. I went back and forth a bunch on how to best do this, because it's *such* a mess.

Agreed.

> I had a lot of reservations about the way I had done it, but
> it's so all around
> messy I ended up there. When I refactor it I'll either move that function to
> itree_common.c or craft a way to get that depth information out.

It might be worthwhile to kill this mess off first.  AFAICS it was
written at a time when branch predictors where much worse than
today.  So having a branch for te two cases much lower down in the code
should be just fine with modern CPUs. 

> 
> > > Also updates 'unsigned' to 'unsigned int' a few places that got picked
> > > up by checkpatch.pl.
> >
> > If you want to do these cleanups please split them into a separate
> > patch.
> 
> Noted. I did submit a patch like that back in I think late June or
> early July, but got
> told (politely) to knock it off. I had kept these cleanups because of
> checkpatch.pl
> griping, but as you said below there's no technical downside to leaving it.

checkpatch.pl is a little weird sometimes unfortunately.


  reply	other threads:[~2026-09-21  6:50 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-26 21:41 [PATCH v4 0/3] minix: convert to iomap Jeremy Bingham
2026-08-26 21:41 ` [PATCH v4 1/3] iomap: add iomap_symlink_write Jeremy Bingham
2026-09-18 13:48   ` Christoph Hellwig
2026-08-26 21:41 ` [PATCH v4 2/3] minix: add iomap functions and definitions Jeremy Bingham
2026-09-18 14:12   ` Christoph Hellwig
2026-09-19 17:51     ` Jeremy Bingham
2026-09-21  6:50       ` Christoph Hellwig [this message]
2026-09-21 18:16         ` Jeremy Bingham
2026-08-26 21:41 ` [PATCH v4 3/3] minix: finish wiring in iomap functions Jeremy Bingham
2026-09-18 15:10   ` Christoph Hellwig
2026-09-19 18:02     ` Jeremy Bingham

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=arDTnjK84EGsIeYE@infradead.org \
    --to=hch@infradead.org \
    --cc=brauner@kernel.org \
    --cc=djwong@kernel.org \
    --cc=jack@suse.cz \
    --cc=jbingham@gmail.com \
    --cc=jkoolstra@xs4all.nl \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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.