Re: [PATCH 2/3] virtual block device driver (ramzswap)

[Nitin Gupta]

On 09/24/2009 10:41 AM, KAMEZAWA Hiroyuki wrote:
> On Tue, 22 Sep 2009 10:26:53 +0530
> Nitin Gupta <ngupta@xxxxxxxxxx> wrote:
> 
> <snip>
>> +	if (unlikely(clen > max_zpage_size)) {
>> +		if (rzs->backing_swap) {
>> +			mutex_unlock(&rzs->lock);
>> +			fwd_write_request = 1;
>> +			goto out;
>> +		}
>> +
>> +		clen = PAGE_SIZE;
>> +		page_store = alloc_page(GFP_NOIO | __GFP_HIGHMEM);
> Here, and...
> 
>> +		if (unlikely(!page_store)) {
>> +			mutex_unlock(&rzs->lock);
>> +			pr_info("Error allocating memory for incompressible "
>> +				"page: %u\n", index);
>> +			stat_inc(rzs->stats.failed_writes);
>> +			goto out;
>> +		}
>> +
>> +		offset = 0;
>> +		rzs_set_flag(rzs, index, RZS_UNCOMPRESSED);
>> +		stat_inc(rzs->stats.pages_expand);
>> +		rzs->table[index].page = page_store;
>> +		src = kmap_atomic(page, KM_USER0);
>> +		goto memstore;
>> +	}
>> +
>> +	if (xv_malloc(rzs->mem_pool, clen + sizeof(*zheader),
>> +			&rzs->table[index].page, &offset,
>> +			GFP_NOIO | __GFP_HIGHMEM)) {
> 
> Here.
>     
> Do we need to wait until here for detecting page-allocation-failure ?
> Detecting it here means -EIO for end_swap_bio_write()....unhappy
> ALERT messages etc..
> 
> Can't we add a hook to get_swap_page() for preparing this ("do we have
> enough pool?") and use only GFP_ATOMIC throughout codes ?
> (memory pool for this swap should be big to some extent.)
>

Yes, we do need to wait until this step for detecting alloc failure since
we don't really know when pool grow will (almost) surely wail.
What we can probably do is, hook into OOM notify chain (oom_notify_list)
and whenever we get this callback, we can start sending pages directly
to backing swap and do not even attempt to do any allocation.


 
>>From my user support experience for heavy swap customers,  extra memory allocation for swapping out is just bad...in many cases.
> (*) I know GFP_IO works well to some extent.
> 

We cannot use GFP_IO here as it can cause a deadlock:
ramzswap alloc() --> not enough memory, try to reclaim some --> swap out ...
... some pages to ramzswap --> ramzswap alloc()

Thanks,
Nitin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/