Skip to content

Commit c83eb3c

Browse files
committed
Fix IRP_MJ_READ performance: CcFlushCache gate and dprintf cleanup
CcFlushCache was called on every non-paging IRP_MJ_READ that had a DataSectionObject, including normal cached reads. This forced synchronous write-back of dirty pages before every read, negating the Cache Manager's lazy-writeback benefit and causing a ~45x throughput regression (30 MB/s vs 1350 MB/s sequential read). Add the missing IRP_NOCACHE guard, matching the pattern already used in the write path (zfs_write_wrap), so CcFlushCache is only called when bypassing the Cache Manager. Remove hot-path dprintf calls from fs_read, fs_read_impl, zfs_read_wrap, fs_write_impl, zfs_write_wrap, do_read_job and do_write_job. Each dprintf acquired a global spinlock (cbuf_spin) via printBuffer, serialising all I/O threads. Error-path dprintf calls are retained. Signed-off-by: Jorgen Lundman <lundman@lundman.net>
1 parent ad7e226 commit c83eb3c

1 file changed

Lines changed: 7 additions & 71 deletions

File tree

module/os/windows/zfs/zfs_vnops_windows.c

Lines changed: 7 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1989,7 +1989,7 @@ zfs_vnop_lookup_impl(PIRP Irp, PIO_STACK_LOCATION IrpSp, mount_t *zmo,
19891989
vap->va_mode |= S_IFDIR;
19901990
vap->va_mask |= (ATTR_MODE | ATTR_TYPE);
19911991

1992-
/* Set UID,GID from IRP security context for new dir ownership */
1992+
/* Set UID,GID from IRP security context for new ownership */
19931993
zfs_security_context_pre(vap,
19941994
IrpSp->Parameters.Create.SecurityContext);
19951995

@@ -2349,7 +2349,7 @@ zfs_vnop_lookup_impl(PIRP Irp, PIO_STACK_LOCATION IrpSp, mount_t *zmo,
23492349
break;
23502350
}
23512351

2352-
/* Set UID,GID from IRP security context for new file ownership */
2352+
/* Set UID,GID from IRP security context for new ownership */
23532353
zfs_security_context_pre(vap,
23542354
IrpSp->Parameters.Create.SecurityContext);
23552355

@@ -4273,7 +4273,8 @@ set_reparse_point(PDEVICE_OBJECT DeviceObject, PIRP Irp,
42734273

42744274
/*
42754275
* If a reparse point is already set, the caller's tag must match
4276-
* the existing tag, otherwise Windows requires STATUS_IO_REPARSE_TAG_MISMATCH.
4276+
* the existing tag, otherwise Windows requires
4277+
* STATUS_IO_REPARSE_TAG_MISMATCH.
42774278
*/
42784279
if (zp->z_pflags & ZFS_REPARSE) {
42794280
ULONG existing_tag = get_reparse_tag(zp);
@@ -5749,8 +5750,6 @@ zfs_read_wrap(vnode_t *vp, uint8_t *data, uint64_t start,
57495750
NTSTATUS Status;
57505751
znode_t *zp = VTOZ(vp);
57515752

5752-
dprintf("(%p, %p, %I64x, %I64x, %p)\n", vp, data, start, length, pbr);
5753-
57545753
VERIFY3P(zp, !=, NULL);
57555754

57565755
if (pbr)
@@ -5771,9 +5770,6 @@ zfs_read_wrap(vnode_t *vp, uint8_t *data, uint64_t start,
57715770
zfs_uio_iovec_init(&uio, &iov, 1, start, UIO_SYSSPACE,
57725771
length, 0);
57735772

5774-
dprintf("%s: offset %llx size %llx\n", __func__,
5775-
start, length);
5776-
57775773
if (Irp->MdlAddress == NULL &&
57785774
Irp->UserBuffer != NULL) {
57795775
if (!LockUserBuffer(Irp, IoWriteAccess, length)) {
@@ -5815,12 +5811,6 @@ fs_read_impl(PIRP Irp, boolean_t wait, uint64_t *bytes_read)
58155811
if (!vp || !VTOZ(vp))
58165812
return (STATUS_INTERNAL_ERROR);
58175813

5818-
dprintf("vp = %p\n", vp);
5819-
dprintf("offset = %I64x, length = %lx\n", start, length);
5820-
dprintf("paging_io = %s, no cache = %s\n",
5821-
Irp->Flags & IRP_PAGING_IO ? "true" : "false",
5822-
nocache ? "true" : "false");
5823-
58245814
if (!vnode_isreg(vp))
58255815
return (STATUS_INVALID_DEVICE_REQUEST);
58265816

@@ -5841,10 +5831,6 @@ fs_read_impl(PIRP Irp, boolean_t wait, uint64_t *bytes_read)
58415831
return (STATUS_END_OF_FILE);
58425832
}
58435833

5844-
dprintf("FileObject %p vp %p FileSize = %I64x st_size = %I64x\n",
5845-
FileObject, vp, vp->FileHeader.FileSize.QuadPart,
5846-
VTOZ(vp)->z_size);
5847-
58485834
if (!nocache && (IrpSp->MinorFunction & IRP_MN_MDL)) {
58495835
NTSTATUS Status = STATUS_SUCCESS;
58505836

@@ -5927,13 +5913,6 @@ fs_read_impl(PIRP Irp, boolean_t wait, uint64_t *bytes_read)
59275913
}
59285914

59295915
#if (NTDDI_VERSION >= NTDDI_WIN8)
5930-
dprintf("CcCopyReadEx(%p, %I64x, %lx, %u, %p, %p, %p)\n",
5931-
FileObject, IrpSp->Parameters.Read.ByteOffset.QuadPart,
5932-
length, wait, data, &Irp->IoStatus, Irp->Tail.Overlay.Thread);
5933-
dprintf("sizes = %I64x, %I64x, %I64x\n",
5934-
vp->FileHeader.AllocationSize.QuadPart,
5935-
vp->FileHeader.FileSize.QuadPart,
5936-
vp->FileHeader.ValidDataLength.QuadPart);
59375916
if (!CcCopyReadEx(FileObject,
59385917
&IrpSp->Parameters.Read.ByteOffset,
59395918
length, wait, data, &Irp->IoStatus,
@@ -5943,15 +5922,7 @@ fs_read_impl(PIRP Irp, boolean_t wait, uint64_t *bytes_read)
59435922
IoMarkIrpPending(Irp);
59445923
return (STATUS_PENDING);
59455924
}
5946-
dprintf("CcCopyReadEx finished\n");
59475925
#else
5948-
dprintf("CcCopyRead(%p, %I64x, %lx, %u, %p, %p)\n", FileObject,
5949-
IrpSp->Parameters.Read.ByteOffset.QuadPart, length, wait,
5950-
data, &Irp->IoStatus);
5951-
dprintf("sizes = %I64x, %I64x, %I64x\n",
5952-
vp->FileHeader.AllocationSize.QuadPart,
5953-
vp->FileHeader.FileSize.QuadPart,
5954-
vp->FileHeader.ValidDataLength.QuadPart);
59555926
if (!CcCopyRead(FileObject,
59565927
&IrpSp->Parameters.Read.ByteOffset,
59575928
length, wait, data, &Irp->IoStatus)) {
@@ -5960,7 +5931,6 @@ fs_read_impl(PIRP Irp, boolean_t wait, uint64_t *bytes_read)
59605931
IoMarkIrpPending(Irp);
59615932
return (STATUS_PENDING);
59625933
}
5963-
dprintf("CcCopyRead finished\n");
59645934
#endif
59655935
} except(EXCEPTION_EXECUTE_HANDLER) {
59665936
Status = GetExceptionCode();
@@ -5990,8 +5960,6 @@ fs_read_impl(PIRP Irp, boolean_t wait, uint64_t *bytes_read)
59905960
}
59915961

59925962
*bytes_read += addon;
5993-
dprintf("read %llu bytes\n", *bytes_read);
5994-
59955963
Irp->IoStatus.Information = *bytes_read;
59965964

59975965
return (Status);
@@ -6080,7 +6048,9 @@ fs_read(PDEVICE_OBJECT DeviceObject, PIRP Irp, PIO_STACK_LOCATION IrpSp)
60806048
// Don't offload jobs when doing paging IO - otherwise this can lead to
60816049
// deadlocks in CcCopyRead.
60826050

6083-
if (!(Irp->Flags & IRP_PAGING_IO) && FileObject->SectionObjectPointer &&
6051+
if ((Irp->Flags & IRP_NOCACHE) &&
6052+
!(Irp->Flags & IRP_PAGING_IO) &&
6053+
FileObject->SectionObjectPointer &&
60846054
FileObject->SectionObjectPointer->DataSectionObject) {
60856055
IO_STATUS_BLOCK iosb;
60866056

@@ -6130,13 +6100,6 @@ fs_read(PDEVICE_OBJECT DeviceObject, PIRP Irp, PIO_STACK_LOCATION IrpSp)
61306100

61316101
Irp->IoStatus.Status = Status;
61326102

6133-
dprintf("IrpSp->Parameters.Read.Length = %08lx\n",
6134-
IrpSp->Parameters.Read.Length);
6135-
dprintf("Irp->IoStatus.Status = %08lx\n",
6136-
Irp->IoStatus.Status);
6137-
dprintf("Irp->IoStatus.Information = %Iu bytesread %llu\n",
6138-
Irp->IoStatus.Information, bytes_read);
6139-
61406103
VN_RELE(vp);
61416104

61426105
zfs_exit(zfsvfs, FTAG);
@@ -6165,9 +6128,6 @@ zfs_write_wrap(PDEVICE_OBJECT DeviceObject, PIRP Irp,
61656128
pagefile;
61666129
ULONG filter = 0;
61676130

6168-
dprintf("(%p, %p, %I64x, %p, %lx, %u, %u)\n", DeviceObject,
6169-
FileObject, offset.QuadPart, buf, *length, paging_io, no_cache);
6170-
61716131
if (*length == 0) {
61726132
dprintf("returning success for zero-length write\n");
61736133
return (STATUS_SUCCESS);
@@ -6198,8 +6158,6 @@ zfs_write_wrap(PDEVICE_OBJECT DeviceObject, PIRP Irp,
61986158

61996159
off64 = offset.QuadPart;
62006160

6201-
dprintf("vp->Header.Flags = %x\n", vp->FileHeader.Flags);
6202-
62036161
if (!no_cache && !CcCanIWrite(FileObject, *length, wait,
62046162
deferred_write)) {
62056163
IoMarkIrpPending(Irp);
@@ -6296,8 +6254,6 @@ zfs_write_wrap(PDEVICE_OBJECT DeviceObject, PIRP Irp,
62966254
if (zp->z_unlinked)
62976255
newlength = 0;
62986256

6299-
dprintf("newlength = %I64x\n", newlength);
6300-
63016257
if (off64 + *length > newlength) {
63026258
if (paging_io) {
63036259
if (off64 >= newlength) {
@@ -6384,9 +6340,6 @@ zfs_write_wrap(PDEVICE_OBJECT DeviceObject, PIRP Irp,
63846340
* being called before the job has run. See ifstest ReadWriteTest.
63856341
*/
63866342

6387-
dprintf("CcCopyWrite(%p, %I64x, %lx, %p, %p)\n",
6388-
FileObject, off64, *length, buf,
6389-
Irp->Tail.Overlay.Thread);
63906343
#if (NTDDI_VERSION >= NTDDI_WIN8)
63916344
if (!CcCopyWriteEx(FileObject, &offset, *length,
63926345
TRUE, buf, Irp->Tail.Overlay.Thread)) {
@@ -6556,12 +6509,8 @@ zfs_write_wrap(PDEVICE_OBJECT DeviceObject, PIRP Irp,
65566509
end:
65576510
if (NT_SUCCESS(Status) && FileObject->Flags & FO_SYNCHRONOUS_IO &&
65586511
!paging_io) {
6559-
dprintf("CurrentByteOffset was: %I64x\n",
6560-
FileObject->CurrentByteOffset.QuadPart);
65616512
FileObject->CurrentByteOffset.QuadPart =
65626513
offset.QuadPart + (NT_SUCCESS(Status) ? *length : 0);
6563-
dprintf("CurrentByteOffset now: %I64x\n",
6564-
FileObject->CurrentByteOffset.QuadPart);
65656514
}
65666515

65676516
if (paging_lock)
@@ -6589,9 +6538,6 @@ fs_write_impl(PDEVICE_OBJECT DeviceObject, PIRP Irp, PIO_STACK_LOCATION IrpSp,
65896538

65906539
Irp->IoStatus.Information = 0;
65916540

6592-
dprintf("offset = %I64x\n", offset.QuadPart);
6593-
dprintf("length = %lx\n", IrpSp->Parameters.Write.Length);
6594-
65956541
if (IrpSp->Parameters.Write.Length == 0) {
65966542
Status = STATUS_SUCCESS;
65976543
goto exit;
@@ -6611,8 +6557,6 @@ fs_write_impl(PDEVICE_OBJECT DeviceObject, PIRP Irp, PIO_STACK_LOCATION IrpSp,
66116557
buf = Irp->AssociatedIrp.SystemBuffer;
66126558
}
66136559

6614-
dprintf("buf = %p\n", buf);
6615-
66166560
if (vp && !(Irp->Flags & IRP_PAGING_IO) &&
66176561
!FsRtlCheckLockForWriteAccess(&vp->lock, Irp)) {
66186562
dprintf("tried to write to locked region\n");
@@ -6790,15 +6734,11 @@ do_read_job(PDEVICE_OBJECT DeviceObject, PIRP Irp)
67906734

67916735
Irp->IoStatus.Status = Status;
67926736

6793-
dprintf("read %Iu bytes\n", Irp->IoStatus.Information);
6794-
67956737
IoCompleteRequest(Irp, IO_NO_INCREMENT);
67966738

67976739
if (top_level)
67986740
IoSetTopLevelIrp(NULL);
67996741

6800-
dprintf("returning %08lx\n", Status);
6801-
68026742
return (Status);
68036743
}
68046744

@@ -6822,15 +6762,11 @@ do_write_job(PDEVICE_OBJECT DeviceObject, PIRP Irp)
68226762

68236763
Irp->IoStatus.Status = Status;
68246764

6825-
dprintf("wrote %Iu bytes\n", Irp->IoStatus.Information);
6826-
68276765
IoCompleteRequest(Irp, IO_NO_INCREMENT);
68286766

68296767
if (top_level)
68306768
IoSetTopLevelIrp(NULL);
68316769

6832-
dprintf("returning %08lx\n", Status);
6833-
68346770
return (Status);
68356771
}
68366772

0 commit comments

Comments
 (0)