Commit Graph

120 Commits

Author SHA1 Message Date
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
Pierre Schweitzer
953dc72dad [NTOSKRNL] Drop the VACB lock.
This has have several benefits for ReactOS Cc:
- It helps reducing potential deadlocks situations in Cc
- It speeds up ReactOS by reducing locks
- It gets us a bit closer to Windows VACB

CORE-14349
2018-04-15 22:52:53 +02:00
Pierre Schweitzer
40017a54f9 [NTOSKRNL] Use interlocked operations when dealing with map count.
CORE-14349
2018-04-15 22:52:53 +02:00
Pierre Schweitzer
1b672981e2 [NTOSKRNL] Map the VACB in kernel space before inserting it in lists.
The avoids race conditions where attempts to read from disk to
not fully initialized VACB were performed.
Also, added more debug prints in such situations.

CORE-14349
2018-04-15 22:52:53 +02:00
Pierre Schweitzer
42df4683d7 [NTOSKRNL] Add extra sanity checks for VACB lists.
We now always initialize list members from the VACB
and make sure the list entry has properly been removed
from the list before free.

CORE-14349
2018-04-15 22:52:53 +02:00
Serge Gautherie
6618a2fd2c [NTOS:CC] Use UNIMPLEMENTED_ONCE instead of custom code
- Rewrite e319f85e67.
2018-04-07 12:00:10 +02:00
Pierre Schweitzer
ffd524275e
[NTOSKRNL] Properly delete VACB in CcRosCreateVacb() when mapping fails.
Spotted by Thomas.

CORE-14478
CORE-14502
2018-03-25 18:27:19 +02:00
Pierre Schweitzer
14b05e65ff
[NTOSKRNL] Use interlocked operations for VACB reference counting.
CORE-14480
CORE-14285
2018-03-24 19:15:58 +01:00
Pierre Schweitzer
dea9c291ab
[NTOSKRNL] Add a few asserts when mapping a VACB in kernel space
Also, reset VACB content when returning it to the lookaside list

CORE-14478
2018-03-24 19:15:58 +01:00
Pierre Schweitzer
2b6df67f0a
[NTOSKRNL] More asserts regarding reference count
CORE-14285
CORE-14480
2018-03-24 11:59:45 +01:00
Pierre Schweitzer
1e579843bc
[NTOSKNRL] Always reference a newly created VACB
This allows being consistent between newly created and looked up
so that VACB can always safely be released.

Should really help with reference issues.

CORE-14481
CORE-14480
CORE-14482
2018-03-18 18:16:55 +01:00
Pierre Schweitzer
2a0e996c9d
[NTOSKRNL] In CcRosInternalFreeVacb(), in case of invalid free, also print file name.
CORE-14481
CORE-14480
CORE-14482
2018-03-18 13:21:54 +01:00
Pierre Schweitzer
2fbba22789
[NTOSKRNL] In CcFlushCache(), release the VACB using CcRosReleaseVacb()
Instead of reimplementing it partially and wrongly.

CORE-14481
CORE-14480
CORE-14482
2018-03-18 13:21:54 +01:00
Pierre Schweitzer
13b57fb5b5
[NTOSKRNL] Misc fixes to VACB reference counting
This fixes various bugs linked to VACB counting:
- VACB not released when it should be
- Reference count expectations not being accurate

For the record, VACB should always have at least a reference
count of 1, unless you are to free it and removed it from
any linked list.

This commit also adds a bunch of asserts that should
help triggering invalid reference counting.

It should also fix numerous ASSERT currently triggered and
may help fixing random behaviours in Cc.

CORE-14285
CORE-14401
CORE-14293
2018-03-17 11:56:25 +01:00