Skip to contentSkip to conversation

Add Zone Append command support for NVMe Zoned Namespaces (ZNS)

PATCH Alexey Dobriyan 1 participant 1 message Jul 23, 2020

lore
Share & tools
lore
#0 lore raw
Quoted 1 line

+.. option:: zone_append=bool

This forces 1 thread into either write or append. I'm 99% sure this what everyone will benchmark, but still...

We're using different scheme, but it 100% rewrites I/O generation path. :-\

Review

Quoted 5 lines

+.BI zone_append

(quoted diff) +4 -0
+For \fBzonemode\fR =zbd and for \fBrw\fR=write or \fBrw\fR=randwrite, if
+zone_append is enabled, the io_u points to the starting offset of a zone. On
+successful completion the multiple of sectors relative to the zone's starting
+offset is returned.

io_u->offset is a techicality, no user cares about. This should be rewritten in user friendly language like "issue Zone Append command instead of Write".

I also think that io_u->offset should have normal value before ioengine for debugging and detecting reordering.

Quoted 8 lines
	struct io_u {
	 	/*
		 * for zone append this is the start offset of the zone.
		 */
		unsigned long long zone_start_offset;
(quoted diff) +2 -0
+	if (td->o.zone_append) {
+		pthread_mutex_lock(&z->mutex);

Review

Quoted 10 lines
(quoted diff) +10 -0
+		if (z->pending_ios > 0) {
+			z->pending_ios--;
+			/*
+			 * Other threads may be waiting for pending I/O's to
+			 * complete for this zone. Notify them.
+			 */
+			if (!z->pending_ios)
+				pthread_cond_broadcast(&z->reset_cond);
+		}
+	}

You can do

	if (--z->pending_ios == 0) {
		pthread_cond_broadcast(&z->reset_cond);
	}

This is probably wrong (see spurious wakeups):

Quoted 7 lines
(quoted diff) +7 -0
+				 * Wait for the pending requests to be completed
+				 * else we are ok to reset this zone.
+				 */
+				if (zb->pending_ios) {
+					pthread_cond_wait(&zb->reset_cond, &zb->mutex);
+					goto proceed;
+				}