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
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
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()
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.
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.
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.
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.
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.
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
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
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
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
- 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
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
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
