* Re: [PATCH v2 1/7] blk-mq-dma: restore BLK_STS_TARGET for unsupported P2P transfers
From: Logan Gunthorpe @ 2026-07-20 18:42 UTC (permalink / raw)
To: Mykola Marzhan, Christoph Hellwig
Cc: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Sagi Grimberg,
linux-block, linux-raid, linux-nvme, Li Nan, Xiao Ni,
Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Bjorn Helgaas,
Shivaji Kant, Pranjal Shrivastava, Henrique Carvalho,
linux-kernel, linux-rdma, linux-pci
In-Reply-To: <CAPzsNDv6hUV3gHdaxuwjRcr4couUKq_M5DKObFNaYqCHnm7ZMw@mail.gmail.com>
On 2026-07-20 11:30, Mykola Marzhan wrote:
> On Mon, Jul 20, 2026 at 4:49 PM Christoph Hellwig <hch@lst.de
>> This is a really weird writing style, and suggested to me you neither
>> understand the code nor the problem. Please actually think yourself,
>> write the patches and commit logs yourself instdad of this garbage.
>>
>> And please also explain how you even generate the I/O that fails this
>> way.
> - md mirrors the write: the copy to member 1 succeeds, the copy to far
> member fails as BLK_STS_INVAL.
> - md deliberately ignores BLK_STS_INVAL, so the write is counted as
> written and the mirror silently diverges.
Seeing I just reviewed this and answered it for myself I'll expand on
what's going on here:
The switch statement will take the default branch when
pci_p2pdma_state() returns PCI_P2PDMA_MAP_NOT_SUPPORTED. This indicates
the memory that's trying to be mapped in this way will not succeed
because it's going through an unsupported host bridge (or if the device
isn't a PCI device, etc, but this is rarer or perhaps not possible).
Returning BLK_STS_INVAL doesn't seem correct here, to me. (Arguably it
might have been clearer if the switch case explicitly stated
PCI_P2PDMA_MAP_NOT_SUPPORTED instead of relying on the default).
Returning -EREMOTEIO/BLK_STS_TARGET was the convention for this I had
originally set when I wrote some of this and differentiating the error
does seem important in Patch 6 in this series.
Logan
^ permalink raw reply
* Re: [PATCH v2 1/7] blk-mq-dma: restore BLK_STS_TARGET for unsupported P2P transfers
From: Logan Gunthorpe @ 2026-07-20 18:49 UTC (permalink / raw)
To: Mykola Marzhan, Christoph Hellwig
Cc: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Sagi Grimberg,
linux-block, linux-raid, linux-nvme, Li Nan, Xiao Ni,
Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Bjorn Helgaas,
Shivaji Kant, Pranjal Shrivastava, Henrique Carvalho,
linux-kernel, linux-rdma, linux-pci
In-Reply-To: <b2c37f4f-79dd-428f-915a-2a862d20924c@deltatee.com>
On 2026-07-20 12:42, Logan Gunthorpe wrote:
>
>
> On 2026-07-20 11:30, Mykola Marzhan wrote:
>> On Mon, Jul 20, 2026 at 4:49 PM Christoph Hellwig <hch@lst.de
>>> This is a really weird writing style, and suggested to me you neither
>>> understand the code nor the problem. Please actually think yourself,
>>> write the patches and commit logs yourself instdad of this garbage.
>>>
>>> And please also explain how you even generate the I/O that fails this
>>> way.
>> - md mirrors the write: the copy to member 1 succeeds, the copy to far
>> member fails as BLK_STS_INVAL.
>> - md deliberately ignores BLK_STS_INVAL, so the write is counted as
>> written and the mirror silently diverges.
>
> Seeing I just reviewed this and answered it for myself I'll expand on
> what's going on here:
>
> The switch statement will take the default branch when
> pci_p2pdma_state() returns PCI_P2PDMA_MAP_NOT_SUPPORTED. This indicates
> the memory that's trying to be mapped in this way will not succeed
> because it's going through an unsupported host bridge (or if the device
> isn't a PCI device, etc, but this is rarer or perhaps not possible).
> Returning BLK_STS_INVAL doesn't seem correct here, to me. (Arguably it
> might have been clearer if the switch case explicitly stated
> PCI_P2PDMA_MAP_NOT_SUPPORTED instead of relying on the default).
>
> Returning -EREMOTEIO/BLK_STS_TARGET was the convention for this I had
> originally set when I wrote some of this and differentiating the error
> does seem important in Patch 6 in this series.
Sorry, one other note I almost forgot about which is probably important:
BLK_STS_INVAL will attempt a retry, where BLK_STS_TARGET will not (I
think that's part of why I chose that specific error in the first
place). A retry here doesn't make any sense because it will always hit
the same case and fail. So it's probably worth applying this patch even
without the specific error being used in Patch 6.
Logan
^ permalink raw reply
page: | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox