On 10/21/24 14:50, David Sterba wrote:
On Mon, Oct 14, 2024 at 06:18:27PM +0100, Mark Harmstone wrote:
Adds an io_uring command for encoded reads, using the same interface as
Add ...
the existing BTRFS_IOC_ENCODED_READ ioctl.
This is probably a good summary in a changelog but the patch is quite
long so it feels like this should be described in a more detail how it's
done. Connecting two interfaces can be done in various ways, so at least
mention that it's a simple pass through, or if there are any
complications regardign locking, object lifetime and such.
Signed-off-by: Mark Harmstone <maharmstone@xxxxxx>
...
+
+ kfree(priv->pages);
+ kfree(priv->iov);
+ kfree(priv);
+}
+
+static void btrfs_uring_read_extent_cb(void *ctx, int err)
+{
+ struct btrfs_uring_priv *priv = ctx;
+
+ *(uintptr_t*)priv->cmd->pdu = (uintptr_t)priv;
Isn't there a helper for that? Type casting should be done in justified
cases and as an exception.
FWIW, I haven't taken a look yet, but for this one, please use
io_uring_cmd_to_pdu(cmd, type), it'll check the size for you.
And there in no need to cast it to uintptr, would be much nicer
to
struct btrfs_cmd {
struct btrfs_uring_priv *priv;
};
struct btrfs_cmd *bc = io_uring_cmd_to_pdu(priv->cmd, struct btrfs_cmd);
bc->priv = priv;
You get more type checking this way. You can also wrap around
io_uring_cmd_to_pdu() into a static inline helper, if that
looks better.
...>> +
+ start = ALIGN_DOWN(pos, fs_info->sectorsize);
+ lockend = start + BTRFS_MAX_UNCOMPRESSED - 1;
+
+ ret = btrfs_encoded_read(&kiocb, &iter, &args,
+ issue_flags & IO_URING_F_NONBLOCK,
+ &cached_state, &disk_bytenr, &disk_io_size);
+ if (ret < 0 && ret != -EIOCBQUEUED)
+ goto out_free;
+
+ file_accessed(file);
+
+ if (copy_to_user((void*)(uintptr_t)cmd->sqe->addr + copy_end,
+ (char *)&args + copy_end_kernel,
Be aware, SQE data is not stable, you should assume that the user
space can change it at any moment. It should be a READ_ONCE, and
likely you don't want it to be read twice, unless you handle it /
verify values / etc. I'd recommend to save it early in the callback
and stash somewhere, e.g. into struct btrfs_cmd I mentioned above.
So many type casts again
--
Pavel Begunkov
