On Tue, Dec 17, 2019 at 6:40 AM Jens Axboe [off-list ref] wrote:

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_ctx instead. The actor gets a const
version of that, they are not supposed to change anything in it.

Looks generally like what I expected, but when looking at the patch I
notice that the type of 'len' is crazy and wrong.

It was wrong before too, though:

-dax_iomap_actor(struct inode *inode, loff_t pos, loff_t length, void *data,

'loff_t length' is not right.

+       loff_t pos = data->pos;
+       loff_t length = pos + data->len;

And WTH is that? "pos + data->len" is not "length", that's end. And this:

        loff_t end = pos + length, done = 0;

What? Now 'end' is 'pos+length', which is 'pos+pos+data->len'.

WHAA?

More insanity. "ret" shouldn't be loff_t.

dax_iomap_rw() returns a ssize_t.

+       loff_t count = data->len;

More of this crazy things.

 iomap_zero_range(struct inode *inode, loff_t pos, loff_t len, bool *did_zero,

This was wrong before.

+struct iomap_ctx {
+       struct inode    *inode;
+       loff_t          pos;
+       loff_t          len;
+       void            *priv;
+       unsigned        flags;
+};

Please make 'len' be 'size_t' or something.

If you're on a 32-bit architecture, you shouldn't be writing more than
4GB in a go anyway.

Is there some reason for this horrible case of "let's allow 64-bit sizes?"

Because even if there is, it shouldn't be "loff_t". That's an
_offset_. Not a length.

            Linus