Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1270765 > unrolled thread
| Started by | Yaowei Bai <baiyaowei@cmss.chinamobile.com> |
|---|---|
| First post | 2015-11-17 03:10 +0100 |
| Last post | 2015-11-17 09:40 +0100 |
| Articles | 3 — 2 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.
Re: [PATCH 6/7] mm/gfp: make gfp_zonelist return directly and bool Yaowei Bai <baiyaowei@cmss.chinamobile.com> - 2015-11-17 03:10 +0100
Re: [PATCH 6/7] mm/gfp: make gfp_zonelist return directly and bool David Rientjes <rientjes@google.com> - 2015-11-17 06:50 +0100
Re: [PATCH 6/7] mm/gfp: make gfp_zonelist return directly and bool Yaowei Bai <baiyaowei@cmss.chinamobile.com> - 2015-11-17 09:40 +0100
| From | Yaowei Bai <baiyaowei@cmss.chinamobile.com> |
|---|---|
| Date | 2015-11-17 03:10 +0100 |
| Subject | Re: [PATCH 6/7] mm/gfp: make gfp_zonelist return directly and bool |
| Message-ID | <qvDSy-75S-3@gated-at.bofh.it> |
On Mon, Nov 16, 2015 at 02:05:46AM -0800, David Rientjes wrote:
> On Mon, 16 Nov 2015, Yaowei Bai wrote:
>
> > diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> > index 6523109..1da03f5 100644
> > --- a/include/linux/gfp.h
> > +++ b/include/linux/gfp.h
> > @@ -375,12 +375,9 @@ static inline enum zone_type gfp_zone(gfp_t flags)
> > * virtual kernel addresses to the allocated page(s).
> > */
> >
> > -static inline int gfp_zonelist(gfp_t flags)
> > +static inline bool gfp_zonelist(gfp_t flags)
> > {
> > - if (IS_ENABLED(CONFIG_NUMA) && unlikely(flags & __GFP_THISNODE))
> > - return 1;
> > -
> > - return 0;
> > + return IS_ENABLED(CONFIG_NUMA) && unlikely(flags & __GFP_THISNODE);
> > }
> >
> > /*
>
> This function is used to index into a pgdat's node_zonelists[] array, bool
> makes no sense.
Yes, you'r right, but i think hardcoding the index here is not a good idea.
How about this:
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 6523109..14a6249 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -378,9 +378,9 @@ static inline enum zone_type gfp_zone(gfp_t flags)
static inline int gfp_zonelist(gfp_t flags)
{
if (IS_ENABLED(CONFIG_NUMA) && unlikely(flags & __GFP_THISNODE))
- return 1;
+ return ZONELIST_NOFALLBACK;
- return 0;
+ return ZONELIST_FALLBACK;
}
/*
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index e23a9e7..9664d6c 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -576,8 +576,6 @@ static inline bool zone_is_empty(struct zone *zone)
/* Maximum number of zones on a zonelist */
#define MAX_ZONES_PER_ZONELIST (MAX_NUMNODES * MAX_NR_ZONES)
-#ifdef CONFIG_NUMA
-
/*
* The NUMA zonelists are doubled because we need zonelists that restrict the
* allocations to a single node for __GFP_THISNODE.
@@ -585,10 +583,13 @@ static inline bool zone_is_empty(struct zone *zone)
* [0] : Zonelist with fallback
* [1] : No fallback (__GFP_THISNODE)
*/
-#define MAX_ZONELISTS 2
-#else
-#define MAX_ZONELISTS 1
+enum {
+ ZONELIST_FALLBACK,
+#ifdef CONFIG_NUMA
+ ZONELIST_NOFALLBACK,
#endif
+ MAX_ZONELISTS
+};
/*
* This struct contains information about a zone in a zonelist. It is stored
--
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]
| From | David Rientjes <rientjes@google.com> |
|---|---|
| Date | 2015-11-17 06:50 +0100 |
| Message-ID | <qvHjr-QF-3@gated-at.bofh.it> |
| In reply to | #1270765 |
On Tue, 17 Nov 2015, Yaowei Bai wrote:
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 6523109..14a6249 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -378,9 +378,9 @@ static inline enum zone_type gfp_zone(gfp_t flags)
> static inline int gfp_zonelist(gfp_t flags)
> {
> if (IS_ENABLED(CONFIG_NUMA) && unlikely(flags & __GFP_THISNODE))
> - return 1;
> + return ZONELIST_NOFALLBACK;
>
> - return 0;
> + return ZONELIST_FALLBACK;
> }
>
> /*
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index e23a9e7..9664d6c 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -576,8 +576,6 @@ static inline bool zone_is_empty(struct zone *zone)
> /* Maximum number of zones on a zonelist */
> #define MAX_ZONES_PER_ZONELIST (MAX_NUMNODES * MAX_NR_ZONES)
>
> -#ifdef CONFIG_NUMA
> -
> /*
> * The NUMA zonelists are doubled because we need zonelists that restrict the
> * allocations to a single node for __GFP_THISNODE.
> @@ -585,10 +583,13 @@ static inline bool zone_is_empty(struct zone *zone)
> * [0] : Zonelist with fallback
> * [1] : No fallback (__GFP_THISNODE)
> */
> -#define MAX_ZONELISTS 2
> -#else
> -#define MAX_ZONELISTS 1
> +enum {
> + ZONELIST_FALLBACK,
> +#ifdef CONFIG_NUMA
> + ZONELIST_NOFALLBACK,
> #endif
> + MAX_ZONELISTS
> +};
>
> /*
> * This struct contains information about a zone in a zonelist. It is stored
This is a different change than the original. I don't see a benefit from
it, but I have no strong feelings on it. If someone else finds value in
this, please update the comment when defining the enum as well.
--
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]
| From | Yaowei Bai <baiyaowei@cmss.chinamobile.com> |
|---|---|
| Date | 2015-11-17 09:40 +0100 |
| Message-ID | <qvJXY-2zA-21@gated-at.bofh.it> |
| In reply to | #1270883 |
On Mon, Nov 16, 2015 at 09:44:11PM -0800, David Rientjes wrote:
> On Tue, 17 Nov 2015, Yaowei Bai wrote:
>
> > diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> > index 6523109..14a6249 100644
> > --- a/include/linux/gfp.h
> > +++ b/include/linux/gfp.h
> > @@ -378,9 +378,9 @@ static inline enum zone_type gfp_zone(gfp_t flags)
> > static inline int gfp_zonelist(gfp_t flags)
> > {
> > if (IS_ENABLED(CONFIG_NUMA) && unlikely(flags & __GFP_THISNODE))
> > - return 1;
> > + return ZONELIST_NOFALLBACK;
> >
> > - return 0;
> > + return ZONELIST_FALLBACK;
> > }
> >
> > /*
> > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> > index e23a9e7..9664d6c 100644
> > --- a/include/linux/mmzone.h
> > +++ b/include/linux/mmzone.h
> > @@ -576,8 +576,6 @@ static inline bool zone_is_empty(struct zone *zone)
> > /* Maximum number of zones on a zonelist */
> > #define MAX_ZONES_PER_ZONELIST (MAX_NUMNODES * MAX_NR_ZONES)
> >
> > -#ifdef CONFIG_NUMA
> > -
> > /*
> > * The NUMA zonelists are doubled because we need zonelists that restrict the
> > * allocations to a single node for __GFP_THISNODE.
> > @@ -585,10 +583,13 @@ static inline bool zone_is_empty(struct zone *zone)
> > * [0] : Zonelist with fallback
> > * [1] : No fallback (__GFP_THISNODE)
> > */
> > -#define MAX_ZONELISTS 2
> > -#else
> > -#define MAX_ZONELISTS 1
> > +enum {
> > + ZONELIST_FALLBACK,
> > +#ifdef CONFIG_NUMA
> > + ZONELIST_NOFALLBACK,
> > #endif
> > + MAX_ZONELISTS
> > +};
> >
> > /*
> > * This struct contains information about a zone in a zonelist. It is stored
>
> This is a different change than the original.
The original patch doesn't make sense as you said so let's drop it.
> I don't see a benefit from
> it, but I have no strong feelings on it. If someone else finds value in
> this, please update the comment when defining the enum as well.
OK, i'll send a update patch to review first and if nobody disagrees with it i will
resend this patchset.
--
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