On 09/04/2017 02:52 PM, Kirill Tkhai wrote:
> On 31.08.2017 16:45, Andrey Ryabinin wrote:
>> On 08/03/2017 12:54 PM, Kirill Tkhai wrote:
>>> static int tswap_frontswap_store(unsigned type, pgoff_t offset,
>>> struct page *page)
>>> {
>>> swp_entry_t entry = swp_entry(type, offset);
>>> + int zero_filled = -1, err = 0;
>>> struct page *cache_page;
>>> - int err = 0;
>>>
>>> if (!tswap_active)
>>> return -1;
>>>
>>> cache_page = tswap_lookup_page(entry);
>>> - if (cache_page)
>>> - goto copy;
>>> + if (cache_page) {
>>> + zero_filled = is_zero_filled_page(page);
>>> + /* If type of page has not changed, just reuse it */
>>> + if (zero_filled == (cache_page == ZERO_PAGE(0)))
>>> + goto copy;
>>> + tswap_delete_page(entry, NULL);
>>> + put_page(cache_page);
>>
>> I think if we race with tswap_frontswap_load() this will lead to double
>> put_page().
>
> Hm, then if load and store may be executed in parallel, doesn't the current
> code have the following problem?
>
I don't think so.
> tswap_frontswap_store() tswap_frontswap_load()
> cache_page = tswap_lookup_page(entry); ...
> goto copy; ...
> ... cache_page =
> tswap_delete_page()
cache_page will be NULL here
and tswap_frontswap_load() will bail out without putting page.
> ... put_page(cache_page);
> ... ...
> copy_highpage(cache_page, page); ...
>
> ^^^ copy to freed page?
>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
