Re: [patch] take 2 Re: PG_swap_entry bug in recent kernels

Kanoj Sarcar
2000-04-08T00:37:30Z

> >
> > >Also, did you have a good reason to want to make lookup_swap_cache()
> > >invoke find_get_page(), and not find_lock_page()? I coded some of the
> >
> > Using find_lock_page and then unlocking the page is meaningless. If you
> > are going to unconditionally unlock the page then you shouldn't lock it in
> > first place.
>
> I will have to think a little bit about why the code does what it does
> currently. I will let you know ...
>

Okay, I think I found at least one reason why the lockpage was being done
in lookup_swap_cache(). It was effectively to check the PageSwapCache bit,
since shrink_mmap:__delete_from_swap_cache could race with a
lookup_swap_cache.

Yes, I did notice the recent shrink_mmap SMP race fixes that you posted,
now it _*might*_ be unneccesary to do a find_lock_page() in
lookup_swap_cache() (just for this race). I will have to look at the
latest prepatch to confirm that. In any case, there are still races with
swapspace deletion and swapcache lookup, so unless you had a good reason
to want to replace the find_lock_page with lookup_swap_cache, I would much
rather see it the way it is currently ...

Kanoj