[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: CVS commit: src/sys
> Module Name: src
> Committed By: reinoud
> Date: Sat Sep 27 13:01:07 UTC 2008
> Modified Files:
> src/sys/conf: files
> src/sys/kern: vfs_init.c
> Added Files:
> src/sys/kern: vfs_dirhash.c
> src/sys/sys: dirhash.h
> Log Message:
> Add generic FS agnostic directory hashing support. Currently only in use by
> UDF. Future users could be msdosfs, ufs, nilfs2 (when ready), cd9660 etc.
It would be nice to see UFS using this in future! I have few comments:
- Locking in vfs_dirhash.c is not clear, it does not describe which and who
protects the structures, nor how synchronisation is done. It looks like some
parts are protected by caller, some by dirhashmutex. KASSERTs do not explain
- In dirhash_purge_entries() memory is freed (pool_put() called) while lock
is held (dirhashmutex) - that reduces the concurrency. Also, if caller
holds some lock, API might need to be adjusted to free the structures later.
- It looks like hash(9) and hashinit(9) interfaces could be used for hashing
part, instead of re-implementation of them.
- These could be static, thought: dirhashmutex, maxdirhashsize, dirhashsize,
- It would be good to add prefixes for the members of dirhash_entry and
dirhash structures, like dh_* and dhe_*. No need to name arguments in
function prototypes. Also, header needs #ifndef _SYS_DIRHASH_H_ macros
to avoid double inclusion.
- Please create sysctl()s dynamically, that is, using CTL_CREATE, instead of
CURDIRHASHSIZE_SYSCTLOPT and MAXDIRHASHSIZE_SYSCTLOPT.
- Having minimal comments about functions or dirhash(9) man page would help
to understand the interface better too. :)
Main Index |
Thread Index |