On Thu, 2020-11-05 at 16:09 +0000, David Woodhouse wrote: > From: David Woodhouse <[email protected]> > > This ended up with an odd mix of recursion (albeit *mostly* > tail-recursion) and interation that could have been prettier. In > addition, while recursing it potentially adjusted op->count which is > used by callers to see the amount of I/O actually performed. > > Fix it by bringing nvme_build_prpl() into the normal loop using 'i' > as the offset in the op. > > Fixes: 94f0510dc ("nvme: Split requests by maximum allowed size") > Signed-off-by: David Woodhouse <[email protected]> > --- > v2: Fix my bug with checking a u16 for being < 0. > Fix the bug I inherited from commit 94f0510dc but made unconditional. > > src/hw/nvme.c | 77 ++++++++++++++++++++++----------------------------- > 1 file changed, 33 insertions(+), 44 deletions(-) >
Now, on top of that we *could* do something like this...
--- a/src/hw/nvme.c
+++ b/src/hw/nvme.c
@@ -742,6 +742,15 @@ nvme_cmd_readwrite(struct nvme_namespace *ns, struct
disk_op_s *op, int write)
blocks = blocks_remaining < max_blocks ? blocks_remaining
: max_blocks;
+ if (blocks < blocks_remaining && ns->block_size < NVME_PAGE_SIZE &&
+ !(((u32)op_buf) & (ns->block_size-1))) {
+ u32 align = ((u32)op_buf & (NVME_PAGE_SIZE - 1)) /
ns->block_size;
+ if (blocks > (max_blocks - align)) {
+ dprintf(3, "Restrain to %u blocks to align (buf %p
size %u/%u)\n", max_blocks - align, op_buf, NVME_PAGE_SIZE, ns->block_size);
+ blocks = max_blocks - align;
+ }
+ }
+
if (write) {
memcpy(ns->dma_buffer, op_buf, blocks * ns->block_size);
}
While debugging this I watched a boot sector, after being loaded at
0000:7c00, load another 63 sectors at 0000:7e00. It didn't get to use
the prpl at all, and looked something like this...
ns 1 read lba 0+1: 0
Booting from 0000:7c00
ns 1 read lba 1+8: 0
ns 1 read lba 9+8: 0
ns 1 read lba 17+8: 0
ns 1 read lba 25+8: 0
ns 1 read lba 33+8: 0
ns 1 read lba 41+8: 0
ns 1 read lba 49+8: 0
ns 1 read lba 57+7: 0
If we make an *attempt* to align it, such as the proof-of-concept shown
above, then it ends up getting back in sync:
ns 1 read lba 0+1: 0
Booting from 0000:7c00
Restrain to 1 blocks to align (buf 0x00007e00 size 4096/512)
ns 1 read lba 1+1: 0
ns 1 read lba 1+62: 0
I just don't know that I care enough about the optimisation to make it
worth the ugliness of that special case in the loop, which includes a
division.
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ SeaBIOS mailing list -- [email protected] To unsubscribe send an email to [email protected]
