hbirr@svn.reactos.com wrote:
- Started the rewriting of the cache manger.
- The new cache manager uses section objects and maps the content of cached files
into the kernel address space for functions like CcCopyRead.
- The implementation isn't complete. It works for me to compile ros on ros if enough ram is available.
Very good work!
I tried it and needed the attached changes in order to boot... [cc02.diff] Don't try to map data past the end of file in VFAT. [cc03.diff] Fix incorrect check in CcMapData.
Also I've patch which fixes fast mutex implementation and it's usage, which depends on these cache manager changes.
Regards, Filip
Filip Navara schrieb:
I tried it and needed the attached changes in order to boot...
Thanks for testing my changes.
[cc02.diff] Don't try to map data past the end of file in VFAT.
I think that this patch isn't necessary, because the real problem is in pin.c.
[cc03.diff] Fix incorrect check in CcMapData.
The check which you have changed was correct. Currently the file size is limited to the number of cache view entries. I will implement a x-level tree structure in the future. But some other checks are incorrect. I've add a patch.
Also I've patch which fixes fast mutex implementation and it's usage, which depends on these cache manager changes.
The global lock (fast mutex) implementation is a little bit dirty. Currently I implement a function which does remove the unneeded data section objects after some time. My problem is the sequence to acquire the segment lock and the global lock.
- Hartmut
E:\Sandbox\ros_cache\reactos>set SVN_EDITOR=notepad
E:\Sandbox\ros_cache\reactos>d:\programme\subversion\bin\svn.exe diff ntoskrnl\cc\pin.c Index: ntoskrnl/cc/pin.c =================================================================== --- ntoskrnl/cc/pin.c (Revision 13570) +++ ntoskrnl/cc/pin.c (Arbeitskopie) @@ -67,20 +67,26 @@
Bcb = FileObject->SectionObjectPointer->SharedCacheMap;
- if (FileOffset->QuadPart + Length > Bcb->FileSizes.AllocationSize.QuadPart) + if (FileOffset->QuadPart >= Bcb->FileSizes.FileSize.QuadPart) { - DPRINT("%d %I64d %I64d\n", Length, FileOffset->QuadPart + Length, Bcb->FileSizes.AllocationSize.QuadPart); -// KEBUGCHECK(0); + CHECKPOINT1; + *piBcb = NULL; + *pBuffer = NULL; + return FALSE; }
- if (FileOffset->QuadPart + Length - ROUND_DOWN(FileOffset->QuadPart, CACHE_VIEW_SIZE) > CACHE_VIEW_SIZE) + if (FileOffset->QuadPart + Length > Bcb->FileSizes.FileSize.QuadPart) { - /* not implemented */ + CHECKPOINT1; + Length = Bcb->FileSizes.FileSize.QuadPart - FileOffset->QuadPart; + } + + if (ROUND_DOWN(FileOffset->QuadPart, CACHE_VIEW_SIZE) + CACHE_VIEW_SIZE != ROUND_UP(FileOffset->QuadPart + Length, CACHE_VIEW_SIZE)) + { + /* Mapping over the border of a cache view isn't implemented */ KEBUGCHECK(0); }
- - if (Bcb->FileSizes.AllocationSize.QuadPart > sizeof(Bcb->CacheView) / sizeof(Bcb->CacheView[0]) * CACHE_VIEW_SIZE) { /* not implemented */
Hartmut Birr wrote:
[cc02.diff] Don't try to map data past the end of file in VFAT.
I think that this patch isn't necessary, because the real problem is in pin.c.
At first I did the same thing as you did in the attached patch. Then I actually followed it by some testing on Windows and _it is possible to map data past the end of file_ with CcMapData. I haven't tried to access the mapped memory though...
[cc03.diff] Fix incorrect check in CcMapData.
The check which you have changed was correct. Currently the file size is limited to the number of cache view entries. I will implement a x-level tree structure in the future. But some other checks are incorrect. I've add a patch.
Oh, sorry, I didn't realize it's checked because of multi-level VACB tables. A comment would have been helpful ;-)
Also I've patch which fixes fast mutex implementation and it's usage, which depends on these cache manager changes.
[snip]
To clarify this, the fix I have fixes some small issues with FAST_MUTEX implementation and changes it to actually raise to APC_LEVEL instead of entring critical section. Many people (especially Mike Nordell ;-) have been complaining about it for some time and all previous attempts to fix it failed because of the broken locking in cache manager. Now that you have done the dirty work, it's finally possible to correctly fix this issue... ;-)
Regards, Filip
Filip Navara wrote:
Hartmut Birr wrote:
[cc02.diff] Don't try to map data past the end of file in VFAT.
I think that this patch isn't necessary, because the real problem is in pin.c.
At first I did the same thing as you did in the attached patch. Then I actually followed it by some testing on Windows and _it is possible to map data past the end of file_ with CcMapData. I haven't tried to access the mapped memory though...
I did more testing and the result is the attached patch (comment inside). Does it look ok?
Regards, Filip
Index: section.c =================================================================== --- section.c (revision 13615) +++ section.c (working copy) @@ -866,15 +866,20 @@
FileOffset.QuadPart = SegOffset + SectionData->Segment->FileOffset; Length = PageCount * PAGE_SIZE; - if (SegOffset > SectionData->Segment->RawLength) + + if (SegOffset >= SectionData->Segment->RawLength) { - KEBUGCHECK(0); + /* It is perfectly valid to map area past the end of file with CcMapData + * and then access the pages. In this case the page fault doesn't result + * in I/O request and we get zeroed pages. + */ + return STATUS_SUCCESS; } + if (Length + SegOffset > SectionData->Segment->RawLength) { Length = SectionData->Segment->RawLength - SegOffset; } -
Status = MmspRawReadPages(SectionData->Section->FileObject, SectorSize, @@ -1717,7 +1722,7 @@ Status = MmspReadSectionSegmentPages(SectionData, Offset, PageCount, Pfn); if (!NT_SUCCESS(Status)) { - DPRINT1("MiReadPage failed (Status %x)\n", Status); + DPRINT1("MmspReadSectionSegmentPages failed (Status %x)\n", Status); } MmLockAddressSpace(AddressSpace); if (!NT_SUCCESS(Status))
Filip Navara schrieb:
Filip Navara wrote:
Hartmut Birr wrote:
[cc02.diff] Don't try to map data past the end of file in VFAT.
I think that this patch isn't necessary, because the real problem is in pin.c.
At first I did the same thing as you did in the attached patch. Then I actually followed it by some testing on Windows and _it is possible to map data past the end of file_ with CcMapData. I haven't tried to access the mapped memory though...
I did more testing and the result is the attached patch (comment inside). Does it look ok?
Regards, Filip
I think that we must add more checks and that the change is at the wrong position. The zero pages at the end of the file should be mapped like the image sections with segments whose raw length is shorter than the real length. What should we do if pages past the end of the file are modified and the section object is deleted?
- Hartmut