From 24cbd6a333ac253102193b21ff836900d296f8ba Mon Sep 17 00:00:00 2001 From: Douglas Thain Date: Mon, 18 Dec 2023 09:55:40 -0500 Subject: [PATCH] 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 | 2 +- kernel/diskfs.c | 19 ++++++++++++++++--- kernel/diskfs.h | 5 ++++- kernel/kshell.c | 24 +++++++++++------------- kernel/process.c | 2 ++ 5 files changed, 34 insertions(+), 18 deletions(-) diff --git a/kernel/ata.c b/kernel/ata.c index a22e7d9a..8be75f32 100644 --- a/kernel/ata.c +++ b/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(); } } diff --git a/kernel/diskfs.c b/kernel/diskfs.c index 0b25cba0..baf678d1 100644 --- a/kernel/diskfs.c +++ b/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;iitems[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,6 +391,8 @@ 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); @@ -389,7 +400,9 @@ struct fs_dirent * diskfs_dirent_create_file_or_dir( struct fs_dirent *d, const } 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;jitems[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; diff --git a/kernel/diskfs.h b/kernel/diskfs.h index ddf85d37..ee63efb3 100644 --- a/kernel/diskfs.h +++ b/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() diff --git a/kernel/kshell.c b/kernel/kshell.c index 371bd1f9..98f117c3 100644 --- a/kernel/kshell.c +++ b/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 \nstart \nkill \nreap \nwait\nlist\nautomount\nmount \numount\nformat \ninstall \nmkdir \nremove time\nbcache_stats\nbcache_flush\nreboot\nhelp\n\n"); + printf("Kernel Shell Commands:\nrun \nstart \nkill \nreap \nwait\nlist\nautomount\nmount \numount\nformat \ninstall atapi ata \nmkdir \nremove 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)); diff --git a/kernel/process.c b/kernel/process.c index a453fcf3..2d3201f6 100644 --- a/kernel/process.c +++ b/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); }