Skip to content

Commit 307af6c

Browse files
jankaratytso
jankara
authored and
tytso
committed
mbcache: automatically delete entries from cache on freeing
Use the fact that entries with elevated refcount are not removed from the hash and just move removal of the entry from the hash to the entry freeing time. When doing this we also change the generic code to hold one reference to the cache entry, not two of them, which makes code somewhat more obvious. Signed-off-by: Jan Kara <jack@suse.cz> Link: https://lore.kernel.org/r/20220712105436.32204-10-jack@suse.cz Signed-off-by: Theodore Ts'o <tytso@mit.edu>
1 parent 7589633 commit 307af6c

2 files changed

Lines changed: 55 additions & 77 deletions

File tree

‎fs/mbcache.c‎

Lines changed: 39 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ int mb_cache_entry_create(struct mb_cache *cache, gfp_t mask, u32 key,
9090
return -ENOMEM;
9191

9292
INIT_LIST_HEAD(&entry->e_list);
93-
/* One ref for hash, one ref returned */
93+
/* Initial hash reference */
9494
atomic_set(&entry->e_refcnt, 1);
9595
entry->e_key = key;
9696
entry->e_value = value;
@@ -106,21 +106,28 @@ int mb_cache_entry_create(struct mb_cache *cache, gfp_t mask, u32 key,
106106
}
107107
}
108108
hlist_bl_add_head(&entry->e_hash_list, head);
109-
hlist_bl_unlock(head);
110-
109+
/*
110+
* Add entry to LRU list before it can be found by
111+
* mb_cache_entry_delete() to avoid races
112+
*/
111113
spin_lock(&cache->c_list_lock);
112114
list_add_tail(&entry->e_list, &cache->c_list);
113-
/* Grab ref for LRU list */
114-
atomic_inc(&entry->e_refcnt);
115115
cache->c_entry_count++;
116116
spin_unlock(&cache->c_list_lock);
117+
hlist_bl_unlock(head);
117118

118119
return 0;
119120
}
120121
EXPORT_SYMBOL(mb_cache_entry_create);
121122

122-
void __mb_cache_entry_free(struct mb_cache_entry *entry)
123+
void __mb_cache_entry_free(struct mb_cache *cache, struct mb_cache_entry *entry)
123124
{
125+
struct hlist_bl_head *head;
126+
127+
head = mb_cache_entry_head(cache, entry->e_key);
128+
hlist_bl_lock(head);
129+
hlist_bl_del(&entry->e_hash_list);
130+
hlist_bl_unlock(head);
124131
kmem_cache_free(mb_entry_cache, entry);
125132
}
126133
EXPORT_SYMBOL(__mb_cache_entry_free);
@@ -134,7 +141,7 @@ EXPORT_SYMBOL(__mb_cache_entry_free);
134141
*/
135142
void mb_cache_entry_wait_unused(struct mb_cache_entry *entry)
136143
{
137-
wait_var_event(&entry->e_refcnt, atomic_read(&entry->e_refcnt) <= 3);
144+
wait_var_event(&entry->e_refcnt, atomic_read(&entry->e_refcnt) <= 2);
138145
}
139146
EXPORT_SYMBOL(mb_cache_entry_wait_unused);
140147

@@ -155,10 +162,9 @@ static struct mb_cache_entry *__entry_find(struct mb_cache *cache,
155162
while (node) {
156163
entry = hlist_bl_entry(node, struct mb_cache_entry,
157164
e_hash_list);
158-
if (entry->e_key == key && entry->e_reusable) {
159-
atomic_inc(&entry->e_refcnt);
165+
if (entry->e_key == key && entry->e_reusable &&
166+
atomic_inc_not_zero(&entry->e_refcnt))
160167
goto out;
161-
}
162168
node = node->next;
163169
}
164170
entry = NULL;
@@ -218,10 +224,9 @@ struct mb_cache_entry *mb_cache_entry_get(struct mb_cache *cache, u32 key,
218224
head = mb_cache_entry_head(cache, key);
219225
hlist_bl_lock(head);
220226
hlist_bl_for_each_entry(entry, node, head, e_hash_list) {
221-
if (entry->e_key == key && entry->e_value == value) {
222-
atomic_inc(&entry->e_refcnt);
227+
if (entry->e_key == key && entry->e_value == value &&
228+
atomic_inc_not_zero(&entry->e_refcnt))
223229
goto out;
224-
}
225230
}
226231
entry = NULL;
227232
out:
@@ -244,37 +249,25 @@ EXPORT_SYMBOL(mb_cache_entry_get);
244249
struct mb_cache_entry *mb_cache_entry_delete_or_get(struct mb_cache *cache,
245250
u32 key, u64 value)
246251
{
247-
struct hlist_bl_node *node;
248-
struct hlist_bl_head *head;
249252
struct mb_cache_entry *entry;
250253

251-
head = mb_cache_entry_head(cache, key);
252-
hlist_bl_lock(head);
253-
hlist_bl_for_each_entry(entry, node, head, e_hash_list) {
254-
if (entry->e_key == key && entry->e_value == value) {
255-
if (atomic_read(&entry->e_refcnt) > 2) {
256-
atomic_inc(&entry->e_refcnt);
257-
hlist_bl_unlock(head);
258-
return entry;
259-
}
260-
/* We keep hash list reference to keep entry alive */
261-
hlist_bl_del_init(&entry->e_hash_list);
262-
hlist_bl_unlock(head);
263-
spin_lock(&cache->c_list_lock);
264-
if (!list_empty(&entry->e_list)) {
265-
list_del_init(&entry->e_list);
266-
if (!WARN_ONCE(cache->c_entry_count == 0,
267-
"mbcache: attempt to decrement c_entry_count past zero"))
268-
cache->c_entry_count--;
269-
atomic_dec(&entry->e_refcnt);
270-
}
271-
spin_unlock(&cache->c_list_lock);
272-
mb_cache_entry_put(cache, entry);
273-
return NULL;
274-
}
275-
}
276-
hlist_bl_unlock(head);
254+
entry = mb_cache_entry_get(cache, key, value);
255+
if (!entry)
256+
return NULL;
257+
258+
/*
259+
* Drop the ref we got from mb_cache_entry_get() and the initial hash
260+
* ref if we are the last user
261+
*/
262+
if (atomic_cmpxchg(&entry->e_refcnt, 2, 0) != 2)
263+
return entry;
277264

265+
spin_lock(&cache->c_list_lock);
266+
if (!list_empty(&entry->e_list))
267+
list_del_init(&entry->e_list);
268+
cache->c_entry_count--;
269+
spin_unlock(&cache->c_list_lock);
270+
__mb_cache_entry_free(cache, entry);
278271
return NULL;
279272
}
280273
EXPORT_SYMBOL(mb_cache_entry_delete_or_get);
@@ -306,42 +299,24 @@ static unsigned long mb_cache_shrink(struct mb_cache *cache,
306299
unsigned long nr_to_scan)
307300
{
308301
struct mb_cache_entry *entry;
309-
struct hlist_bl_head *head;
310302
unsigned long shrunk = 0;
311303

312304
spin_lock(&cache->c_list_lock);
313305
while (nr_to_scan-- && !list_empty(&cache->c_list)) {
314306
entry = list_first_entry(&cache->c_list,
315307
struct mb_cache_entry, e_list);
316-
if (entry->e_referenced || atomic_read(&entry->e_refcnt) > 2) {
308+
/* Drop initial hash reference if there is no user */
309+
if (entry->e_referenced ||
310+
atomic_cmpxchg(&entry->e_refcnt, 1, 0) != 1) {
317311
entry->e_referenced = 0;
318312
list_move_tail(&entry->e_list, &cache->c_list);
319313
continue;
320314
}
321315
list_del_init(&entry->e_list);
322316
cache->c_entry_count--;
323-
/*
324-
* We keep LRU list reference so that entry doesn't go away
325-
* from under us.
326-
*/
327317
spin_unlock(&cache->c_list_lock);
328-
head = mb_cache_entry_head(cache, entry->e_key);
329-
hlist_bl_lock(head);
330-
/* Now a reliable check if the entry didn't get used... */
331-
if (atomic_read(&entry->e_refcnt) > 2) {
332-
hlist_bl_unlock(head);
333-
spin_lock(&cache->c_list_lock);
334-
list_add_tail(&entry->e_list, &cache->c_list);
335-
cache->c_entry_count++;
336-
continue;
337-
}
338-
if (!hlist_bl_unhashed(&entry->e_hash_list)) {
339-
hlist_bl_del_init(&entry->e_hash_list);
340-
atomic_dec(&entry->e_refcnt);
341-
}
342-
hlist_bl_unlock(head);
343-
if (mb_cache_entry_put(cache, entry))
344-
shrunk++;
318+
__mb_cache_entry_free(cache, entry);
319+
shrunk++;
345320
cond_resched();
346321
spin_lock(&cache->c_list_lock);
347322
}
@@ -433,11 +408,6 @@ void mb_cache_destroy(struct mb_cache *cache)
433408
* point.
434409
*/
435410
list_for_each_entry_safe(entry, next, &cache->c_list, e_list) {
436-
if (!hlist_bl_unhashed(&entry->e_hash_list)) {
437-
hlist_bl_del_init(&entry->e_hash_list);
438-
atomic_dec(&entry->e_refcnt);
439-
} else
440-
WARN_ON(1);
441411
list_del(&entry->e_list);
442412
WARN_ON(atomic_read(&entry->e_refcnt) != 1);
443413
mb_cache_entry_put(cache, entry);

‎include/linux/mbcache.h‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,16 @@ struct mb_cache;
1313
struct mb_cache_entry {
1414
/* List of entries in cache - protected by cache->c_list_lock */
1515
struct list_head e_list;
16-
/* Hash table list - protected by hash chain bitlock */
16+
/*
17+
* Hash table list - protected by hash chain bitlock. The entry is
18+
* guaranteed to be hashed while e_refcnt > 0.
19+
*/
1720
struct hlist_bl_node e_hash_list;
21+
/*
22+
* Entry refcount. Once it reaches zero, entry is unhashed and freed.
23+
* While refcount > 0, the entry is guaranteed to stay in the hash and
24+
* e.g. mb_cache_entry_try_delete() will fail.
25+
*/
1826
atomic_t e_refcnt;
1927
/* Key in hash - stable during lifetime of the entry */
2028
u32 e_key;
@@ -29,20 +37,20 @@ void mb_cache_destroy(struct mb_cache *cache);
2937

3038
int mb_cache_entry_create(struct mb_cache *cache, gfp_t mask, u32 key,
3139
u64 value, bool reusable);
32-
void __mb_cache_entry_free(struct mb_cache_entry *entry);
40+
void __mb_cache_entry_free(struct mb_cache *cache,
41+
struct mb_cache_entry *entry);
3342
void mb_cache_entry_wait_unused(struct mb_cache_entry *entry);
34-
static inline int mb_cache_entry_put(struct mb_cache *cache,
35-
struct mb_cache_entry *entry)
43+
static inline void mb_cache_entry_put(struct mb_cache *cache,
44+
struct mb_cache_entry *entry)
3645
{
3746
unsigned int cnt = atomic_dec_return(&entry->e_refcnt);
3847

3948
if (cnt > 0) {
40-
if (cnt <= 3)
49+
if (cnt <= 2)
4150
wake_up_var(&entry->e_refcnt);
42-
return 0;
51+
return;
4352
}
44-
__mb_cache_entry_free(entry);
45-
return 1;
53+
__mb_cache_entry_free(cache, entry);
4654
}
4755

4856
struct mb_cache_entry *mb_cache_entry_delete_or_get(struct mb_cache *cache,

0 commit comments

Comments
 (0)