From: Alan Adamson <[email protected]>
Support reading atomic write registers to fill in request_queue
properties.
Use following method to calculate limits:
atomic_write_max_bytes = flp2(NAWUPF ?: AWUPF)
atomic_write_unit_min = logical_block_size
atomic_write_unit_max = flp2(NAWUPF ?: AWUPF)
atomic_write_boundary = NABSPF
Signed-off-by: Alan Adamson <[email protected]>
Signed-off-by: John Garry <[email protected]>
---
drivers/nvme/host/core.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 21783aa2ee8e..aa0daacf4d7c 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1926,6 +1926,35 @@ static void nvme_update_disk_info(struct gendisk *disk,
blk_queue_io_min(disk->queue, phys_bs);
blk_queue_io_opt(disk->queue, io_opt);
+ atomic_bs = rounddown_pow_of_two(atomic_bs);
+ if (id->nsfeat & NVME_NS_FEAT_ATOMICS && id->nawupf) {
+ if (id->nabo) {
+ dev_err(ns->ctrl->device, "Support atomic NABO=%x\n",
+ id->nabo);
+ } else {
+ u32 boundary = 0;
+
+ if (le16_to_cpu(id->nabspf))
+ boundary = (le16_to_cpu(id->nabspf) + 1) * bs;
+
+ if (is_power_of_2(boundary) || !boundary) {
+ blk_queue_atomic_write_max_bytes(disk->queue, atomic_bs);
+ blk_queue_atomic_write_unit_min_sectors(disk->queue, 1);
+ blk_queue_atomic_write_unit_max_sectors(disk->queue,
+ atomic_bs / bs);
+ blk_queue_atomic_write_boundary_bytes(disk->queue, boundary);
+ } else {
+ dev_err(ns->ctrl->device, "Unsupported atomic boundary=0x%x\n",
+ boundary);
+ }
+ }
+ } else if (ns->ctrl->subsys->awupf) {
+ blk_queue_atomic_write_max_bytes(disk->queue, atomic_bs);
+ blk_queue_atomic_write_unit_min_sectors(disk->queue, 1);
+ blk_queue_atomic_write_unit_max_sectors(disk->queue, atomic_bs / bs);
+ blk_queue_atomic_write_boundary_bytes(disk->queue, 0);
+ }
+
/*
* Register a metadata profile for PI, or the plain non-integrity NVMe
* metadata masquerading as Type 0 if supported, otherwise reject block
--
2.31.1
> +++ b/drivers/nvme/host/core.c
> @@ -1926,6 +1926,35 @@ static void nvme_update_disk_info(struct gendisk *disk,
> blk_queue_io_min(disk->queue, phys_bs);
> blk_queue_io_opt(disk->queue, io_opt);
>
> + atomic_bs = rounddown_pow_of_two(atomic_bs);
> + if (id->nsfeat & NVME_NS_FEAT_ATOMICS && id->nawupf) {
> + if (id->nabo) {
> + dev_err(ns->ctrl->device, "Support atomic NABO=%x\n",
> + id->nabo);
> + } else {
> + u32 boundary = 0;
> +
> + if (le16_to_cpu(id->nabspf))
> + boundary = (le16_to_cpu(id->nabspf) + 1) * bs;
> +
> + if (is_power_of_2(boundary) || !boundary) {
> + blk_queue_atomic_write_max_bytes(disk->queue, atomic_bs);
> + blk_queue_atomic_write_unit_min_sectors(disk->queue, 1);
> + blk_queue_atomic_write_unit_max_sectors(disk->queue,
> + atomic_bs / bs);
blk_queue_atomic_write_unit_[min| max]_sectors expects sectors (512 bytes unit)
as input but no conversion is done here from device logical block size
to SECTORs.
> + blk_queue_atomic_write_boundary_bytes(disk->queue, boundary);
> + } else {
> + dev_err(ns->ctrl->device, "Unsupported atomic boundary=0x%x\n",
> + boundary);
> + }
>
On 04/10/2023 12:39, Pankaj Raghav wrote:
>> +++ b/drivers/nvme/host/core.c
>> @@ -1926,6 +1926,35 @@ static void nvme_update_disk_info(struct gendisk *disk,
>> blk_queue_io_min(disk->queue, phys_bs);
>> blk_queue_io_opt(disk->queue, io_opt);
>>
>> + atomic_bs = rounddown_pow_of_two(atomic_bs);
>> + if (id->nsfeat & NVME_NS_FEAT_ATOMICS && id->nawupf) {
>> + if (id->nabo) {
>> + dev_err(ns->ctrl->device, "Support atomic NABO=%x\n",
>> + id->nabo);
>> + } else {
>> + u32 boundary = 0;
>> +
>> + if (le16_to_cpu(id->nabspf))
>> + boundary = (le16_to_cpu(id->nabspf) + 1) * bs;
>> +
>> + if (is_power_of_2(boundary) || !boundary) {
note to self/Alan: boundary just needs to be multiple of atomic write
unit max, and not necessarily a power-of-2
>> + blk_queue_atomic_write_max_bytes(disk->queue, atomic_bs);
>> + blk_queue_atomic_write_unit_min_sectors(disk->queue, 1);
>> + blk_queue_atomic_write_unit_max_sectors(disk->queue,
>> + atomic_bs / bs);
> blk_queue_atomic_write_unit_[min| max]_sectors expects sectors (512 bytes unit)
> as input but no conversion is done here from device logical block size
> to SECTORs.
Yeah, you are right. I think that we can just use:
blk_queue_atomic_write_unit_max_sectors(disk->queue,
atomic_bs >> SECTOR_SHIFT);
Thanks,
John
>> + blk_queue_atomic_write_boundary_bytes(disk->queue, boundary);
>> + } else {
>> + dev_err(ns->ctrl->device, "Unsupported atomic boundary=0x%x\n",
>> + boundary);
>> + }
>>
>>> + blk_queue_atomic_write_max_bytes(disk->queue, atomic_bs);
>>> + blk_queue_atomic_write_unit_min_sectors(disk->queue, 1);
>>> + blk_queue_atomic_write_unit_max_sectors(disk->queue,
>>> + atomic_bs / bs);
>> blk_queue_atomic_write_unit_[min| max]_sectors expects sectors (512 bytes unit)
>> as input but no conversion is done here from device logical block size
>> to SECTORs.
>
> Yeah, you are right. I think that we can just use:
>
> blk_queue_atomic_write_unit_max_sectors(disk->queue,
> atomic_bs >> SECTOR_SHIFT);
>
Makes sense.
I still don't grok the difference between max_bytes and unit_max_sectors here.
(Maybe NVMe spec does not differentiate it?)
I assume min_sectors should be as follows instead of setting it to 1 (512 bytes)?
blk_queue_atomic_write_unit_min_sectors(disk->queue, bs >> SECTORS_SHIFT);
> Thanks,
> John
>
>>> + blk_queue_atomic_write_boundary_bytes(disk->queue, boundary);
>>> + } else {
>>> + dev_err(ns->ctrl->device, "Unsupported atomic boundary=0x%x\n",
>>> + boundary);
>>> + }
>>>
>
On 05/10/2023 14:32, Pankaj Raghav wrote:
>>> te_unit_[min| max]_sectors expects sectors (512 bytes unit)
>>> as input but no conversion is done here from device logical block size
>>> to SECTORs.
>> Yeah, you are right. I think that we can just use:
>>
>> blk_queue_atomic_write_unit_max_sectors(disk->queue,
>> atomic_bs >> SECTOR_SHIFT);
>>
> Makes sense.
> I still don't grok the difference between max_bytes and unit_max_sectors here.
> (Maybe NVMe spec does not differentiate it?)
I think that max_bytes does not need to be a power-of-2 and could be
relaxed.
Having said that, max_bytes comes into play for merging of bios - so if
we are in a scenario with no merging, then may a well leave
atomic_write_max_bytes == atomic_write_unit_max.
But let us check this proposal to relax.
>
> I assume min_sectors should be as follows instead of setting it to 1 (512 bytes)?
>
> blk_queue_atomic_write_unit_min_sectors(disk->queue, bs >> SECTORS_SHIFT);
Yeah, right, we want unit_min to be the logical block size.
Thanks,
John