staging: exfat: Update the TODO file
Updating with the current laundry list of things that need attention. Signed-off-by: Valdis Kletnieks <Valdis.Kletnieks@vt.edu> Link: https://lore.kernel.org/r/20191112223609.163501-1-Valdis.Kletnieks@vt.edu Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
65b88d81f5
commit
6366e523eb
@ -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 <gregkh@linuxfoundation.org>
|
||||
Valdis Kletnieks <valdis.kletnieks@vt.edu>
|
||||
|
Loading…
Reference in New Issue
Block a user