[Devel] [PATCH rh7] tswap: Add support for zero-filled pages

[Andrey Ryabinin]

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?
>