On 12/13/19 1:32 PM, Darrick J. Wong wrote:

On Thu, Dec 12, 2019 at 12:01:32PM -0700, Jens Axboe wrote:

quoted

We pass a lot of arguments to iomap_apply(), and subsequently to the
actors that it calls. In preparation for adding one more argument,
switch them to using a struct iomap_data instead. The actor gets a const
version of that, they are not supposed to change anything in it.

Signed-off-by: Jens Axboe <axboe@kernel.dk>

Looks good, only a couple of questions...

Thanks!

quoted

 fs/dax.c               |  25 +++--
 fs/iomap/apply.c       |  26 +++---
 fs/iomap/buffered-io.c | 202 +++++++++++++++++++++++++----------------
 fs/iomap/direct-io.c   |  57 +++++++-----
 fs/iomap/fiemap.c      |  48 ++++++----
 fs/iomap/seek.c        |  64 ++++++++-----
 fs/iomap/swapfile.c    |  27 +++---
 include/linux/iomap.h  |  15 ++-
 8 files changed, 278 insertions(+), 186 deletions(-)

diff --git a/fs/dax.c b/fs/dax.c
index 1f1f0201cad1..d1c32dbbdf24 100644
--- a/fs/dax.c
+++ b/fs/dax.c

@@ -1090,13 +1090,16 @@ int __dax_zero_page_range(struct block_device *bdev,
 EXPORT_SYMBOL_GPL(__dax_zero_page_range);
 
 static loff_t
-dax_iomap_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
-		struct iomap *iomap, struct iomap *srcmap)
+dax_iomap_actor(const struct iomap_data *data, struct iomap *iomap,

I wonder, is 'struct iomap_ctx' a better name for the context structure?

Yeah I think you are right, does seem like a better fit. I'll rename it
for the next version.

quoted

@@ -43,17 +44,18 @@ iomap_apply(struct inode *inode, loff_t pos, loff_t length, unsigned flags,
 	 * expose transient stale data. If the reserve fails, we can safely
 	 * back out at this point as there is nothing to undo.
 	 */
-	ret = ops->iomap_begin(inode, pos, length, flags, &iomap, &srcmap);
+	ret = ops->iomap_begin(data->inode, data->pos, data->len, data->flags,
+				&iomap, &srcmap);

...and second, what do people think about about passing "const struct
iomap_ctx *ctx" to iomap_begin and iomap_end to reduce the argument
counts there too?

(That's definitely a separate patch though, and I might just do that on
my own if nobody beats me to it...)

To be honest, I just did what I needed, but I do think it's worth
pursuing in general. The argument clutter is real.

-- 
Jens Axboe