Commit graph

133 commits

Author SHA1 Message Date
Serge Gautherie a0d2170729 [NTOS:CC] CcWriteVirtualAddress(): Do not round 'Size'
Revert 'Properly align VACB writes' part of 2a80ae2bb6.

CORE-15384 CORE-16018
2019-07-01 21:30:25 +02:00
Pierre Schweitzer 77b6899d89
[NTOSKRNL] Don't set VACB dirty on release if already dirty
CORE-15954
2019-04-20 11:23:35 +02:00
Pierre Schweitzer bd75947372
[NTOSKRNL] Drop the no longer needed Dirty field
CORE-15954
2019-04-18 08:14:41 +02:00
Pierre Schweitzer 8b6696fcdb
[NTOSKRNL] Don't mark VACB dirty on unpin
This could happen if BCB was marked dirty previously.
Marking VACB dirty on unpin could lead to a double write of
the VACB, even if clean.
Indeed, now that setting BCB dirty leads to marking VACB
dirty, the VACB can be flushed in between by the lazy-writer.
The BCB state is not reset on VACB flush, contrary to the VACB state.
Thus, on unpin even if the VACB was already flushed, we were
setting back the dirty state, leading the VACB to be flushed again.

This could bring a small performance downgrade. Though it remains
limited since this is mostly used for FS metadata.
Possibly it could lead to metadata corruption, but this is likely
less possible.

CORE-15954
2019-04-17 22:35:19 +02:00
Timo Kreuzer 71fefa32db
[NDK][NTOS] Add global definition of INIT_FUNCTION/INIT_SECTION (#779)
* Add an NDK header to define INIT_FUNCTION/INIT_SECTION globally
* Use _declspec(allocate(x)) and _declspec(code_seg(x)) on MSVC versions that support it
* Use INIT_FUNCTION on functions only and INIT_SECTION on data only (required by MSVC)
* Place INIT_FUNCTION before the return type (required by MSVC)
* Make sure declarations and implementations share the same modifiers (required by MSVC)
* Add a global linker option to suppress warnings about defined but unused INIT section
* Merge INIT section into .text in freeldr
2018-12-30 12:19:11 +01:00
Pierre Schweitzer e16241255d
[NTOSKRNL] Set the MemoryMaker flag when executing write behind 2018-12-23 14:59:42 +01:00
Pierre Schweitzer 7e97071c8b
[NTOSKRNL] Implement write behind in Cc
For now, this is just a split between scan and flush that
were both done during lazy scan previously.
Lazy scan shouldn't perform any write operation, but only
queue a write behind operation.

Our implementation is far from the original, as it seems
our lazy scan should queue a write behind operation per
shared cache map. Right now, we only perform global
operation.
2018-12-23 14:45:38 +01:00
Pierre Schweitzer 1a267045f8
[NTOSKRNL] Honor files that shouldn't be lazy written 2018-12-23 12:10:58 +01:00
Pierre Schweitzer 0917c64812
[NTOSKRNL] When growing a file, invalid the last VACB so that it can be refreshed
This will avoid corruption when a file size is little grown and read afterwards.
Up to now, FSD where reading 0es instead of expected data, causing corruption.

This fixes MS FastFAT not being able to mount a FAT volume in ReactOS, corrupting
the FAT.
This also fixes the CcSetFileSizes kmtest tests.

This is based on a patch by Thomas Faber.

CORE-11819
2018-12-23 11:19:14 +01:00
Serge Gautherie 4d1f13cf77 [NTOS:CC] CcCanIWrite(): Use BYTES_TO_PAGES(Length) 2018-12-21 19:20:59 +01:00
Pierre Schweitzer 1435ff95b4
[NTOSKRNL] Don't call AcquireForLazyWrite with the master lock held
This incorrect behavior was leading to a call at too high IRQL for paged code.
This was triggered by MS FastFAT.

ReleaseFromLazyWrite call was already correctly called to that regard.

CORE-11819
2018-12-21 08:46:40 +01:00
Pierre Schweitzer 4f8b041bf0
[NTOSKRNL] Drop the ViewLock mutex in favour of the master spin lock
This will allow Cc calls during DPC, which is required by MS FastFAT

CORE-11819
2018-12-19 22:51:45 +01:00
Pierre Schweitzer 182cc5c5ab
[NTOSKRNL] Don't dereference VACB when allocating its memory area fails
This avoids performing a double-free (even though that's hidden by the
fact we use lookaside allocations for VACB), and it avoids freeing
a memory address at an uninitialized address.
We don't care about references here, the VACB was just allocated, never
linked and we're its only user.

CORE-15413
2018-12-08 19:56:03 +01:00
Pierre Schweitzer cf7969fbfa
[NTOSKRNL] Fix refcounting for BCBs
Now, we make sure that we update ref count and BCB list membership
with the BCB lock held, in a row.
This will avoid race conditions where the BCB was removed from the
list, then referenced again, leading to inconsistencies in memory
and crashes later on.
This could notably be triggered while building ReactOS on ReactOS
(one would call this a regression).

CORE-15235
2018-10-28 20:48:01 +01:00
Pierre Schweitzer 18f3922725
[NTOSKRNL] Call internal helper to get VACB on mapping
We already properly round our offset
2018-10-27 09:38:55 +02:00
Pierre Schweitzer f3b9beeb39
[NTOSKRNL] Translate pinning flags to mapping flags when first mapping a file 2018-10-23 22:07:53 +02:00
Pierre Schweitzer 3d13a464f7
[NTOSKRNL] Add the CcPinMappedDataCount counter 2018-10-13 22:51:44 +02:00
Pierre Schweitzer 1afcbbd125
[NTOSKRNL] Rewrite the way we create BCB for pinning
We won't reuse a BCB created for mapping, we will now have
our own dedicated BCB.
This allows having a bit more cleaner implementation of CcPinMappedData()
2018-10-13 22:51:44 +02:00
Pierre Schweitzer 1acb5a9fab
[NTOSKRNL] Don't keep the spin lock hold too long when we lost the BCB race
This will avoid a deadlock on unpin.
2018-10-12 08:20:32 +02:00
Pierre Schweitzer cf8ba3bd9c
[NTOSKRNL] Rewrite BCB handling to be more robust
We now handle race conditions when creating BCB to avoid
having duplicated BCB per shared maps.
Also, we already specify whether the memory will be pinned
when creating the BCB, to avoid potential duplications or
BCB misuse.
2018-10-11 23:15:01 +02:00
Pierre Schweitzer bd39459f89
[NTOSKRNL] Implement support for PIN_IF_BCB flag 2018-10-05 21:26:16 +02:00
Pierre Schweitzer 7fd2751c87
[NTOSKRNL] When pinning data, try to find an already pinned BCB
If found, attempt to lock it and return it.

This fixes a lot of CcPinRead tests (and seems to speed up a bit ReactOS)
2018-10-05 21:26:16 +02:00
Pierre Schweitzer 9fc75c1132
[NTOSKRNL] When mapping data, try to find if there's already a BCB
If so, return such BCB instead of creating a new one. This will
allow (at some point) to be more consistent in case of concurrent
mapping.

This fixes a few CcMapData tests.
2018-10-05 21:26:16 +02:00
Pierre Schweitzer f284947622
[NTOSKRNL] Move the PinCount out of the VACB to the BCB
Given current ReactOS implementation, a VACB can be pinned
several times, with different BCB. In next commits, a single
BCB will be able to be pinned several times. That would
lead to severe inconsistencies in counting and thus corruption.
2018-10-05 21:26:16 +02:00
Pierre Schweitzer 2a80ae2bb6
[NTOSKRNL] Properly align VACB writes
Also simplify VACB reads alignment code.
Also add some sanity ASSERTs.
2018-09-23 10:32:14 +02:00
Pierre Schweitzer 15a3ca08b0
[NTOSKRNL] Avoid integer overflow when computing VACB read/write size
This could be triggered when attempting to read/write to really big
files. It was causing an attempt to read 0 bytes in Cc, leading to
asserts failure in the kernel (and corrupted file).

CORE-15067
2018-09-21 08:37:20 +02:00
Pierre Schweitzer 02da7b452c
[NTOSKRNL] Move data mapping implementation to an internel helper 2018-09-09 14:02:13 +02:00
Pierre Schweitzer 3dabca398f
[NTOSKRNL] Don't raise a status when parameters are invalid on file mapping 2018-09-05 22:06:29 +02:00
Pierre Schweitzer e17f61138c
[NTOSKRNL] When allocating a new BCB, save it in a list
This list is stored in the shared map. Later, this will allow
reusing BCB when appropriate
2018-09-05 22:06:25 +02:00
Pierre Schweitzer f96f1224a7
[NTOSKRNL] Fail on pinning when there's no pin access set
Instead of assert, now, CcPinRead will just fail. This is
not consistent without Windows behavior, but still better
than asserting while testing!
2018-09-01 12:41:01 +02:00
Pierre Schweitzer f0eb39084e
[NTOSKRNL] Fix a typo 2018-08-31 19:48:32 +02:00
Pierre Schweitzer e806d16b06
[NTOSKRNL] Warn about unimplemented feature in CcMapData() (in all callers)
Currently, our CcMapData() behavior (same goes for CcPinRead()) is broken
and is the total opposite of what Windows kernel does. By default, the later
will let you map a view in memory without even attempting to bring its
data in memory. On first access, there will be a fault and memory will
be read from the hardware and brought to memory. If you want to force read
on mapping/pinning, you have to set the MAP_NO_READ (or PIN_NO_READ) flag
where kernel will fault on your behalf (hence the need for MAP_WAIT/PIN_WAIT).

On ReactOS, by default, on mapping (and thus pinning), we will force a view
read so that data is in memory. The way our cache memory is managed at the
moment seems not to allow to fault on invalid access and if we don't force
read, the memory content will just be zeroed.
So trying to match Windows behavior, by default, now CcMapData() will enforce
the MAP_NO_READ flag and warn once about this behavior change.
2018-08-31 19:48:32 +02:00
Pierre Schweitzer b8e4af606a
[NTOSKRNL] Properly reset pinning state on pinning failure 2018-08-26 22:56:25 +02:00
Pierre Schweitzer 54f89baad4
[NTOSKRNL] When acquiring BCB shared, starve exclusive waiters 2018-08-26 22:47:48 +02:00
Pierre Schweitzer c1dd4c142f
[NTOSKRNL] Handle the PIN_WAIT flag in CcPinMappedData() 2018-08-26 22:05:11 +02:00
Pierre Schweitzer 469e15c7ae
[NTOSKRNL] Stubplement CcPinMappedData() and simplify CcPinRead()
It's based on the code that was in CcPinRead() implementation. This
made no sense to have CcPinMappedData() doing nothing while implementing
everything in CcPinRead(). Indeed, drivers (starting with MS drivers)
can map data first and pin it afterwards with CcPinMappedData(). It was
leading to incorrect behavior with our previous noop implementation.
2018-08-26 22:05:11 +02:00
Pierre Schweitzer 0075c2a02d
[NTOSKRNL] Be noisy when deferring writes. 2018-07-15 09:57:16 +02:00
Thomas Faber 1d398057a3
[NTOS:CC] Access SectionObjectPointers without lock in CcRosInitializeFileCache. CORE-14691
kmtest:NtCreateSection calls CcInitializeCacheMap with a
NULL value for SectionObjectPointers. This will cause an exception when
trying to access it, which in Windows can be handled gracefully.
However accessing it while holding ViewLock means the lock will not be
released, leading to an APC_INDEX_MISMATCH bugcheck.

This solves the problem by allocating SharedCacheMap outside the lock,
then freeing it again under lock if another thread has updated SharedCacheMap
in the mean time. This is also What Windows Does(TM).
2018-06-05 16:24:13 +02:00
Pierre Schweitzer 2cf9a69bce
[NTOSKRNL] Addendum to 8a8cb4d: don't print uninit pointer. 2018-05-23 08:44:43 +02:00
Pierre Schweitzer 8a8cb4d890
[NTOSKRNL] Only consider SharedCacheMap value once ViewLock is acquired.
This avoids a really nasty race condition in our cache controler where
two concurrents could try to initialize cache on the same file.
This had two nasty effects: first shared map was purely leaked and erased
by the second one. And the private cache map, allocated on the first shared
cache map couldn't be freed and was leading to Mm BSOD (free in a middle of
a block).

This was often triggered while building ReactOS on ReactOS (with multi threads).
With that patch, I cannot crash anylonger while building ReactOS.

CORE-14634
2018-05-23 08:41:46 +02:00
Pierre Schweitzer 65e29b4b1f
[NTOSKRNL] Optimize a bit deferred writes.
In the lazy writer run, first post items that are queued for this.
Only then, start executing deferred writes if any.
If there were any, reschedule immediately a lazy writer run, to keep
Cc warm and to make it unqueue write faster in case of high IOs situation.
To make second lazy writer run happen faster, we keep our state active to
use short delay (1s) instead of standard idle (3s).
2018-05-02 23:33:45 +02:00
Pierre Schweitzer 54c049bd6e
[NTOKSNRL] Always flush dirty VACB.
Recent changes seem to show that it's not
required to be exclusive on VACB to be able
to flush it.

This commit goes with f2c44aa and fixes the
last issues going with copying huge files.
There are no longer BSODs (be it in Mm or Cc).
I could, with 750MB RAM extract a 2GB file from
a 53MB archive and copy a 2,5GB file from a VBox
share to the disk. Note that writes are often
deferred, so if copy works, it's not that fast for now.

Note that it also brings some beloved behavior from
Windows: copy times are totally unreliable now when
writes are deferred. Little remaining times when
actively copying, high remaining times when deferred
writes in action. And goes between both... Sorry! ;-)

https://xkcd.com/612/

CORE-9696
CORE-11175
2018-04-30 22:24:30 +02:00
Pierre Schweitzer 74c5d8b6bd
[NTOSKRNL] Free unused VACB when required.
Same mechanism exists in Windows (even their Cc
is way different from ours...) where when Cc is
out of memory (in their case, out of VACB), we
will start scavenge old & unused VACB to free
some of the memory.

It's useful in case we're operating we big files
operations, we may run out of memory where to map
VACB for them, so start to scavenge VACB to free
some of that memory.

With this, I am able to install Qt 4.8.6 with 2,5GB of RAM,
scavenging acting when needed!

CORE-12081
CORE-14582
2018-04-30 12:10:24 +02:00
Pierre Schweitzer cc54e51495
[NTOSKRNL] Unmark dirty first, and then write.
This will avoid trying to flush twice a dirty VACB under
high IOs pressure.

CORE-14584
2018-04-30 10:36:19 +02:00
Pierre Schweitzer f2c44aa483
[NTOSKRNL] Fix lazy writer for in-use VACB.
Adjusting refcount and enabling lazy-write for pinned
VACB makes it actually more efficient, often purging
data to disk, reducing memory stress for the system.

This is required for defering writes.

This commit unfortunately (?) reverts a previous revert.

CORE-12081
CORE-14582
CORE-14313
2018-04-29 20:42:53 +02:00
Pierre Schweitzer 2ea6de8a42
[NTOSKRNL] Also try to extract name from FCB when leaking VACB 2018-04-27 19:01:35 +02:00
Pierre Schweitzer 43836b0fbb
[NTOSKRNL] In !filecache, try to display FCB name
When no name is set in the file object, try to read the name
from the FCB. We only support FastFAT (ours) FCB for now.

This is clearly a hack, but for a kdbg command, so ;-)
2018-04-27 18:57:30 +02:00
Pierre Schweitzer 579a784e04
[NTOSKNRL] In case we leak a VACB, debug as much information as possible.
CORE-14578
2018-04-27 14:14:56 +02:00
Pierre Schweitzer fcf83315dc
[NTOSKRNL] Noisily dereference mapped VACB on cache release.
It seems that on process killing, some VACB may be deleted while
still mapped. With current reference counting, they will actually
not be deleted, but leaked, and an ASSERT will be triggered.

CORE-14578
2018-04-27 10:23:06 +02:00
Pierre Schweitzer fd3a6c1089 [NTOSKRNL] Properly reset VACB on free
CID 1434271
2018-04-15 22:52:53 +02:00