Improving disk performance by changing VirtualMemory path

[haiqu]

Currently the haiku BFS is a little slow. My guess is that DMA transfers have not yet been enabled.

There is a simple interim solution. By allowing the user to select a virtual memory path on a separate drive - preferrably on another bus entirely - disk swapping speed can be increased significantly. This is due to the fact that the physical heads in your drive need to change what they're doing to service a virtual memory call.

By placing the virtual memory on a separate bus, optimum improvement is gained due to the fact that the PC's controller is now less loaded, and because there is now less data on the bus.

I believe this would be a fairly simple change, and very worthwhile even in the long-term. Checking to ensure the drive is mounted and recreating virtual memory if this fails is about all the extra work required.

No change would be noticed in systems with a single HDD, but then again no improvement is available from Haiku's ability to run SMP either if you only have one CPU ...

[hamish]

I completed basic support for this on the preflet side in a Google Code-In patch, and I think I can implement it in the kernel too (if everyone approves).

I have a couple of questions:

Is it acceptable to use the C++ APIs in the kernel code -- in order to test for free space on drives and such -- or would it be preferable to use C functions such as those defined in kernel/fs_info.h?

Where should the swap file be stored on drives that aren't the boot drive? Simply in the root directory of the drive, or in /drive/var?

[bonefish]

Replying to hamish:

Is it acceptable to use the C++ APIs in the kernel code -- in order to test for free space on drives and such -- or would it be preferable to use C functions such as those defined in kernel/fs_info.h?

Most C++ API is simple not available in the kernel. Also only part of the userland C API is implemented in the kernel. You might need to use the private kernel API. E.g. for getting information about and manipulating disk devices, the disk device manager interface can be used (cf. ​haiku/trunk/headers/private/kernel/disk_device_manager).

Where should the swap file be stored on drives that aren't the boot drive? Simply in the root directory of the drive, or in /drive/var?

I'd store it in the root directory or make that user-configurable.

[axeld]

Replying to hamish:

Is it acceptable to use the C++ APIs in the kernel code -- in order to test for free space on drives and such -- or would it be preferable to use C functions such as those defined in kernel/fs_info.h?

In addition to what Ingo wrote: the functions in kernel/ should pretty much all be available (with the exception of spawn_thread() in favor of spawn_kernel_thread() IIRC), and might provide enough data for the issue.

[hamish]

The main issue is getting the name of a volume (the name that appears in Tracker) before it's mounted. Nothing in the disk_device_manager headers seems to facilitate this. KPartition::Name() appears to give a name identifying the hard-disk model, KDiskSystem::Name() the filesystem type, etc.

[bonefish]

The <fs_info.h> C API is also available in the kernel. It allows you to get the volume and root node ID for any mounted volume. Given such a pair (aka node ref) you can employ the private VFS API (header/private/kernel/vfs.h) to get a path (vfs_entry_ref_to_path()), get the file's stat data (vfs_stat_node_ref()), or open it (vfs_entry_ref_to_vnode() plus vfs_open_vnode()).

The disk device manager API might be interesting, if the volume for the swap file is not even mounted yet and you need to find and/or mount it first.

[hamish]

Yeah, my original idea was to use dev_for_path() from fs_info.h, but I realised that swap is initialized before the mount server is started, so only the boot device would be mounted.

I've used the disk device manager to loop through the unmounted disks (KDiskDeviceManager::NextDevice and KPartition::VisitEachDescendant) but there seems to be no way to actually check the names of the volumes (which are needed, as the swap volume is specified in the preflet by name).

[bonefish]

Replying to hamish:

but there seems to be no way to actually check the names of the volumes (which are needed, as the swap volume is specified in the preflet by name).

Try KPartition::ContentName().

[hamish]

Bingo. Thanks!

Should be easy going from here.

[anevilyak]

Isn't that potentially a bit fragile? As I recall nothing prevents two different volumes from having the same name.

[axeld]

Indeed, I was just about to make the same comment. Hamish, you could have a look at how the mount server decides which volumes to mount upon start; it has a scoring system that has the volume name, the device path, the size of the device, and eventually other things.

That helps not losing the device (or choosing the wrong one) when something changed or there are several options around.

Additionally, if it couldn't find the swap device anymore, a warning could be given.

[hamish]

Looking at the mount server code, it seems a little simpler than that. It only mounts the volume if both the device path and the volume name match what's stored in the settings file (anything less and it doesn't mount). Should I follow its example, or go with a scoring system?

Currently I have it set up so that if any error occurs with an alternate swap device, it falls back on the boot device to store the swap file.

[axeld]

Hm, maybe stippi has optimized it away, or maybe it's only used in other stuff; I was referring to https://dev.haiku-os.org/browser/haiku/trunk/src/kits/tracker/Utilities.cpp#L988 -- I guess copying the mount server behavior would be okay, too, though, as long as the user is prompted when anything goes wrong.

[hamish]

Ah. I was looking at ​http://dev.haiku-os.org/browser/haiku/trunk/src/servers/mount/AutoMounter.cpp#L503

That code in Utilities.cpp looks good -- I'll adapt that.

With regards to prompting the user, how does one do that in the kernel without BAlert?

[axeld]

The only way would be testing the configuration in the Bootscript.

[hamish]

In the volume-match scoring code linked in comment 12, the most important element to the final score is the creation time associated with the root directory of the volume.

I'm not sure how to retrieve this creation time for an unmounted volume -- I originally thought of stating the device path under /dev/disk/... but the creation time returned from here is not the same.

[axeld]

There is no mechanism to do that; I would just leave it out.

[hamish]

Apologies for not posting any updates: the power supply on the machine I use for development failed, and I'm waiting for a replacement. The hard disk is fine, and my work is all backed up, so I'll be able to resume from where I left off once I'm up and running again.

[hamish]

I've got swap (seemingly) working on other partitions now.

I'm working on modifying the preflet to give a better overview of the available volumes for swap. I've attached two screenshots so I can get a second opinion before I continue...

I thought perhaps a list view might be more appropriate than a menu field considering the width of the items. Should I even bother showing the path of the device?

[diver]

FWIW, I don't think that showing the path of the device is needed in this preflet.

[hamish]

Yeah, I didn't think so either.

I replaced the menu field with a column list view with "Name", "Free space", and "Capacity" columns and a "Device path" column that's hidden by default, and it looks much better.

However, I've noticed a problem with BColumnListView. While BStringColumn correctly respects font size and sizes itself accordingly, the BRow class is fixed-height.

Its height is given in the constructor, so it's possible to just pass in the current font height, but a nicer solution would be a BStringRow class which automatically sets its height as per the font. Is it possible for me to implement this without breaking binary compatibility?

[axeld]

First of all, I really think the popup menu field is the way to go here, just with less information (I'd even consider removing the capacity as well from there). I also would only show mounted volumes.

Second of all, I don't think you should bother with BColumnListView a lot - this class has never gone official (was experimental in Be times), and is supposed to be replaced with something real one day (think model view controller).

[hamish]

I think I'm pretty much done with the preflet.

Now the swap volume selection menu field simply contains the volume name, and the mount point (so volumes with the same name can be distinguished from one another). I've also hooked it up to the node monitor.

I was wondering if it might be a good idea to create a small userland interface to the vm code in the kernel (to return info on the current swap files, to check if an error occurred during vmem init, etc).

That way, the preflet could accurately report the size of the current swap file without having to guess which volume it's on (and it would prevent it from being fooled by a user-created file called "swap").

It would also solve the problem of reporting any error finding the swap volume at boot (without needing to resort to a nasty method like writing out a file to the hard drive and checking it later).

[hamish]

Swap volume support in kernel for #3723

[hamish]

Swap volume support in preflet for #3723

[hamish]

Swap volume check utility for #3723

[hamish]

There's the patches. I created a bunch of drives in VirtualBox to test and it seems to work fine.

Also, I wrote a little copy of the BPartition::GetMountPoint function in VMAnonymousCache.cpp for generating a unique mount point. Maybe this could be moved into KPartition (and also I could implement KPartition::Mount)?

[axeld]

Thanks, and sorry for the delay! It looks pretty good, but I have a few minor coding style related complains:

• in swapcheck.cpp: two blank lines between sections, the main() goes to the next line.
• two blank lines between functions is also sometimes violated (for example, in SettingsWindow.cpp)
• 'vol', and 'cap' aren't good names; you named them properly at other places.
• Some of the lines in Settings.cpp (maybe at other places, too) are unnecessarily broken into two lines, even though they don't have more than 80 characters.

It would be great if you could rework this, otherwise, I guess someone else would need to do this sooner or later.

[hamish]

Fixed all the style violations I could find. Ran it through the style checker too and it seems all right.

[leavengood]

Now that we are talking about fixing Haiku's VM system again, I think it would be a good time to revisit this ticket.

Hamish's patches look pretty good to me. I'm not sure we should add this feature so late in the alpha4 release process, but I would certainly like to get these patches applied soon after the release. Though if we want to properly fix the other VM issues for alpha4, this includes some helpful code for that (such as checking the available space in a volume before setting the swap size.)

Sorry Hamish that such good work has been left sitting. But at least we have kept you busy on the Java port in the GSoC :)

[kallisti5]

I'm working through this now and have the first kernel patch prepared.

see #7742

[kallisti5]

The kernel vm patch was applied to master in hrev44611 and hrev44612. The release coordinator has mentioned a soft acceptance of the changes to the kernel vm.

I'm working through the gui changes now.. there seem to be a few bugs left. (the current preflet code should function with the kernel changes post-hrev44612 however)

[leavengood]

This can probably be marked as fixed now, right?

Also Alexander, while you are in no way obligated to fix all the VM issues, maybe you could take a look at #4843 since knowledge of the code is fresh in your mind. I think it is still an issue.

[kallisti5]

resolved post hrev44622

I'll look at #4843

#8971 opened. It's minimal, but an issue. (definitely not an R1A4 blocker though)