On 04/06/2018 07:28 PM, Matthew Wilcox wrote:
> I'm happy to help clean this up in advance of the XArray code going in ...
>
> This loop is actually buggy in two or three different ways. Here's how
> it should have looked:
>
> @@ -1098,13 +1098,18 @@ static void shmem_evict_inode(struct inode *inode)
> static unsigned long find_swap_entry(struct radix_tree_root *root, void *item)
> {
> struct radix_tree_iter iter;
> - void **slot;
> + void __rcu **slot;
> unsigned long found = -1;
> unsigned int checked = 0;
>
> rcu_read_lock();
> radix_tree_for_each_slot(slot, root, &iter, 0) {
> - if (*slot == item) {
> + void *entry = radix_tree_deref_slot(slot);
> + if (radix_tree_deref_retry(entry)) {
> + slot = radix_tree_iter_retry(&iter);
> + continue;
> + }
> + if (entry == item) {
> found = iter.index;
> break;
> }
>
Thank you! I was worried about searching for swap entries that would
be marked RADIX_TREE_EXCEPTIONAL_ENTRY. Your changes above make perfect
sense. Do you mind if I roll them into a patch that adds all the other
missing __rcu annotations in the file, and add your Signed-off-by?
--
Mike Kravetz
