Differential D45311

nvme: Count number of alginment splits
ClosedPublic
Actions

Authored by imp on May 22 2024, 11:08 PM.

Tags

None

Referenced Files

	F86953781: D45311.id139050.diff
	Thu, Jun 27, 5:53 PM

	Unknown Object (File)
	Fri, Jun 21, 8:33 PM

	Unknown Object (File)
	May 25 2024, 9:02 AM

	Unknown Object (File)
	May 24 2024, 7:20 PM

	Unknown Object (File)
	May 24 2024, 10:28 AM

	Unknown Object (File)
	May 23 2024, 9:38 AM

	Unknown Object (File)
	May 23 2024, 8:48 AM

Subscribers

Details

Reviewers

chuck
mav

Commits

rGd09ee08f106d: nvme: Count number of alginment splits

Summary

When possible, we split up I/Os to NVMe drives that advertise a
preferred alignment. Add a counter for this.

Sponsored by: Netflix

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 57858
Build 54746: arc lint + arc unit

Event Timeline

imp created this revision.May 22 2024, 11:08 PM

Herald added a subscriber: dab. · View Herald TranscriptMay 22 2024, 11:08 PM

imp requested review of this revision.May 22 2024, 11:08 PM

imp added a parent revision: D45310: cam: Drop periph lock when completing I/O with ENOMEM status.

Harbormaster completed remote builds in B57858: Diff 138952.May 22 2024, 11:08 PM

imp added a reviewer: chuck.May 22 2024, 11:09 PM

imp added a reviewer: mav.

gallatin added a subscriber: gallatin.May 23 2024, 1:37 AM

gallatin added inline comments.

sys/dev/nvme/nvme_ns.c
431	You should be using the counter api, not atomics, for something like this.
sys/dev/nvme/nvme_private.h
322	Or if you really prefer an atomic, at least put it on a cacheline by itself.

imp added inline comments.May 23 2024, 2:13 AM

sys/dev/nvme/nvme_ns.c
431	Yea. This counter should be totally cold... but if it's not, we don't want it to mess thing up...

Move to counters for this.

Harbormaster completed remote builds in B57864: Diff 138978.May 23 2024, 3:07 AM

chuck added inline comments.May 23 2024, 1:49 PM

sys/dev/nvme/nvme_sysctl.c
425	The first couple of times I read the description, it sounded like this variable tracked how many splits were in an I/O. Would "Number of I/Os split to get better alignment" convey the right meaning of this sysctl or am I still confused? Regardless of how this gets worded, maybe drop "namespace" as the counter is per controller.

chuck accepted this revision.May 23 2024, 1:49 PM

This revision is now accepted and ready to land.May 23 2024, 1:49 PM

mav added inline comments.May 23 2024, 6:27 PM

sys/dev/nvme/nvme_ctrlr.c
1445 ↗	(On Diff #138978)	I have feeling there should be counter_u64_free() somewhere.

Update per review comments: description and cleanup.

This revision now requires review to proceed.May 23 2024, 8:28 PM

Harbormaster completed remote builds in B57884: Diff 139014.May 23 2024, 8:28 PM

mav accepted this revision.May 23 2024, 9:36 PM

This revision is now accepted and ready to land.May 23 2024, 9:36 PM

Closed by commit rGd09ee08f106d: nvme: Count number of alginment splits (authored by imp). · Explain WhyMay 24 2024, 3:42 PM

This revision was automatically updated to reflect the committed changes.

imp added a commit: rGd09ee08f106d: nvme: Count number of alginment splits.

Revision Contents
Changeset List

Path

Size

sys/

dev/

nvme/

1 line

3 lines

4 lines

Diff 138952

sys/dev/nvme/nvme_ns.c

Loading...

sys/dev/nvme/nvme_private.h

Loading...

sys/dev/nvme/nvme_sysctl.c

Loading...