Re: [PATCH 05/10] mm, page_alloc: Distinguish between being unable to sleep, unwilling to sleep and avoiding waking kswapd

["Drokin, Oleg" <oleg.drokin@intel.com>]

Hello!
On Sep 30, 2015, at 8:26 AM, Vlastimil Babka wrote:

> [diff --git a/drivers/staging/lustre/lnet/lnet/router.c b/drivers/staging/lustre/lnet/lnet/router.c
>> 
>> index 4fbae5ef44a9..dad9816dfee7 100644
>> --- a/drivers/staging/lustre/lnet/lnet/router.c
>> +++ b/drivers/staging/lustre/lnet/lnet/router.c
>> @@ -1246,7 +1246,7 @@ lnet_new_rtrbuf(lnet_rtrbufpool_t *rbp, int cpt)
>>  	for (i = 0; i < npages; i++) {
>>  		page = alloc_pages_node(
>>  				cfs_cpt_spread_node(lnet_cpt_table(), cpt),
>> -				__GFP_ZERO | GFP_IOFS, 0);
>> +				GFP_KERNEL | __GFP_ZERO, 0);
>>  		if (page == NULL) {
>>  			while (--i >= 0)
>>  				__free_page(rb->rb_kiov[i].kiov_page);

This one is ok, it's in the non-fs part, so cannot enter via an fs operation.

>> diff --git a/drivers/staging/lustre/lnet/selftest/conrpc.c b/drivers/staging/lustre/lnet/selftest/conrpc.c
>> index a1a4e08f7391..3fc37de8d304 100644
>> --- a/drivers/staging/lustre/lnet/selftest/conrpc.c
>> +++ b/drivers/staging/lustre/lnet/selftest/conrpc.c
>> @@ -861,7 +861,7 @@ lstcon_testrpc_prep(lstcon_node_t *nd, int transop, unsigned feats,
>>  			bulk->bk_iovs[i].kiov_offset = 0;
>>  			bulk->bk_iovs[i].kiov_len    = len;
>>  			bulk->bk_iovs[i].kiov_page   =
>> -				alloc_page(GFP_IOFS);
>> +				alloc_page(GFP_KERNEL);
>> 
>>  			if (bulk->bk_iovs[i].kiov_page == NULL) {
>>  				lstcon_rpc_put(*crpc);
>> diff --git a/drivers/staging/lustre/lnet/selftest/rpc.c b/drivers/staging/lustre/lnet/selftest/rpc.c
>> index 6ae133138b17..aa0f88fbb221 100644
>> --- a/drivers/staging/lustre/lnet/selftest/rpc.c
>> +++ b/drivers/staging/lustre/lnet/selftest/rpc.c
>> @@ -146,7 +146,7 @@ srpc_alloc_bulk(int cpt, unsigned bulk_npg, unsigned bulk_len, int sink)
>>  		int nob;
>> 
>>  		pg = alloc_pages_node(cfs_cpt_spread_node(lnet_cpt_table(), cpt),
>> -				      GFP_IOFS, 0);
>> +				      GFP_KERNEL, 0);
>>  		if (pg == NULL) {
>>  			CERROR("Can't allocate page %d of %d\n", i, bulk_npg);
>>  			srpc_free_bulk(bk);

These two are in "lnet-selftest" that is self-hosted. so also ok.

>> diff --git a/drivers/staging/lustre/lustre/libcfs/module.c b/drivers/staging/lustre/lustre/libcfs/module.c
>> index 806f9747a3a2..303143f28c06 100644
>> --- a/drivers/staging/lustre/lustre/libcfs/module.c
>> +++ b/drivers/staging/lustre/lustre/libcfs/module.c
>> @@ -321,7 +321,7 @@ static int libcfs_ioctl(struct cfs_psdev_file *pfile, unsigned long cmd, void *a
>>  	struct libcfs_ioctl_data *data;
>>  	int err = 0;
>> 
>> -	LIBCFS_ALLOC_GFP(buf, 1024, GFP_IOFS);
>> +	LIBCFS_ALLOC_GFP(buf, 1024, GFP_KERNEL);
>>  	if (buf == NULL)
>>  		return -ENOMEM;
>> 
>> diff --git a/drivers/staging/lustre/lustre/libcfs/tracefile.c b/drivers/staging/lustre/lustre/libcfs/tracefile.c
>> index effa2af58c13..a7d72f69c4eb 100644
>> --- a/drivers/staging/lustre/lustre/libcfs/tracefile.c
>> +++ b/drivers/staging/lustre/lustre/libcfs/tracefile.c
>> @@ -810,7 +810,7 @@ int cfs_trace_allocate_string_buffer(char **str, int nob)
>>  	if (nob > 2 * PAGE_CACHE_SIZE)	    /* string must be "sensible" */
>>  		return -EINVAL;
>> 
>> -	*str = kmalloc(nob, GFP_IOFS | __GFP_ZERO);
>> +	*str = kmalloc(nob, GFP_KERNEL | __GFP_ZERO);
> 
> This could use kzalloc.
> 
>>  	if (*str == NULL)
>>  		return -ENOMEM;
>> 
>> diff --git a/drivers/staging/lustre/lustre/llite/remote_perm.c b/drivers/staging/lustre/lustre/llite/remote_perm.c
>> index 39022ea88b5f..b27f016c3dd4 100644
>> --- a/drivers/staging/lustre/lustre/llite/remote_perm.c
>> +++ b/drivers/staging/lustre/lustre/llite/remote_perm.c
>> @@ -84,7 +84,7 @@ static struct hlist_head *alloc_rmtperm_hash(void)
>> 
>>  	OBD_SLAB_ALLOC_GFP(hash, ll_rmtperm_hash_cachep,
>>  			   REMOTE_PERM_HASHSIZE * sizeof(*hash),
>> -			   GFP_IOFS);
>> +			   GFP_KERNEL);
>>  	if (!hash)
>>  		return NULL;
>> 

This is called from ll_inode_permission (the inode ops->permission method), so I imagine this must be GFP_NOFS.

>> diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
>> index 019ee2f256aa..79551319d754 100644
>> --- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
>> +++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
>> @@ -198,7 +198,7 @@ struct config_llog_data *do_config_log_add(struct obd_device *obd,
>>  	CDEBUG(D_MGC, "do adding config log %s:%p\n", logname,
>>  	       cfg ? cfg->cfg_instance : NULL);
>> 
>> -	cld = kzalloc(sizeof(*cld) + strlen(logname) + 1, GFP_NOFS);
>> +	cld = kzalloc(sizeof(*cld) + strlen(logname) + 1, GFP_KERNEL);
>>  	if (!cld)
>>  		return ERR_PTR(-ENOMEM);
>> 
>> @@ -1127,7 +1127,7 @@ static int mgc_apply_recover_logs(struct obd_device *mgc,
>>  	LASSERT(cfg->cfg_instance != NULL);
>>  	LASSERT(cfg->cfg_sb == cfg->cfg_instance);
>> 
>> -	inst = kzalloc(PAGE_CACHE_SIZE, GFP_NOFS);
>> +	inst = kzalloc(PAGE_CACHE_SIZE, GFP_KERNEL);
>>  	if (!inst)
>>  		return -ENOMEM;
>> 
>> @@ -1334,14 +1334,14 @@ static int mgc_process_recover_log(struct obd_device *obd,
>>  	if (cfg->cfg_last_idx == 0) /* the first time */
>>  		nrpages = CONFIG_READ_NRPAGES_INIT;
>> 
>> -	pages = kcalloc(nrpages, sizeof(*pages), GFP_NOFS);
>> +	pages = kcalloc(nrpages, sizeof(*pages), GFP_KERNEL);
>>  	if (pages == NULL) {
>>  		rc = -ENOMEM;
>>  		goto out;
>>  	}
>> 
>>  	for (i = 0; i < nrpages; i++) {
>> -		pages[i] = alloc_page(GFP_IOFS);
>> +		pages[i] = alloc_page(GFP_KERNEL);
>>  		if (pages[i] == NULL) {
>>  			rc = -ENOMEM;
>>  			goto out;
>> @@ -1492,7 +1492,7 @@ static int mgc_process_cfg_log(struct obd_device *mgc,
>>  	if (cld->cld_cfg.cfg_sb)
>>  		lsi = s2lsi(cld->cld_cfg.cfg_sb);
>> 
>> -	env = kzalloc(sizeof(*env), GFP_NOFS);
>> +	env = kzalloc(sizeof(*env), GFP_KERNEL);
>>  	if (!env)
>>  		return -ENOMEM;

These should live in it's own separate thread so I imagine should be fine.

>> diff --git a/drivers/staging/lustre/lustre/obdecho/echo_client.c b/drivers/staging/lustre/lustre/obdecho/echo_client.c
>> index 27bd170c3a28..7c8443644300 100644
>> --- a/drivers/staging/lustre/lustre/obdecho/echo_client.c
>> +++ b/drivers/staging/lustre/lustre/obdecho/echo_client.c
>> @@ -1561,7 +1561,7 @@ static int echo_client_kbrw(struct echo_device *ed, int rw, struct obdo *oa,
>>  		  (oa->o_valid & OBD_MD_FLFLAGS) != 0 &&
>>  		  (oa->o_flags & OBD_FL_DEBUG_CHECK) != 0);
>> 
>> -	gfp_mask = ((ostid_id(&oa->o_oi) & 2) == 0) ? GFP_IOFS : GFP_HIGHUSER;
>> +	gfp_mask = ((ostid_id(&oa->o_oi) & 2) == 0) ? GFP_KERNEL : GFP_HIGHUSER;
>> 
>>  	LASSERT(rw == OBD_BRW_WRITE || rw == OBD_BRW_READ);
>>  	LASSERT(lsm != NULL);

This is it's own thing, so ok

>> diff --git a/drivers/staging/lustre/lustre/osc/osc_cache.c b/drivers/staging/lustre/lustre/osc/osc_cache.c
>> index c72035e048aa..6fa6bc6874ab 100644
>> --- a/drivers/staging/lustre/lustre/osc/osc_cache.c
>> +++ b/drivers/staging/lustre/lustre/osc/osc_cache.c
>> @@ -346,7 +346,7 @@ static struct osc_extent *osc_extent_alloc(struct osc_object *obj)
>>  {
>>  	struct osc_extent *ext;
>> 
>> -	OBD_SLAB_ALLOC_PTR_GFP(ext, osc_extent_kmem, GFP_IOFS);
>> +	OBD_SLAB_ALLOC_PTR_GFP(ext, osc_extent_kmem, GFP_KERNEL);
>>  	if (ext == NULL)
>>  		return NULL;
>> 

These are called in IO path, so should be GFP_NOFS, really.

Thanks!