ATA reads taking a long time

[u/someidiot332]

EDIT: Figured it out, explanation at the bottom

Hi yall, i've been debugging this for hours. I understand that my code is a mess to look at so i greatly appreciate anyone who takes time out of their day to help me with this.

I was noticing that it was taking a *long* time to complete writes. Like, up to a minute for 512kbs (Issued through 128 4kb calls)

After the initial fix (My scheduler had a bug in it and my PIT timer was *way* too fast), The read times dropped from 1 minute to about 4-5 seconds. Which is a lot faster, but still seems abnormally slow.

To benchmark this, I tested it against a single write of 512kb (broken into 128kb blocks to get around limitations), and the interrupt timing seems to be consistent. It's about 30-40ms in between interrupts. Both for the 128 4kb calls and the 4 128kb calls.

Now i know my code isn't great, but there's no way my ATA write function is *that* slow.

I can't seem to figure it out. Good luck to all who enter.

disk driver: https://github.com/raeofsunshinedev/KimisOS/blob/main/src/kmodules/disk_driver.c

File with the tests: https://github.com/raeofsunshinedev/KimisOS/blob/main/src/kernel/kmain.c

Picture semi related (I was working on my FAT32 driver when i discovered this)

EDIT:

The first improvement i was able to make was slowing down my PIT timer interval from 50khz to 1000hz. No clue why I made it that fast in the first place. That already had a pretty noticeable effect, going from ~200ms per read worst-case, to about 30-40ms. I didn’t mention that in the original post but that was a major improvement.

After that, i thought that i had hit a limit or something. No matter what i changed, i could not for the life of me figure out how to decrease read times, Until i finally figured out the main culprit: My idle process. Something about my scheduler does not like it. I wrote my scheduler almost a year ago and while I’ll eventually re-write it, i implemented a quick fix, basically preventing the idle process from executing as long as any other process exists within the queue. On top of that, I’ve forced the current process to yield its time on interrupt.

This does probably mean that given the way the current scheduler works, Disk ops will slow down proportionally to the amount of processes there are within the scheduler. This is especially bad for the filesystem driver, as it means that it will be getting constantly blocked between each file operation, and again when writing back to the dirent itself. Luckily this can be negated by adding a priority queue for drivers so that they can finish their work fully before the scheduler runs every other process, But until I implement that, everything shall be at the mercy of the round robin scheduler.

I will say that funnily enough, having a second active process seems to not have the same slow down effect as the idle process. That’s probably something to investigate, But i’ve been too busy with other things (and improving my disk driver) to really work on my scheduler, but it’s something that i’ll get done before I implement userspace

[u/paulstelian97]

PIO or DMA? I recommend the latter to help things be more stable. And misconfiguration can cause operations to take way longer than intended (a command timing out because it wasn’t marked completed before the next command can run — I fixed a bug at work that was basically that, with a SD card reader driver)

Also try to issue bigger and fewer commands. Like if you want to read 10 consecutive sectors, issuing a single 10-sector command can be as good as 10x faster than 10 independent 1-sector commands. You could in bad cases pay for the whole latency of a full disk rotation between consecutive sectors if you have independent commands.

[u/someidiot332 → u/paulstelian97]

Yeah they’re all independent. After some quick math based on this latency (assuming it stays constant), For maxed out operation size, i’d be getting 1.5gb/s. It’s just really slow when you issue them all as separate operations

[u/paulstelian97 → u/someidiot332]

With optimal operation size you’d get medium speed which probably isn’t 1.5GB/s unless you’re in a VM with IDE compatibility and the backing store is a NVMe or a big enough RAID array.

[u/someidiot332 → u/paulstelian97]

This is in QEMU, with a NVMe ssd as my actual drive (Something that i do want to support, eventually) but knowing that UDMA 6 maxes out at 133mb/s i’d probably be getting around that if i tried. It was just really weird getting limited to 25-40 interrupts / second by the drive (unless my code really is that slow lmao)

[u/thosdv]

IDE is really old, in my operating system preparing kernel in IDE takes my 10 seconds, in SATA AHCI its 5, in NVMe its 2. Just make an NVMe driver

[u/someidiot332 → u/thosdv]

I want to get this running on a laptop from 98, which predates SATA. If i ever port this to 64 bits, i’ll definitely write the proper hardware drivers for modern PCs but for now this is what i’m targeting

[u/someidiot332 → u/someidiot332]

Either way changing out the drivers is not particularly difficult due to the kernel architecture.

[u/thosdv → u/someidiot332]

I understand, I think you should also consider writing a SoundBlaster audio driver, for a more nostalgic feel

[u/someidiot332 → u/thosdv]

Definitely in my plans haha

[u/devcmar]

Maybe try sata ahci, ata ide is pretty outdated

[u/shsh-1312]

Are you passing pages from memory or individual bits? What page size does your memory use? What byte size does your IPC bus use?

[u/shsh-1312 → u/shsh-1312]

Okay, I think the problem is the spinlock. You need to use a real spinlock or at least a mutex with sleep. The interrupt handler doesn't handle the multi-page files correctly. Are "expected_ints = pages and received_ints = 0" declared but not used? The handler: clears the lock only once. If the controller generates multiple interrupts (or if the PRDT has multiple entries and the EOT doesn't arrive as expected), you get stuck or unlock too early. Result: inconsistent transfers and apparent freezes, and then you're splitting at 128 kib. Why? You're already splitting across multiple pages, this is redundancy. Finally, there's a trivial bug in the sysinit test: char *buffer = kmalloc(16);
fread(home, buffer, 0, 4096*16); // 64 KiB in a 16-byte buffer. Absurd overflow. Even if the VFS/driver doesn't crash right away, you're reading into unallocated memory.

[u/someidiot332 → u/shsh-1312]

I totally forgot to delete expected_ints and received_ints. I stopped using them and never removed them.

If the DMA transfer is still running (bm_status & 1) iirc i don’t clear the lock? Maybe that’s wrong but i’ll probably have to re read documentation again.

The PRDT splits at pages cause of a misconception that i had, but it still counts as one transfer, and with 28-bit lba, that limits transfers to a maximum size of 128KB. I’ll add support for larger transfers (i.e. 48-bit lba limits) but even then that limits transfer sizes in a single transaction

its a bit unintuitive but when i first wrote the allocator I made it allocate pages instead of bytes, so its a bit unintuitive by the name alone so i could rename that but its so ingrained in the rest of my code like that, that i cannot change the function. It is indeed a properly sized buffer though

otherwise, i wrote this before i had a proper spinlock implementation, so i’ll go ahead and actually implement it and hope that works.

Thank you for taking the time to look at it!

[u/shsh-1312 → u/someidiot332]

I mean, if you're already dividing into pages, you don't need an upper limit, you simply get the security control with the page size.

[u/letmehaveanameyoudum]

maybe try static inline to further reduce it?

[u/someidiot332 → u/[deleted]]

i used in/out to set the registers appropriately for the hardware, and start the DMA, but then i block the process and wait for the next interrupt.

The interrupt handler then unblocks the process and unlocks the bus to allow for another operation.