Commit graph

32 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 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 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 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 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
Pierre Schweitzer 0fbdf31709
[NTOSKRNL] Add the CcPinReadWait and CcPinReadNoWait counters 2018-02-24 14:52:04 +01:00
Pierre Schweitzer 45964099f3
[NTOSKRNL] Return some Cc counters in SystemPerformanceInformation 2018-02-24 13:36:26 +01:00
Pierre Schweitzer 227c4321c2
[NTOSKRNL] Add the CcMapDataWait and CcMapDataNoWait counters 2018-02-24 13:36:26 +01:00
Pierre Schweitzer de897cbe3d
[NTOSKRNL] Revert 2362e0f and 07e6e9c (partly) they seem to have caused high instability in Cc.
This reverts BCB being lazy written when marked dirty.

We'll go back to this behavior when this part will have been reworked and stabilized.

CORE-14263
CORE-14279
CORE-14285
2018-02-08 13:40:54 +01:00
Pierre Schweitzer 52287be9a9
[NTOSKRNL] Misc fixes to Cc:
- CcUnpinDataForThread() only release VACB when the last BCB reference is gone. This avoids having a valid BCB with an invalid VACB
- CcRosMarkDirtyVacb() will only accept non-dirty VACB now. This avoids a major bug where a an already dirty VACB was over-dereferenced
- Thanks to previous point, simplify CcRosUnmapVacb(), CcRosReleaseVacb() implementation
- And only set VACB dirty once in CcSetDirtyPinnedData()
- Add a few sanity checks

With that I can again install ReactOS with 128MB RAM :-).

CORE-14263
CORE-14268
2018-01-28 11:14:15 +01:00
Pierre Schweitzer 07e6e9c9c1
[NTOSKRNL] When marking a BCB dirty, also mark the underlying VACB dirty.
Experiment and MSDN tend to show that a dirty BCB is queued for lazy write.
This will do the job here!

Also, renamed CcRosMarkDirtyFile() which is more accurate, and added a new
function CcRosMarkDirtyVacb() which just takes a VACB as arg (expected locked)
and marks it dirty (using previous implementation). Make CcRosMarkDirtyFile()
use it.

CORE-14235
2018-01-23 23:25:26 +01:00
Pierre Schweitzer 371aba1abe
[NTOSKRNL] When mapping data in CcMapData(), don't truncate offset to ULONG.
This would affect reads/writes on large volumes where offset is higher than what a ULONG can hold.

This really nasty bug was hitting CcMapData() but also CcPinRead() (due to the nature of its implementation)
and both were returning garbage data under certain circumstances with Ext2Fsd.

This should (I hope!) help some other FSDs to work better in ROS.

CORE-12456
2018-01-13 21:30:37 +01:00
Colin Finck c2c66aff7d Git conversion: Make reactos the root directory, move rosapps, rostests, wallpapers into modules, and delete rossubsys. 2017-10-03 07:45:34 +00:00
Renamed from reactos/ntoskrnl/cc/pin.c (Browse further)