>> - First, dsk.h defines the maximum sector size (MAX_SEC_SIZE) as 2048  
>> (as
>> the commit comment says) while the relevant LoL entry (maxsecsize) in
>> kernel.asm is initialised to 4096.
> Yes - this is simply for testing purposes; I plan to default both to
> 512.  I would prefer both set from the same define to ensure they stay
> in sync - I am aware they are different, I am tracking down how to fix
> support for larger than 2KB sectors and MAX_SEC_SIZE will be set to
> 4096 to test or possibly eliminated in favor of using only maxsecsize.

Ah, yes, Eric speculated that this might be a proof of concept for now,  
lacking an "unstable" branch. I happen to keep track of kernel commits  
recently. Sorry that I already reviewed this premature implementation!

Values should definitely be tied later, either by consistently using  
MAX_SEC_SIZE, or (preferably) by initialising maxsecsize and the init  
buffer's size with MAX_SEC_SIZE and then always referring to maxsecsize  

>> - Third, if I'm not entirely mistaken, the patch makes all buffers grow  
>> to
>> 2048 bytes, even in the absence of any block devices with large sectors.
>> This will unnecessarily waste memory on most systems.
> yes, buffers are allocated using size of max sector size so will waste
> lots of memory if > 512 byte sectors are used.  I will be committing
> updates to ensure max sector size defaults to 512 so as to avoid waste
> of space.

Regarding that, I once considered that unused buffer space could be used  
to load multiple smaller sectors. Say, if you use 4096-byte buffers but a  
particular drive has 512-byte sectors, you can load eight sectors at once  
whenever reading. Would have to align sector reads to those boundaries  
though, plus a special check for the end of the file system. Additionally,  
the "written" bit would either have to be expanded to a bitmap, or you'd  
have to write eight sectors together all the time too. This would probably  
abandon any MS-DOS compatibility of the buffer layout, if the kernel  
currently does have any (not sure).

>> This scheme could obviously be extended by additional tools/interfaces  
>> to
>> increase the buffer size later on, and a CONFIG option to artificially
>> increase it in anticipation of a post-CONFIG requirement.
> agreed, a config option to allow manually specifying a larger value
> than needed at boot time - should buffers be extended to
> BUFFERS=COUNT#,READAHEAD#,SECTORSIZE# or would it be better to add a
> whole new command MAXSECSIZE=SECTORSIZE#  ?  (the latter seems simpler
> so the one I will plan on for now).

Yes, Eric suggested that one too. Plus, I think it's preferable because  
then a user can specify MAXSEXSIZE without having to specify the first two  
values for BUFFERS.

> I don't see us using max sector size < 512, but using tdsk with
> smaller than 512 sector size does seem to work - though at this time I
> don't see me researching  if it correctly works or merely happens to
> work on my test setup.

I think you need to review at least dsk.c around line 388, where it says  
"pbpbarray->bpb_nbyte % 512" now. I didn't look into what that check  
actually does, but it sure seems like it would have to be adjusted. While  
you're at it, you can make it the proper check for powers of two between  
32 (or 64,128, regarding (E)(N)BPB size) and 8192 (or 2048 for now).

> currently any value > 3KB causes memory corruption if
> too many buffers are specified (I have successfully gotten 4KB sectors
> to finish booting by changing buffers=2).

> I currently have tracked down
> part of my issue to allocation problems if buffers is too large, but
> still working on relearning the init time segment relocations and
> order to determine a proper fix for buffer allocations.  It would help
> if printf worked correctly in all the init code...

The virtues of high-level language kernel development ;)

C. Masloch

Try before you buy = See our experts in action!
The most comprehensive online learning library for Microsoft developers
is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3,
Metro Style Apps, more. Free future releases when you subscribe now!
Freedos-kernel mailing list

Reply via email to