---- On Wed, 11 Sep 2024 17:56:37 +0100 Robert Beckett wrote --- > ---- On Tue, 10 Sep 2024 18:53:23 +0100 Keith Busch wrote --- > > On Tue, Sep 10, 2024 at 06:27:55PM +0100, Robert Beckett wrote: > > > nvme.io_queue_depth=2 appears to fix it. Could you explain the implications of this? > > > I assume it is limiting to 2 outstanding requests concurrently. > > > > You'd think so, but not quite. NVMe queues need to leave one entry > > empty, so a submission queue with depth "2" means you can have at most 1 > > command outstanding. > > > > > Does it suggest an issue with the specific device's FW? > > > > I think that sounds probable. Especially considering the dmapool code > > has had considerable run time in real life, and no other such issue has > > been reported. > > > > > I assume this would suggest that it is not actually anything wrong with the dmapool, it was just exposing the issue of the device/fw? > > > > That's what I'm thinking, though, if you have a single queue with depth > > 2, we're not stressing the dmapool implementation either. It's always > > going to return the same dma block for each command. > > > > > Any advice for handling this and/or investigating further? > > > > If you have the resources for it, get protocol analyzer trace and show > > it to your nvme vendor. > > Unfortunately this is infeasible for us. > > > > > > My initial speculation was that maybe the disk fw is signalling completion of an access before it has actually finished making it's way to ram. I checked the code and saw that the dmapool appears to be used for storing the buffer page addresses, so I imagine that is not updated by the disk at all, which would rule out my assumption. > > > > Right, it's used to make the prp/sgl list. Once we get a completion, > > that dma block becomes immediately available for the very next command. > > If you have a higher queue depth, it's possible that dma block is reused > > immediately while the driver is still notifying the block layer of the > > completion. > > > > If we're thinking that the device is completing the command before it's > > really done with the list (which could explain your observation), that > > would be a problem. Going to single queue-depth might introduce a delay > > or work around some firmware issue when dealing with concurrent > > commands. > > > > Prior to the "new" dmapool allocation, it was much less likely (though I > > think still possible) for your next command to reuse the same dma block > > of the command currently being completed. > > > > given this ~9 year old temporary fix is still in the kernel for the Apple device, could we just add another device specific override? I could maybe convert it to a quirk that is set for them both (and any future devices) https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/nvme/host/pci.c?h=v6.11-rc7#n2570 >
