* [PATCH] nvme: Add support for NVMe 1.3 Timestamp Feature
@ 2017-07-12 22:11 Jon Derrick
2017-07-12 23:24 ` Keith Busch
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jon Derrick @ 2017-07-12 22:11 UTC (permalink / raw)
NVME's Timestamp feature allows controllers to be aware of the epoch
time in milliseconds. This patch adds the set features hook for various
transports. It also wires it up to the pci reset path, so that resets
and resumes can update the controller as necessary.
Signed-off-by: Jon Derrick <jonathan.derrick at intel.com>
---
drivers/nvme/host/core.c | 18 ++++++++++++++++++
drivers/nvme/host/nvme.h | 1 +
drivers/nvme/host/pci.c | 4 ++++
include/linux/nvme.h | 2 ++
4 files changed, 25 insertions(+)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 4a07a98..aa4e765 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1735,6 +1735,24 @@ static void nvme_init_subnqn(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
memset(ctrl->subnqn + off, 0, sizeof(ctrl->subnqn) - off);
}
+void nvme_set_timestamp(struct nvme_ctrl *ctrl)
+{
+ u64 cur_ms;
+ u8 ts[8] = { 0, };
+ int status;
+ u32 result;
+
+ cur_ms = ktime_to_ms(ktime_get_real());
+ put_unaligned_le64(cur_ms, &ts[0]);
+
+ status = nvme_set_features(ctrl, NVME_FEAT_TIMESTAMP, 0, ts, 8,
+ &result);
+ if (status < 0)
+ dev_warn_once(ctrl->device,
+ "could not set timestamp (%08x)\n", result);
+}
+EXPORT_SYMBOL_GPL(nvme_set_timestamp);
+
/*
* Initialize the cached copies of the Identify data and various controller
* register in our nvme_ctrl structure. This should be called as soon as
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index b40b9af..3ef1a07 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -286,6 +286,7 @@ void nvme_start_ctrl(struct nvme_ctrl *ctrl);
void nvme_stop_ctrl(struct nvme_ctrl *ctrl);
void nvme_put_ctrl(struct nvme_ctrl *ctrl);
int nvme_init_identify(struct nvme_ctrl *ctrl);
+void nvme_set_timestamp(struct nvme_ctrl *ctrl);
void nvme_queue_scan(struct nvme_ctrl *ctrl);
void nvme_remove_namespaces(struct nvme_ctrl *ctrl);
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 5d5aa7f..82ce677 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2156,6 +2156,10 @@ static void nvme_reset_work(struct work_struct *work)
if (result)
goto out;
+ if (dev->ctrl.vs >= NVME_VS(1, 3, 0) &&
+ dev->ctrl.oncs & NVME_CTRL_ONCS_TIMESTAMP)
+ nvme_set_timestamp(&dev->ctrl);
+
/*
* Keep the controller around but remove all namespaces if we don't have
* any working I/O queue.
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index 983975b..b7185ae 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -254,6 +254,7 @@ enum {
NVME_CTRL_ONCS_WRITE_UNCORRECTABLE = 1 << 1,
NVME_CTRL_ONCS_DSM = 1 << 2,
NVME_CTRL_ONCS_WRITE_ZEROES = 1 << 3,
+ NVME_CTRL_ONCS_TIMESTAMP = 1 << 6,
NVME_CTRL_VWC_PRESENT = 1 << 0,
NVME_CTRL_OACS_SEC_SUPP = 1 << 0,
NVME_CTRL_OACS_DIRECTIVES = 1 << 5,
@@ -688,6 +689,7 @@ enum {
NVME_FEAT_ASYNC_EVENT = 0x0b,
NVME_FEAT_AUTO_PST = 0x0c,
NVME_FEAT_HOST_MEM_BUF = 0x0d,
+ NVME_FEAT_TIMESTAMP = 0x0e,
NVME_FEAT_KATO = 0x0f,
NVME_FEAT_SW_PROGRESS = 0x80,
NVME_FEAT_HOST_ID = 0x81,
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] nvme: Add support for NVMe 1.3 Timestamp Feature
2017-07-12 22:11 [PATCH] nvme: Add support for NVMe 1.3 Timestamp Feature Jon Derrick
@ 2017-07-12 23:24 ` Keith Busch
2017-07-13 7:01 ` Sagi Grimberg
2017-07-13 7:35 ` Christoph Hellwig
2 siblings, 0 replies; 6+ messages in thread
From: Keith Busch @ 2017-07-12 23:24 UTC (permalink / raw)
On Wed, Jul 12, 2017@04:11:01PM -0600, Jon Derrick wrote:
> +void nvme_set_timestamp(struct nvme_ctrl *ctrl)
> +{
> + u64 cur_ms;
> + u8 ts[8] = { 0, };
> + int status;
> + u32 result;
> +
> + cur_ms = ktime_to_ms(ktime_get_real());
> + put_unaligned_le64(cur_ms, &ts[0]);
> +
> + status = nvme_set_features(ctrl, NVME_FEAT_TIMESTAMP, 0, ts, 8,
> + &result);
> + if (status < 0)
> + dev_warn_once(ctrl->device,
> + "could not set timestamp (%08x)\n", result);
> +}
We don't actually need 'result' for this feature. It won't mean anything
for the feature, and it's actually not going to be set if status < 0,
but we'd want see the value of 'status' if it's != 0.
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] nvme: Add support for NVMe 1.3 Timestamp Feature
2017-07-12 22:11 [PATCH] nvme: Add support for NVMe 1.3 Timestamp Feature Jon Derrick
2017-07-12 23:24 ` Keith Busch
@ 2017-07-13 7:01 ` Sagi Grimberg
2017-07-18 21:12 ` Jon Derrick
2017-07-13 7:35 ` Christoph Hellwig
2 siblings, 1 reply; 6+ messages in thread
From: Sagi Grimberg @ 2017-07-13 7:01 UTC (permalink / raw)
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 5d5aa7f..82ce677 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -2156,6 +2156,10 @@ static void nvme_reset_work(struct work_struct *work)
> if (result)
> goto out;
>
> + if (dev->ctrl.vs >= NVME_VS(1, 3, 0) &&
> + dev->ctrl.oncs & NVME_CTRL_ONCS_TIMESTAMP)
> + nvme_set_timestamp(&dev->ctrl);
> +
Why after setting up the I/O queues?
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] nvme: Add support for NVMe 1.3 Timestamp Feature
2017-07-13 7:01 ` Sagi Grimberg
@ 2017-07-18 21:12 ` Jon Derrick
0 siblings, 0 replies; 6+ messages in thread
From: Jon Derrick @ 2017-07-18 21:12 UTC (permalink / raw)
On 07/13/2017 01:01 AM, Sagi Grimberg wrote:
>
>> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
>> index 5d5aa7f..82ce677 100644
>> --- a/drivers/nvme/host/pci.c
>> +++ b/drivers/nvme/host/pci.c
>> @@ -2156,6 +2156,10 @@ static void nvme_reset_work(struct work_struct *work)
>> if (result)
>> goto out;
>>
>> + if (dev->ctrl.vs >= NVME_VS(1, 3, 0) &&
>> + dev->ctrl.oncs & NVME_CTRL_ONCS_TIMESTAMP)
>> + nvme_set_timestamp(&dev->ctrl);
>> +
>
> Why after setting up the I/O queues?
>
No reason in particular; I just wanted it in the reset path.
For v2 I've moved it to the identify path.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] nvme: Add support for NVMe 1.3 Timestamp Feature
2017-07-12 22:11 [PATCH] nvme: Add support for NVMe 1.3 Timestamp Feature Jon Derrick
2017-07-12 23:24 ` Keith Busch
2017-07-13 7:01 ` Sagi Grimberg
@ 2017-07-13 7:35 ` Christoph Hellwig
2017-07-18 21:13 ` Jon Derrick
2 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2017-07-13 7:35 UTC (permalink / raw)
On Wed, Jul 12, 2017@04:11:01PM -0600, Jon Derrick wrote:
> NVME's Timestamp feature allows controllers to be aware of the epoch
> time in milliseconds. This patch adds the set features hook for various
> transports. It also wires it up to the pci reset path, so that resets
> and resumes can update the controller as necessary.
Please find a way to do this entirely from common code, currently you
miss out on all the non-PCIe transports.
> + u64 cur_ms;
> + u8 ts[8] = { 0, };
> + int status;
> + u32 result;
> +
> + cur_ms = ktime_to_ms(ktime_get_real());
> + put_unaligned_le64(cur_ms, &ts[0]);
> +
> + status = nvme_set_features(ctrl, NVME_FEAT_TIMESTAMP, 0, ts, 8,
I think this could simply be something like:
__le64 ts;
ts = cpu_to_le64(ktime_to_ms(ktime_get_real()));
status = nvme_set_features(ctrl, NVME_FEAT_TIMESTAMP, 0, &ts,
sizeof(ts),
> + &result);
> + if (status < 0)
> + dev_warn_once(ctrl->device,
> + "could not set timestamp (%08x)\n", result);
> +}
The Timestamp feature does not define a result, so you should just
pass NULL for the result parameter. And if you want to print something
on error the status value is probably more useful.
> + if (dev->ctrl.vs >= NVME_VS(1, 3, 0) &&
> + dev->ctrl.oncs & NVME_CTRL_ONCS_TIMESTAMP)
> + nvme_set_timestamp(&dev->ctrl);
No need to check for the version - NVMe TPs also apply to all previous
versions if they are backwards compatible, which this one is.
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] nvme: Add support for NVMe 1.3 Timestamp Feature
2017-07-13 7:35 ` Christoph Hellwig
@ 2017-07-18 21:13 ` Jon Derrick
0 siblings, 0 replies; 6+ messages in thread
From: Jon Derrick @ 2017-07-18 21:13 UTC (permalink / raw)
On 07/13/2017 01:35 AM, Christoph Hellwig wrote:
> On Wed, Jul 12, 2017@04:11:01PM -0600, Jon Derrick wrote:
>> NVME's Timestamp feature allows controllers to be aware of the epoch
>> time in milliseconds. This patch adds the set features hook for various
>> transports. It also wires it up to the pci reset path, so that resets
>> and resumes can update the controller as necessary.
>
> Please find a way to do this entirely from common code, currently you
> miss out on all the non-PCIe transports.
>
>> + u64 cur_ms;
>> + u8 ts[8] = { 0, };
>> + int status;
>> + u32 result;
>> +
>> + cur_ms = ktime_to_ms(ktime_get_real());
>> + put_unaligned_le64(cur_ms, &ts[0]);
>> +
>> + status = nvme_set_features(ctrl, NVME_FEAT_TIMESTAMP, 0, ts, 8,
>
> I think this could simply be something like:
>
> __le64 ts;
>
> ts = cpu_to_le64(ktime_to_ms(ktime_get_real()));
> status = nvme_set_features(ctrl, NVME_FEAT_TIMESTAMP, 0, &ts,
> sizeof(ts),
>
>> + &result);
>> + if (status < 0)
>> + dev_warn_once(ctrl->device,
>> + "could not set timestamp (%08x)\n", result);
>> +}
>
> The Timestamp feature does not define a result, so you should just
> pass NULL for the result parameter. And if you want to print something
> on error the status value is probably more useful.
>
>> + if (dev->ctrl.vs >= NVME_VS(1, 3, 0) &&
>> + dev->ctrl.oncs & NVME_CTRL_ONCS_TIMESTAMP)
>> + nvme_set_timestamp(&dev->ctrl);
>
> No need to check for the version - NVMe TPs also apply to all previous
> versions if they are backwards compatible, which this one is.
>
Thanks for the tips
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-07-18 21:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-12 22:11 [PATCH] nvme: Add support for NVMe 1.3 Timestamp Feature Jon Derrick
2017-07-12 23:24 ` Keith Busch
2017-07-13 7:01 ` Sagi Grimberg
2017-07-18 21:12 ` Jon Derrick
2017-07-13 7:35 ` Christoph Hellwig
2017-07-18 21:13 ` Jon Derrick
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.