Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1232432 > unrolled thread

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

Started byJohannes Weiner <hannes@cmpxchg.org>
First post2015-09-24 23:00 +0200
Last post2015-10-02 14:40 +0200
Articles 8 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 05/10] mm, page_alloc: Distinguish between being unable  to sleep, unwilling to sleep and avoiding waking kswapd Johannes Weiner <hannes@cmpxchg.org> - 2015-09-24 23:00 +0200
    Re: [PATCH 05/10] mm, page_alloc: Distinguish between being unable  to sleep, unwilling to sleep and avoiding waking kswapd Mel Gorman <mgorman@techsingularity.net> - 2015-09-25 15:00 +0200
      Re: [PATCH 05/10] mm, page_alloc: Distinguish between being unable  to sleep, unwilling to sleep and avoiding waking kswapd Johannes Weiner <hannes@cmpxchg.org> - 2015-09-25 21:10 +0200
        Re: [PATCH 05/10] mm, page_alloc: Distinguish between being unable  to sleep, unwilling to sleep and avoiding waking kswapd Mel Gorman <mgorman@techsingularity.net> - 2015-09-29 15:40 +0200
          Re: [PATCH 05/10] mm, page_alloc: Distinguish between being unable to  sleep, unwilling to sleep and avoiding waking kswapd Vlastimil Babka <vbabka@suse.cz> - 2015-09-30 14:30 +0200
            Re: [PATCH 05/10] mm, page_alloc: Distinguish between being unable  to sleep, unwilling to sleep and avoiding waking kswapd Mel Gorman <mgorman@techsingularity.net> - 2015-09-30 15:20 +0200
            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> - 2015-10-01 05:10 +0200
              Re: [PATCH 05/10] mm, page_alloc: Distinguish between being unable  to sleep, unwilling to sleep and avoiding waking kswapd Mel Gorman <mgorman@techsingularity.net> - 2015-10-02 14:40 +0200

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

FromJohannes Weiner <hannes@cmpxchg.org>
Date2015-09-24 23:00 +0200
SubjectRe: [PATCH 05/10] mm, page_alloc: Distinguish between being unable to sleep, unwilling to sleep and avoiding waking kswapd
Message-ID<qclMu-5Lb-19@gated-at.bofh.it>
On Mon, Sep 21, 2015 at 11:52:37AM +0100, Mel Gorman wrote:
> @@ -119,10 +134,10 @@ struct vm_area_struct;
>  #define GFP_USER	(__GFP_WAIT | __GFP_IO | __GFP_FS | __GFP_HARDWALL)
>  #define GFP_HIGHUSER	(GFP_USER | __GFP_HIGHMEM)
>  #define GFP_HIGHUSER_MOVABLE	(GFP_HIGHUSER | __GFP_MOVABLE)
> -#define GFP_IOFS	(__GFP_IO | __GFP_FS)
> -#define GFP_TRANSHUGE	(GFP_HIGHUSER_MOVABLE | __GFP_COMP | \
> -			 __GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_NOWARN | \
> -			 __GFP_NO_KSWAPD)
> +#define GFP_IOFS	(__GFP_IO | __GFP_FS | __GFP_KSWAPD_RECLAIM)

These are some really odd semantics to be given a name like that.

GFP_IOFS was introduced as a short-hand for testing/setting/clearing
these two bits at the same time, not to be used for allocations. In
fact, the only user for allocations is lustre, and it's not at all
obious why those sites shouldn't include __GFP_WAIT as well.

Removing this definition altogether would probably be best.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1232775

FromMel Gorman <mgorman@techsingularity.net>
Date2015-09-25 15:00 +0200
Message-ID<qcALv-1Ox-1@gated-at.bofh.it>
In reply to#1232432
On Thu, Sep 24, 2015 at 04:55:09PM -0400, Johannes Weiner wrote:
> On Mon, Sep 21, 2015 at 11:52:37AM +0100, Mel Gorman wrote:
> > @@ -119,10 +134,10 @@ struct vm_area_struct;
> >  #define GFP_USER	(__GFP_WAIT | __GFP_IO | __GFP_FS | __GFP_HARDWALL)
> >  #define GFP_HIGHUSER	(GFP_USER | __GFP_HIGHMEM)
> >  #define GFP_HIGHUSER_MOVABLE	(GFP_HIGHUSER | __GFP_MOVABLE)
> > -#define GFP_IOFS	(__GFP_IO | __GFP_FS)
> > -#define GFP_TRANSHUGE	(GFP_HIGHUSER_MOVABLE | __GFP_COMP | \
> > -			 __GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_NOWARN | \
> > -			 __GFP_NO_KSWAPD)
> > +#define GFP_IOFS	(__GFP_IO | __GFP_FS | __GFP_KSWAPD_RECLAIM)
> 
> These are some really odd semantics to be given a name like that.
> 
> GFP_IOFS was introduced as a short-hand for testing/setting/clearing
> these two bits at the same time, not to be used for allocations. In
> fact, the only user for allocations is lustre, and it's not at all
> obious why those sites shouldn't include __GFP_WAIT as well.
> 
> Removing this definition altogether would probably be best.

Ok, I'll add a TODO to create a patch that removes GFP_IOFS entirely. It
can be tacked on to the end of the series.

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

[toc] | [prev] | [next] | [standalone]

#1233025

FromJohannes Weiner <hannes@cmpxchg.org>
Date2015-09-25 21:10 +0200
Message-ID<qcGxA-1U9-21@gated-at.bofh.it>
In reply to#1232775
On Fri, Sep 25, 2015 at 01:51:06PM +0100, Mel Gorman wrote:
> On Thu, Sep 24, 2015 at 04:55:09PM -0400, Johannes Weiner wrote:
> > On Mon, Sep 21, 2015 at 11:52:37AM +0100, Mel Gorman wrote:
> > > @@ -119,10 +134,10 @@ struct vm_area_struct;
> > >  #define GFP_USER	(__GFP_WAIT | __GFP_IO | __GFP_FS | __GFP_HARDWALL)
> > >  #define GFP_HIGHUSER	(GFP_USER | __GFP_HIGHMEM)
> > >  #define GFP_HIGHUSER_MOVABLE	(GFP_HIGHUSER | __GFP_MOVABLE)
> > > -#define GFP_IOFS	(__GFP_IO | __GFP_FS)
> > > -#define GFP_TRANSHUGE	(GFP_HIGHUSER_MOVABLE | __GFP_COMP | \
> > > -			 __GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_NOWARN | \
> > > -			 __GFP_NO_KSWAPD)
> > > +#define GFP_IOFS	(__GFP_IO | __GFP_FS | __GFP_KSWAPD_RECLAIM)
> > 
> > These are some really odd semantics to be given a name like that.
> > 
> > GFP_IOFS was introduced as a short-hand for testing/setting/clearing
> > these two bits at the same time, not to be used for allocations. In
> > fact, the only user for allocations is lustre, and it's not at all
> > obious why those sites shouldn't include __GFP_WAIT as well.
> > 
> > Removing this definition altogether would probably be best.
> 
> Ok, I'll add a TODO to create a patch that removes GFP_IOFS entirely. It
> can be tacked on to the end of the series.

Okay, that makes sense to me. Thanks!

Acked-by: Johannes Weiner <hannes@cmpxchg.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1235062

FromMel Gorman <mgorman@techsingularity.net>
Date2015-09-29 15:40 +0200
Message-ID<qe3ir-7Ue-15@gated-at.bofh.it>
In reply to#1233025
> > Ok, I'll add a TODO to create a patch that removes GFP_IOFS entirely. It
> > can be tacked on to the end of the series.
> 
> Okay, that makes sense to me. Thanks!
> 

This?

---8<---
mm: page_alloc: Remove GFP_IOFS

GFP_IOFS was intended to be shorthand for clearing two flags, not a
set of allocation flags. There is only one user of this flag combination
now and there appears to be no reason why Lustre had to be protected
from reclaim stalls. As none of the sites appear to be atomic, this
patch simply deletes GFP_IOFS and converts Lustre to using GFP_KERNEL.

Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
---
 drivers/staging/lustre/lnet/lnet/router.c           |  2 +-
 drivers/staging/lustre/lnet/selftest/conrpc.c       |  2 +-
 drivers/staging/lustre/lnet/selftest/rpc.c          |  2 +-
 drivers/staging/lustre/lustre/libcfs/module.c       |  2 +-
 drivers/staging/lustre/lustre/libcfs/tracefile.c    |  2 +-
 drivers/staging/lustre/lustre/llite/remote_perm.c   |  2 +-
 drivers/staging/lustre/lustre/mgc/mgc_request.c     | 10 +++++-----
 drivers/staging/lustre/lustre/obdecho/echo_client.c |  2 +-
 drivers/staging/lustre/lustre/osc/osc_cache.c       |  2 +-
 include/linux/gfp.h                                 |  1 -
 10 files changed, 13 insertions(+), 14 deletions(-)

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);
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);
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);
 	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;
 
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;
 
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);
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;
 
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 60b2db94d49d..369227202ac2 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -134,7 +134,6 @@ struct vm_area_struct;
 #define GFP_USER	(__GFP_RECLAIM | __GFP_IO | __GFP_FS | __GFP_HARDWALL)
 #define GFP_HIGHUSER	(GFP_USER | __GFP_HIGHMEM)
 #define GFP_HIGHUSER_MOVABLE	(GFP_HIGHUSER | __GFP_MOVABLE)
-#define GFP_IOFS	(__GFP_IO | __GFP_FS | __GFP_KSWAPD_RECLAIM)
 #define GFP_TRANSHUGE	((GFP_HIGHUSER_MOVABLE | __GFP_COMP | \
 			 __GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_NOWARN) & \
 			 ~__GFP_KSWAPD_RECLAIM)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

FromVlastimil Babka <vbabka@suse.cz>
Date2015-09-30 14:30 +0200
SubjectRe: [PATCH 05/10] mm, page_alloc: Distinguish between being unable to sleep, unwilling to sleep and avoiding waking kswapd
Message-ID<qeoGe-4Tw-5@gated-at.bofh.it>
In reply to#1235062
[+CC lustre maintainers]

On 09/29/2015 03:35 PM, Mel Gorman wrote:
>>> Ok, I'll add a TODO to create a patch that removes GFP_IOFS entirely. It
>>> can be tacked on to the end of the series.
>>
>> Okay, that makes sense to me. Thanks!
>>
>
> This?

Thanks for adding this, I think I also pointed this GFP_IOFS oddness in 
earlier versions.

> ---8<---
> mm: page_alloc: Remove GFP_IOFS
>
> GFP_IOFS was intended to be shorthand for clearing two flags, not a
> set of allocation flags. There is only one user of this flag combination
> now and there appears to be no reason why Lustre had to be protected

Looks like a mistake to me. __GFP_IO | __GFP_FS have no effect without 
(former) __GFP_WAIT, so I doubt __GFP_WAIT was omitted on purpose, while 
leaving the other two. The naming of GFP_IOFS suggested it was to be 
used in allocations, leading to the mistake.

But I see you also converted several instances of GFP_NOFS to 
GFP_KERNEL. Is that correct? This is a filesystem driver after all...

> from reclaim stalls. As none of the sites appear to be atomic, this
> patch simply deletes GFP_IOFS and converts Lustre to using GFP_KERNEL.
>
> Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> ---
>   drivers/staging/lustre/lnet/lnet/router.c           |  2 +-
>   drivers/staging/lustre/lnet/selftest/conrpc.c       |  2 +-
>   drivers/staging/lustre/lnet/selftest/rpc.c          |  2 +-
>   drivers/staging/lustre/lustre/libcfs/module.c       |  2 +-
>   drivers/staging/lustre/lustre/libcfs/tracefile.c    |  2 +-
>   drivers/staging/lustre/lustre/llite/remote_perm.c   |  2 +-
>   drivers/staging/lustre/lustre/mgc/mgc_request.c     | 10 +++++-----
>   drivers/staging/lustre/lustre/obdecho/echo_client.c |  2 +-
>   drivers/staging/lustre/lustre/osc/osc_cache.c       |  2 +-
>   include/linux/gfp.h                                 |  1 -
>   10 files changed, 13 insertions(+), 14 deletions(-)
>
> 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);
> 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);
> 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;
>
> 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;
>
> 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);
> 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;
>
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 60b2db94d49d..369227202ac2 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -134,7 +134,6 @@ struct vm_area_struct;
>   #define GFP_USER	(__GFP_RECLAIM | __GFP_IO | __GFP_FS | __GFP_HARDWALL)
>   #define GFP_HIGHUSER	(GFP_USER | __GFP_HIGHMEM)
>   #define GFP_HIGHUSER_MOVABLE	(GFP_HIGHUSER | __GFP_MOVABLE)
> -#define GFP_IOFS	(__GFP_IO | __GFP_FS | __GFP_KSWAPD_RECLAIM)
>   #define GFP_TRANSHUGE	((GFP_HIGHUSER_MOVABLE | __GFP_COMP | \
>   			 __GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_NOWARN) & \
>   			 ~__GFP_KSWAPD_RECLAIM)
>

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

[toc] | [prev] | [next] | [standalone]

#1236277

FromMel Gorman <mgorman@techsingularity.net>
Date2015-09-30 15:20 +0200
Message-ID<qepsC-63w-15@gated-at.bofh.it>
In reply to#1236234
On Wed, Sep 30, 2015 at 02:26:24PM +0200, Vlastimil Babka wrote:
> [+CC lustre maintainers]
> 
> On 09/29/2015 03:35 PM, Mel Gorman wrote:
> >>>Ok, I'll add a TODO to create a patch that removes GFP_IOFS entirely. It
> >>>can be tacked on to the end of the series.
> >>
> >>Okay, that makes sense to me. Thanks!
> >>
> >
> >This?
> 
> Thanks for adding this, I think I also pointed this GFP_IOFS oddness in
> earlier versions.
> 
> >---8<---
> >mm: page_alloc: Remove GFP_IOFS
> >
> >GFP_IOFS was intended to be shorthand for clearing two flags, not a
> >set of allocation flags. There is only one user of this flag combination
> >now and there appears to be no reason why Lustre had to be protected
> 
> Looks like a mistake to me. __GFP_IO | __GFP_FS have no effect without
> (former) __GFP_WAIT, so I doubt __GFP_WAIT was omitted on purpose, while
> leaving the other two. The naming of GFP_IOFS suggested it was to be used in
> allocations, leading to the mistake.
> 

GFP_IOFS is shorthand clearing bits and should not have been used as an
allocation flag. Using it as an allocation flag is almost certainly a
mistake.

At a stretch, GFP_IOFS could make sense if we supprted page reclaim that does
not block (e.g. discard clean pages without buffers to release) but we don't.

> But I see you also converted several instances of GFP_NOFS to GFP_KERNEL. Is
> that correct? This is a filesystem driver after all...
> 

Only in the cases where a reclaim path is reentrant and could already be
holding locks that results in deadlock. I didn't spot such a case but then
again, I'm not familiar with the filesystem and it's complex.

Lets see what they say because how they are currently using GFP_IOFS is
almost certainly wrong or at least surprising.

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

True.

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

[toc] | [prev] | [next] | [standalone]

#1236923

From"Drokin, Oleg" <oleg.drokin@intel.com>
Date2015-10-01 05:10 +0200
Message-ID<qeCpQ-7Y8-7@gated-at.bofh.it>
In reply to#1236234
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!

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

[toc] | [prev] | [next] | [standalone]

#1238164

FromMel Gorman <mgorman@techsingularity.net>
Date2015-10-02 14:40 +0200
Message-ID<qf7N0-3r7-3@gated-at.bofh.it>
In reply to#1236923
On Thu, Oct 01, 2015 at 03:04:37AM +0000, Drokin, Oleg wrote:
> Hello!

Thanks Oleg for taking a look! Can you review the following please?

---8<---
mm: page_alloc: Remove GFP_IOFS

GFP_IOFS was intended to be shorthand for clearing two flags, not a set of
allocation flags. There is only one user of this flag combination now and
there appears to be no reason why Lustre had to be protected from reclaim
stalls. As none of the sites appear to be atomic, this patch simply deletes
GFP_IOFS and converts Lustre to using GFP_KERNEL, GFP_NOFS or GFP_NOIO
as appropriate.

Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
---
 drivers/staging/lustre/lnet/lnet/router.c           | 2 +-
 drivers/staging/lustre/lnet/selftest/conrpc.c       | 2 +-
 drivers/staging/lustre/lnet/selftest/rpc.c          | 2 +-
 drivers/staging/lustre/lustre/libcfs/module.c       | 2 +-
 drivers/staging/lustre/lustre/libcfs/tracefile.c    | 2 +-
 drivers/staging/lustre/lustre/llite/remote_perm.c   | 2 +-
 drivers/staging/lustre/lustre/mgc/mgc_request.c     | 8 ++++----
 drivers/staging/lustre/lustre/obdecho/echo_client.c | 2 +-
 drivers/staging/lustre/lustre/osc/osc_cache.c       | 2 +-
 include/linux/gfp.h                                 | 1 -
 10 files changed, 12 insertions(+), 13 deletions(-)

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);
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);
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);
 	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..b2830035faba 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_NOFS);
 	if (!hash)
 		return NULL;
 
diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
index 019ee2f256aa..67ac6945a8b9 100644
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -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;
 
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);
diff --git a/drivers/staging/lustre/lustre/osc/osc_cache.c b/drivers/staging/lustre/lustre/osc/osc_cache.c
index c72035e048aa..d05a4632f1fe 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_NOIO);
 	if (ext == NULL)
 		return NULL;
 
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 666498d2f6a8..67654f08a28b 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -244,7 +244,6 @@ struct vm_area_struct;
 #define GFP_DMA32	__GFP_DMA32
 #define GFP_HIGHUSER	(GFP_USER | __GFP_HIGHMEM)
 #define GFP_HIGHUSER_MOVABLE	(GFP_HIGHUSER | __GFP_MOVABLE)
-#define GFP_IOFS	(__GFP_IO | __GFP_FS | __GFP_KSWAPD_RECLAIM)
 #define GFP_TRANSHUGE	((GFP_HIGHUSER_MOVABLE | __GFP_COMP | \
 			 __GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_NOWARN) & \
 			 ~__GFP_KSWAPD_RECLAIM)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web