From 6366e523eb56b4b7191fd4261941a2762b6957bd Mon Sep 17 00:00:00 2001 From: Valdis Kletnieks Date: Tue, 12 Nov 2019 17:36:08 -0500 Subject: staging: exfat: Update the TODO file Updating with the current laundry list of things that need attention. Signed-off-by: Valdis Kletnieks Link: https://lore.kernel.org/r/20191112223609.163501-1-Valdis.Kletnieks@vt.edu Signed-off-by: Greg Kroah-Hartman --- drivers/staging/exfat/TODO | 70 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 59 insertions(+), 11 deletions(-) (limited to 'drivers/staging') diff --git a/drivers/staging/exfat/TODO b/drivers/staging/exfat/TODO index b60e50b9cf4e..a283ce534cf4 100644 --- a/drivers/staging/exfat/TODO +++ b/drivers/staging/exfat/TODO @@ -1,17 +1,22 @@ +A laundry list of things that need looking at, most of which will +require more work than the average checkpatch cleanup... + +Note that some of these entries may not be bugs - they're things +that need to be looked at, and *possibly* fixed. + +Clean up the ffsCamelCase function names. + +Fix (thing)->flags to not use magic numbers - multiple offenders + +Sort out all the s32/u32/u8 nonsense - most of these should be plain int. + exfat_core.c - ffsReadFile - the goto err_out seem to leak a brelse(). same for ffsWriteFile. -exfat_core.c - fs_sync(sb,0) all over the place looks fishy as hell. -There's only one place that calls it with a non-zero argument. -Randomly removing fs_sync() calls is *not* the right answer, especially -if the removal then leaves a call to fs_set_vol_flags(VOL_CLEAN), as that -says the file system is clean and synced when we *know* it isn't. -The proper fix here is to go through and actually analyze how DELAYED_SYNC -should work, and any time we're setting VOL_CLEAN, ensure the file system -has in fact been synced to disk. In other words, changing the 'false' to -'true' is probably more correct. Also, it's likely that the one current -place where it actually does an bdev_sync isn't sufficient in the DELAYED_SYNC -case. +All the calls to fs_sync() need to be looked at, particularly in the +context of EXFAT_DELAYED_SYNC. Currently, if that's defined, we only +flush to disk when sync() gets called. We should be doing at least +metadata flushes at appropriate times. ffsTruncateFile - if (old_size <= new_size) { That doesn't look right. How did it ever work? Are they relying on lazy @@ -19,3 +24,46 @@ block allocation when actual writes happen? If nothing else, it never does the 'fid->size = new_size' and do the inode update.... ffsSetAttr() is just dangling in the breeze, not wired up at all... + +Convert global mutexes to a per-superblock mutex. + +Right now, we load exactly one UTF-8 table. Check to see +if that plays nice with different codepage and iocharset values +for simultanous mounts of different devices + +exfat_rmdir() checks for -EBUSY but ffsRemoveDir() doesn't return it. +In fact, there's a complete lack of -EBUSY testing anywhere. + +There's probably a few missing checks for -EEXIST + +check return codes of sync_dirty_buffer() + +Why is remove_file doing a num_entries++?? + +Double check a lot of can't-happen parameter checks (for null pointers for +things that have only one call site and can't pass a null, etc). + +All the DEBUG stuff can probably be tossed, including the ioctl(). Either +that, or convert to a proper fault-injection system. + +exfat_remount does exactly one thing. Fix to actually deal with remount +options, particularly handling R/O correctly. For that matter, allow +R/O mounts in the first place. + +Figure out why the VFAT code used multi_sector_(read|write) but the +exfat code doesn't use it. The difference matters on SSDs with wear leveling. + +exfat_fat_sync(), exfat_buf_sync(), and sync_alloc_bitmap() +aren't called anyplace.... + +Create helper function for exfat_set_entry_time() and exfat_set_entry_type() +because it's sort of ugly to be calling the same functionn directly and +other code calling through the fs_func struc ponters... + +clean up the remaining vol_type checks, which are of two types: +some are ?: operators with magic numbers, and the rest are places +where we're doing stuff with '.' and '..'. + +Patches to: + Greg Kroah-Hartman + Valdis Kletnieks -- cgit v1.2.3-59-g8ed1b