ATA: Fix Slow Operation (#296)

* Use process_yield instead of clock_wait within ata_wait, otherwise polled disk access is much too slow.

* newline

* kshell install takes explicit device names

* Don't automount by default.

* Fix incorrect name comparison: accidentally matching prefixes.

* Fix typo in kshell:install

* Make DISKFS_NAME_MAX a distinct symbol.
Check for name length in diskfs_dirent_create_file_or_dir.

* Use correct name length on comparison.

kernel/ata.c

@@ -130,7 +130,7 @@ static int ata_wait(int id, int mask, int state)
 			ata_reset(id);
 			return 0;
 		}
-		clock_wait(0);
+		process_yield();
 	}
 }
 

kernel/diskfs.c

@@ -273,6 +273,13 @@ int diskfs_dirent_close( struct fs_dirent *d )
 	return 0;
 }
 
+/* Returns true if two strings a and b (with lengths) have the same contents. Note that diskfs_item.name is not null-terminated but has diskfs_item.name_length characters. When comparing to a null-terminated string, we must check the length first and then the bytes of the string. */
+
+static int diskfs_name_equals( const char *a, int alength, const char *b, int blength )
+{
+	return alength==blength && !strncmp(a,b,alength);
+}
+
 struct fs_dirent * diskfs_dirent_lookup( struct fs_dirent *d, const char *name )
 {
 	struct diskfs_block *b = page_alloc(0);
@@ -281,11 +288,13 @@ struct fs_dirent * diskfs_dirent_lookup( struct fs_dirent *d, const char *name )
 	int nblocks = d->size / DISKFS_BLOCK_SIZE;
 	if(d->size%DISKFS_BLOCK_SIZE) nblocks++;
 
+	int name_length = strlen(name);
+
 	for(i=0;i<nblocks;i++) {
 		diskfs_inode_read(d,b,i);
 		for(j=0;j<DISKFS_ITEMS_PER_BLOCK;j++) {
 			struct diskfs_item *r = &b->items[j];
-			if(r->type!=DISKFS_ITEM_BLANK && !strncmp(name,r->name,r->name_length)) {
+			if(r->type!=DISKFS_ITEM_BLANK && diskfs_name_equals(name,name_length,r->name,r->name_length)) {
 				int inumber = r->inumber;
 				page_free(b);
 				return diskfs_dirent_create(d->volume,inumber,r->type);
@@ -382,14 +391,18 @@ static int diskfs_dirent_add( struct fs_dirent *d, const char *name, int type, i
 
 struct fs_dirent * diskfs_dirent_create_file_or_dir( struct fs_dirent *d, const char *name, int type )
 {
+	if(strlen(name)>DISKFS_NAME_MAX) return 0; // KERROR_NAME_TOO_LONG
+
 	struct fs_dirent *t = diskfs_dirent_lookup(d,name);
 	if(t) {
 		diskfs_dirent_close(t);
 		return 0;
 	}
 
 	int inumber = diskfs_inumber_alloc(d->volume);
-	if(inumber==0) return 0; // KERROR_OUT_OF_SPACE
+	if(inumber==0) {
+		return 0; // KERROR_OUT_OF_SPACE
+	}
 
 	struct diskfs_inode inode;
 	memset(&inode,0,sizeof(inode));
@@ -454,7 +467,7 @@ int diskfs_dirent_remove( struct fs_dirent *d, const char *name )
 		for(j=0;j<DISKFS_ITEMS_PER_BLOCK;j++) {
 			struct diskfs_item *r = &b->items[j];
 
-			if(r->type!=DISKFS_ITEM_BLANK && r->name_length==name_length && !strncmp(name,r->name,name_length)) {
+			if(r->type!=DISKFS_ITEM_BLANK && r->name_length==name_length && diskfs_name_equals(name,name_length,r->name,r->name_length)) {
 
 				if(r->type==DISKFS_ITEM_DIR) {
 					struct diskfs_inode inode;

kernel/diskfs.h

@@ -38,12 +38,15 @@ struct diskfs_inode {
 #define DISKFS_ITEM_FILE 1
 #define DISKFS_ITEM_DIR 2
 
+/* Maximum name length chosen so that diskfs_item is 32 bytes. */
+#define DISKFS_NAME_MAX 26
+
 #pragma pack(1)
 struct diskfs_item {
 	uint32_t inumber;
 	uint8_t  type;
 	uint8_t  name_length;
-	char     name[26];
+	char     name[DISKFS_NAME_MAX];
 };
 #pragma pack()
 

kernel/kshell.c

@@ -25,7 +25,7 @@ See the file LICENSE for details.
 static int kshell_mount( const char *devname, int unit, const char *fs_type)
 {
 	if(current->ktable[KNO_STDDIR]) {
-		printf("root filesystem already mounted, please unmount first");
+		printf("root filesystem already mounted, please unmount first\n");
 		return -1;
 	}
 
@@ -85,15 +85,15 @@ to the disk volume dst by performing a recursive copy.
 XXX This needs better error checking.
 */
 
-int kshell_install( int src, int dst )
+int kshell_install( const char *src_device_name, int src_unit, const char *dst_device_name, int dst_unit )
 {
 	struct fs *srcfs = fs_lookup("cdromfs");
 	struct fs *dstfs = fs_lookup("diskfs");
 
 	if(!srcfs || !dstfs) return KERROR_NOT_FOUND;
 
-	struct device *srcdev = device_open("atapi",src);
-	struct device *dstdev = device_open("ata",dst);
+	struct device *srcdev = device_open(src_device_name,src_unit);
+	struct device *dstdev = device_open(dst_device_name,dst_unit);
 
 	if(!srcdev || !dstdev) return KERROR_NOT_FOUND;
 
@@ -105,7 +105,7 @@ int kshell_install( int src, int dst )
 	struct fs_dirent *srcroot = fs_volume_root(srcvolume);
 	struct fs_dirent *dstroot = fs_volume_root(dstvolume);
 
-	printf("copying atapi unit %d to ata unit %d...\n",src,dst);
+	printf("copying %s unit %d to %s unit %d...\n",src_device_name,src_unit,dst_device_name,dst_unit);
 
 	fs_dirent_copy(srcroot, dstroot,0);
 
@@ -309,13 +309,13 @@ static int kshell_execute(int argc, const char **argv)
 			}
 		}
 	} else if(!strcmp(cmd,"install")) {
-		if(argc==3) {
+		if(argc==5) {
 			int src, dst;
-			str2int(argv[1], &src);
-			str2int(argv[2], &dst);
-			kshell_install(src,dst);
+			str2int(argv[2], &src);
+			str2int(argv[4], &dst);
+			kshell_install(argv[1],src,argv[3],dst);
 		} else {
-			printf("install: expected unit #s for cdrom and disk\n");
+			printf("install: expected src-device-name src-unit dest-device-name dest-unit\n");
 		}
 
 	} else if(!strcmp(cmd, "remove")) {
@@ -349,7 +349,7 @@ static int kshell_execute(int argc, const char **argv)
 	} else if(!strcmp(cmd,"bcache_flush")) {
 		bcache_flush_all();
 	} else if(!strcmp(cmd, "help")) {
-		printf("Kernel Shell Commands:\nrun <path> <args>\nstart <path> <args>\nkill <pid>\nreap <pid>\nwait\nlist\nautomount\nmount <device> <unit> <fstype>\numount\nformat <device> <unit><fstype>\ninstall <srcunit> <dstunit>\nmkdir <path>\nremove <path>time\nbcache_stats\nbcache_flush\nreboot\nhelp\n\n");
+		printf("Kernel Shell Commands:\nrun <path> <args>\nstart <path> <args>\nkill <pid>\nreap <pid>\nwait\nlist\nautomount\nmount <device> <unit> <fstype>\numount\nformat <device> <unit><fstype>\ninstall atapi <srcunit> ata <dstunit>\nmkdir <path>\nremove <path>time\nbcache_stats\nbcache_flush\nreboot\nhelp\n\n");
 	} else {
 		printf("%s: command not found\n", argv[0]);
 	}
@@ -387,8 +387,6 @@ int kshell_launch()
 	const char *argv[100];
 	int argc;
 
-	kshell_automount();
-
 	while(1) {
 		printf("kshell> ");
 		kshell_readline(line, sizeof(line));

kernel/process.c

@@ -309,6 +309,8 @@ void process_preempt()
 
 void process_yield()
 {
+	/* no-op if process module not yet initialized. */
+	if(!current) return;
 	process_switch(PROCESS_STATE_READY);
 }