On Sat, Jun 17, 2023 at 9:39 PM Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx> wrote: > > On (23/06/12 11:38), Domenico Cerasuolo wrote: > > +static int zswap_reclaim_entry(struct zswap_pool *pool) > > +{ > > + struct zswap_header *zhdr; > > + struct zswap_entry *entry; > > + struct zswap_tree *tree; > > + pgoff_t swpoffset; > > + int ret; > > + > > + /* Get an entry off the LRU */ > > + spin_lock(&pool->lru_lock); > > + if (list_empty(&pool->lru)) { > > + spin_unlock(&pool->lru_lock); > > + return -EINVAL; > > + } > > + entry = list_last_entry(&pool->lru, struct zswap_entry, lru); > > + list_del_init(&entry->lru); > > A quick question: should we zswap_entry_get() here? We need to hold the tree lock for that, and the lock ordering is tree lock -> lru lock. If we try to grab the tree lock here we may deadlock. In the next code block we release the lru_lock, hold the tree lock, make sure the entry is still valid in the tree, then do zswap_entry_get().