From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f44.google.com (mail-wr1-f44.google.com [209.85.221.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EA33C1B78F3 for ; Fri, 3 Jul 2026 01:21:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=pass smtp.client-ip=209.85.221.44 ARC-Seal:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783041677; cv=pass; b=Vjc23DsdIkjC+uDYDr5lGEfskpFsPpwSIAUpWM1wQUB5L/cEzW4teZ0r3GyszY/Bwclr1QfMVCCCpgdctWm5MZ0bPBr8g77yAffpPbaY7bLkfD9dMbmhATdLaztD+Yj2XFA/9sutiuETSuo7VRco3GjySgVRrtK58yc7Lw78DQ4= ARC-Message-Signature:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783041677; c=relaxed/simple; bh=0nVvzoF5elz2h4q+/Nfneso36ZIRcR/16AhC0hzhbdY=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=DxakCymrOG4GvQMDrWYNvTYdZxoOaTiTEnLDkEqAe2YEC10+RLL2QbV4vcv1k7ewwIzYMQYm9+RQC4jCLmpvNSzdFbg4V6+jHz442YnCDsC5h6U28HGyjJjiVNxlhqU7BTNw/kSxZWD2PXHzlgSuaqeul2xdiHZ13cuf1FY7xSw= ARC-Authentication-Results:i=2; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=lIZI8qWS; arc=pass smtp.client-ip=209.85.221.44 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="lIZI8qWS" Received: by mail-wr1-f44.google.com with SMTP id ffacd0b85a97d-47640541585so30358f8f.1 for ; Thu, 02 Jul 2026 18:21:14 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1783041673; cv=none; d=google.com; s=arc-20260327; b=Za1bRmMjgILczdXzqVY7OSS/zEx1LwrYP5tL56t22P83K5/UIQihPUE7ck/rKs7BVB w2dk9CQKyxavPh41+rD1pfxuiNzYKxv3dEKfobx9pE8s6yavhGguhm7yHzCJozedZ3Dr U1Sju9Dl8R71oxiJomD8VXN7oQ2KlNKNUP1Z/28IqIkC2EhF6yvWtKfeWLOE1KI1/o19 Ees4IAYfVcXBEZ19QsFLVJyHZsrmZ5kUoxM7aFF3IntFZlosY+LuLUlohswnc5oVy+3e PrsHApxXGFIXiRQcyjZXG1+XA4ZQTp+HPvAIU5O78ux7QMteGSOgM+iZVNbI/a75lVTP CbxQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20260327; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=W/PEiwUP7AW5qZ8cAtUeh4Al98sJv81sG43cdiuY0nk=; fh=j5aifHuE8u7AImFSGGr+P3BSf71ZTZLh/eHFkfUjaNI=; b=RJ9oABtbMdrtg2cYUhFhsVfnSmtRDL9V3XcaPF4/VRKA6JVtPuh9aCRRvJg/7cRBFr QBE2rnFyUtsajtWClevUnHfWOgMYy/o55nxp3fxu2e3RRp2m/T71AcXNICG+QpKIDqQ7 5+oWEgndYzni+g5lnrVaUFo2C+bSdRzdrthZuHTaLvKaNG0tznFIHwHv1PPv6JAwrf4J GJjwgDbc75V6e2dJv+B0aS+VlJ09XweIzmbMvPxpQiqNLoYXk5sVd2NLgFgAbzODWxVD 3iO8vuhJnEA/8OkReRvuBfiBZQPXRorz6S1di3NL6+nPe04ckIaLrXGtnAJjkXkYtEL0 wboQ==; darn=vger.kernel.org ARC-Authentication-Results: i=1; mx.google.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1783041673; x=1783646473; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=W/PEiwUP7AW5qZ8cAtUeh4Al98sJv81sG43cdiuY0nk=; b=lIZI8qWSmUfZwRi22jFhtqtAapPw8PDVIbVgqA+kjwLlSazKHR45O5rRrcxmcQVliy M5t10rv0T3KjnfObqSigxVBqvsVgEVdJK2Bodpkbposw97M3uiRx3oc2jGMCb3oYQqF6 jgzRfJZYjO4Q8d9ygExdp93z+mOKDSfziP2U8bbAfqNKP43h6S78EznL5DF3XcRHQITJ UWQBEhnNs+jXldlX2/rczDebMki7W+QB2mB/JuoGzDYBuM7h3tzb8dV/UYp5JGGkG9Y/ 1iNc+CP7Xn0D9XJABf+RvNq/xSjxibtgfv/CYl7N0udx47ToVgvOQoY1WEpYydnPwrph Kr6Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783041673; x=1783646473; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=W/PEiwUP7AW5qZ8cAtUeh4Al98sJv81sG43cdiuY0nk=; b=oMXB81te3bAEYxV2BXG0630+l4Sje6i91nqTPQuSTIRcT+dVH7q3VxG2XNdLg0fdjH jlmQc2w1a7LntHDFOckvh+S8OBVKAzP8P0lS+t141Fmir7wQDFHR9ybiN4vHwoD8xfL1 QIZzqoGUGD1/2U0RO8ZK0rUK/KzfMFDTizCn5hu4og3Q5szW4dt+KxW8j1BxA2n4bZ1m 4BUZEIupple/60C5zISqVq5WWjqjsTeZwMiueQ8qs3Bic/upg5cnkjlV8YApvqudV0D0 w2c11Rf38d7XzuCC1Et4sRwvXmLzdaiIfgAroiKEmHnOYlzbhsDKTBbRxDSWls2e/Zq0 G6gg== X-Forwarded-Encrypted: i=1; AHgh+RrNyyoXMKyejHRf5a/NYlhJd0S1jrO2uRuwSzpTlH7rhYHvF38TZyebisC4kay03WdxUfcEIcf78E/xWkQ=@vger.kernel.org X-Gm-Message-State: AOJu0YwQM+MPeh3ANuZMrJT1pAQ4gd52EstP//M3Bkk7I8Fx49a0ZyVQ bbjEf/eRaF8uefde9zokUIV4a/WdxVEULrD1u+v6b+J8+ibQHrw/oL3tk//Mn+NUv0G/2Ell5MH hU4rXpWkB5mfzYkeHnbVROE87gvI1Muw= X-Gm-Gg: AfdE7ckFZQJSfJysbd7oXz/9JF0QXXit6uFEkiThST2ONgeS/0njkqoOf/30Lo9gtma fmW70NnvYTTamoslOSQg6bx6t+9FUMsKlwrNHyIH0GTMNfOcuxHFyzWGq336QTx6MT/k2WAulN8 6aAaFZicfBxwOa0kBuUU3SXggWflSyYJGKcV5/xogvx81lc0IGp2PCJhQq3QuyOeeo/d6ARgA3T Clv5XhMH70LQRvChmE7ENAWoZXkgkJHprWRiFa/L9kj710YBjrnSBsnC6DQ2zD1GHwTSkygaJ89 HaATrmc9A2V6iO/FYIDPkgJYiV6OgCF2GuP3uChnhmMlRoTRRdKg X-Received: by 2002:a05:6000:310a:b0:46d:d6b2:3959 with SMTP id ffacd0b85a97d-477595687c3mr11592672f8f.46.1783041673292; Thu, 02 Jul 2026 18:21:13 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20260701000949.1666714-1-joannelkoong@gmail.com> <20260701000949.1666714-3-joannelkoong@gmail.com> <20260702164339.GI9392@frogsfrogsfrogs> In-Reply-To: From: Joanne Koong Date: Thu, 2 Jul 2026 18:21:00 -0700 X-Gm-Features: AVVi8CdpsRpxX2zcAJ3Qh58xlRfu6FM1kcs6vvqwCB4h9cAFLYsYD49l-klWp8s Message-ID: Subject: Re: [PATCH v2 02/18] xfs: convert iomap ops to ->iomap_next() To: "Darrick J. Wong" Cc: brauner@kernel.org, hch@lst.de, willy@infradead.org, hsiangkao@linux.alibaba.com, linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, Carlos Maiolino , open list Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, Jul 2, 2026 at 4:59=E2=80=AFPM Joanne Koong wrote: > > On Thu, Jul 2, 2026 at 9:43=E2=80=AFAM Darrick J. Wong wrote: > > > > On Tue, Jun 30, 2026 at 05:09:17PM -0700, Joanne Koong wrote: > > > Convert xfs iomap_ops to the new ->iomap_next() callback. This uses t= he > > > iomap_process() helper, which finishes the previous mapping if needed > > > and produces the next one. No functional changes are intended. > > > > > > Signed-off-by: Joanne Koong > > > --- > > > fs/xfs/xfs_file.c | 4 +- > > > fs/xfs/xfs_iomap.c | 96 +++++++++++++++++++++++++++++++++++++++++---= -- > > > 2 files changed, 88 insertions(+), 12 deletions(-) > > > > > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c > > > index 845a97c9b063..7f8bef1a9954 100644 > > > --- a/fs/xfs/xfs_file.c > > > +++ b/fs/xfs/xfs_file.c > > > @@ -857,9 +857,9 @@ xfs_file_dio_write_atomic( > > > NULL, 0); > > > > > > /* > > > - * The retry mechanism is based on the ->iomap_begin method ret= urning > > > + * The retry mechanism is based on the ->iomap_next method retu= rning > > > * -ENOPROTOOPT, which would be when the REQ_ATOMIC-based write= is not > > > - * possible. The REQ_ATOMIC-based method typically not be possi= ble if > > > + * possible. The REQ_ATOMIC-based method is typically not possi= ble if > > > * the write spans multiple extents or the disk blocks are misa= ligned. > > > */ > > > if (ret =3D=3D -ENOPROTOOPT && dops =3D=3D &xfs_direct_write_io= map_ops) { > > > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c > > > index 225c3de88d03..4fa1a5c985db 100644 > > > --- a/fs/xfs/xfs_iomap.c > > > +++ b/fs/xfs/xfs_iomap.c > > > @@ -1037,8 +1037,18 @@ xfs_direct_write_iomap_begin( > > > return error; > > > } > > > > > > +static int > > > +xfs_direct_write_iomap_next( > > > + const struct iomap_iter *iter, > > > + struct iomap *iomap, > > > + struct iomap *srcmap) > > > > > > > > Now that I see the callsites, I think the "next" name could use some > > bikeshedding . The purpose of this function is either > > > > (a) to look up the first mapping to start iterating; > > (b) to release whatever resources were attached during the current > > iteration and look up the next mapping to continue iterating; or > > (c) to decide that it's time to stop iterating. > > > > From that it seems obvious to me that xfs_direct_write_iomap_next yield= s > > iomaps for iteration. In Python those are called generator functions; > > in Rust they're called objects that implement the Iterator trait (or > > iterators for short). > > > > How about s/iomap_next/iomap_iter/ ? > > > > Then this function would be called xfs_direct_write_iomap_iter, which > > IMO is a closer description of what the function does, which is to say > > iterates iomaps for direct writes. > > > > (Yes, my brain might be polluted with thinking that "iomap next" refers > > to a major shift, in the sense of "linux next". :P) > > Thanks for sharing your thoughts. The idea makes sense to me in theory > but I think in rust and python the "iter" naming is reserved for the > iterator object itself and the method / callback for advancing and > yielding the next item in the iteration is named "next" (eg > Iterator::next in rust and __next__() in python)? I wonder if the > s/_next/_iter gets confusing with multiple things in iomap already > called iter (eg the struct iomap_iter, the iomap_iter() function). I'm > not sure, I'm happy to rename this if that's the preference. > Ahh okay, I think I see the point you were trying to make. The callsite functions are like: ssize_t iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *from= , iomap_next_fn iomap_next, const struct iomap_write_ops *write_ops, void *private); int iomap_fsverity_write(struct file *file, loff_t pos, size_t length, const void *buf, iomap_next_fn iomap_next, const struct iomap_write_ops *write_ops); void iomap_read_folio(iomap_next_fn iomap_next, struct iomap_read_folio_ctx *ctx, void *private); void iomap_readahead(iomap_next_fn iomap_next, struct iomap_read_folio_ctx *ctx, void *private); etc, so it's not clear from the "iomap_next_fn iomap_next" naming that it handles per-iteration logic, whereas renaming it to "iomap_iter_fn iomap_iter" makes it more explicit. Am I interpreting this correctly? I think this is a good idea. If no one has an objection, I'll make this change for v3. Thanks, Joanne > > > > > > +{ > > > + return iomap_process(iter, iomap, srcmap, xfs_direct_write_ioma= p_begin, > > > + NULL); > > > > But then "iomap_iter" sets up a new problem: should iomap_process have = a > > new name that goes along with that? iomap_iter is already taken for th= e > > legacy path. If we were emulating python I'd suggest iomap_iter_yield > > but this is C so we get to reinvent everything from scratch so who > > knows. > > > > --D > >