Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LOCKBIT mechanism for the buffer cache doesn't feel safe #128

Open
PypeBros opened this issue Feb 28, 2018 · 1 comment
Open

LOCKBIT mechanism for the buffer cache doesn't feel safe #128

PypeBros opened this issue Feb 28, 2018 · 1 comment

Comments

@PypeBros
Copy link

Hi. I'm currently having a review of your exFAT driver.

The LOCKBIT mechanism in exfat_cache seems either incomplete or un-needed. There are many places where a buffer is first located with buf_getblk(), and later locked with buf_lock(). Nothing really guarantees (from an exfat_cache perspective) that the sector used by that cache entry hasn't changed between the two operations, although I expect them to be unlikely given that the buffer returned by bug_getblk() is pushed at the head of the recently-used list while allocating happens near the tail.

Similarly, I see many functions doing get_entry_in_dir(); /* modify entry */ ; buf_modify(); activity, with no strong guarantee that the contents of the cached entry remains the same between read and modify. Some of these functions will obviously be mutually exclusive due to the v_sem (volume-level semaphore ?). Exclusive access is less clear for others (e.g. exfat_init_dir_entry).

Finally, I note that exfat_cache doesn't prevent alone that all entries in the LRU list get their LOCKBIT set, in which case buf_cache_get() would happily return the LRU list header, which __buf_getblk() doesn't seem to check. Next call to buf_cache_get() could then loop forever.

If some higher-level exclusive access makes all the above non-issues, then what is the value added by LOCKBIT ?

Best regards,
/PypeBros.

@v-fox
Copy link

v-fox commented Apr 6, 2018

Well, as far as I know, it's not his exFAT driver, it's a fork of abandoned Samsung's implementation. So if you know better how it should be designed, please, for the benefit of whole community, push some patches here. SD-cards are only going to get bigger and most Android builds refuse to work with anything but the FAT crap which is the weakest part of Linux kernel now (in-kernel NTFS implementation is long obsolete and exFAT just doesn't exist).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants