All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Reorderings in struct shrinker and struct shrink_control
@ 2018-07-19 10:51 Kirill Tkhai
  2018-07-19 10:51 ` [PATCH 1/2] mm: Keep int fields in struct shrink_control together Kirill Tkhai
  2018-07-19 10:51 ` [PATCH 2/2] mm: Make flags of unsigned type in struct shrinker Kirill Tkhai
  0 siblings, 2 replies; 6+ messages in thread
From: Kirill Tkhai @ 2018-07-19 10:51 UTC (permalink / raw)
  To: akpm, vdavydov.dev, penguin-kernel, chris, gregkh, linux-kernel, ktkhai

These structures are intensively used during reclaim and,
displace other data in cache, so there is no a reason they
have int fields not grouped together.

---

Kirill Tkhai (2):
      mm: Keep int fields in struct shrink_control together
      mm: Make flags of unsigned type in struct shrinker


 include/linux/shrinker.h |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

--
Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] mm: Keep int fields in struct shrink_control together
  2018-07-19 10:51 [PATCH 0/2] Reorderings in struct shrinker and struct shrink_control Kirill Tkhai
@ 2018-07-19 10:51 ` Kirill Tkhai
  2018-07-19 11:10   ` Michal Hocko
  2018-07-19 10:51 ` [PATCH 2/2] mm: Make flags of unsigned type in struct shrinker Kirill Tkhai
  1 sibling, 1 reply; 6+ messages in thread
From: Kirill Tkhai @ 2018-07-19 10:51 UTC (permalink / raw)
  To: akpm, vdavydov.dev, penguin-kernel, chris, gregkh, linux-kernel, ktkhai

gfp_t is of unsigned type, so let's move nid to keep
them together.

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 include/linux/shrinker.h |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
index b154fd2b084c..d58aaaed34a4 100644
--- a/include/linux/shrinker.h
+++ b/include/linux/shrinker.h
@@ -12,6 +12,9 @@
 struct shrink_control {
 	gfp_t gfp_mask;
 
+	/* current node being shrunk (for NUMA aware shrinkers) */
+	int nid;
+
 	/*
 	 * How many objects scan_objects should scan and try to reclaim.
 	 * This is reset before every call, so it is safe for callees
@@ -26,9 +29,6 @@ struct shrink_control {
 	 */
 	unsigned long nr_scanned;
 
-	/* current node being shrunk (for NUMA aware shrinkers) */
-	int nid;
-
 	/* current memcg being shrunk (for memcg aware shrinkers) */
 	struct mem_cgroup *memcg;
 };


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] mm: Make flags of unsigned type in struct shrinker
  2018-07-19 10:51 [PATCH 0/2] Reorderings in struct shrinker and struct shrink_control Kirill Tkhai
  2018-07-19 10:51 ` [PATCH 1/2] mm: Keep int fields in struct shrink_control together Kirill Tkhai
@ 2018-07-19 10:51 ` Kirill Tkhai
  2018-07-19 11:14   ` Michal Hocko
  1 sibling, 1 reply; 6+ messages in thread
From: Kirill Tkhai @ 2018-07-19 10:51 UTC (permalink / raw)
  To: akpm, vdavydov.dev, penguin-kernel, chris, gregkh, linux-kernel, ktkhai

Currently, there are two flags only, so unsigned
is more then enough. Also, move int seeks to keep
these fields together.

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 include/linux/shrinker.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
index d58aaaed34a4..9443cafd1969 100644
--- a/include/linux/shrinker.h
+++ b/include/linux/shrinker.h
@@ -63,9 +63,9 @@ struct shrinker {
 	unsigned long (*scan_objects)(struct shrinker *,
 				      struct shrink_control *sc);
 
-	int seeks;	/* seeks to recreate an obj */
 	long batch;	/* reclaim batch size, 0 = default */
-	unsigned long flags;
+	int seeks;	/* seeks to recreate an obj */
+	unsigned flags;
 
 	/* These are for internal use */
 	struct list_head list;


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] mm: Keep int fields in struct shrink_control together
  2018-07-19 10:51 ` [PATCH 1/2] mm: Keep int fields in struct shrink_control together Kirill Tkhai
@ 2018-07-19 11:10   ` Michal Hocko
  2018-07-19 11:14     ` Kirill Tkhai
  0 siblings, 1 reply; 6+ messages in thread
From: Michal Hocko @ 2018-07-19 11:10 UTC (permalink / raw)
  To: Kirill Tkhai
  Cc: akpm, vdavydov.dev, penguin-kernel, chris, gregkh, linux-kernel

On Thu 19-07-18 13:51:19, Kirill Tkhai wrote:
> gfp_t is of unsigned type, so let's move nid to keep
> them together.

I guess you meant unsigned int so you can fill up a hole in
shrink_control and as such reduce the stack consumption a bit (at least
on 64b).

An output from pahole is usually a great tool to visualize this.

die__process_function: tag not supported (INVALID)!
struct shrink_control {
	gfp_t                      gfp_mask;             /*     0     4 */

	/* XXX 4 bytes hole, try to pack */

	long unsigned int          nr_to_scan;           /*     8     8 */
	long unsigned int          nr_scanned;           /*    16     8 */
	int                        nid;                  /*    24     4 */

	/* XXX 4 bytes hole, try to pack */

	struct mem_cgroup *        memcg;                /*    32     8 */

	/* size: 40, cachelines: 1, members: 5 */
	/* sum members: 32, holes: 2, sum holes: 8 */
	/* last cacheline: 40 bytes */
};

> 
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  include/linux/shrinker.h |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
> index b154fd2b084c..d58aaaed34a4 100644
> --- a/include/linux/shrinker.h
> +++ b/include/linux/shrinker.h
> @@ -12,6 +12,9 @@
>  struct shrink_control {
>  	gfp_t gfp_mask;
>  
> +	/* current node being shrunk (for NUMA aware shrinkers) */
> +	int nid;
> +
>  	/*
>  	 * How many objects scan_objects should scan and try to reclaim.
>  	 * This is reset before every call, so it is safe for callees
> @@ -26,9 +29,6 @@ struct shrink_control {
>  	 */
>  	unsigned long nr_scanned;
>  
> -	/* current node being shrunk (for NUMA aware shrinkers) */
> -	int nid;
> -
>  	/* current memcg being shrunk (for memcg aware shrinkers) */
>  	struct mem_cgroup *memcg;
>  };

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] mm: Make flags of unsigned type in struct shrinker
  2018-07-19 10:51 ` [PATCH 2/2] mm: Make flags of unsigned type in struct shrinker Kirill Tkhai
@ 2018-07-19 11:14   ` Michal Hocko
  0 siblings, 0 replies; 6+ messages in thread
From: Michal Hocko @ 2018-07-19 11:14 UTC (permalink / raw)
  To: Kirill Tkhai
  Cc: akpm, vdavydov.dev, penguin-kernel, chris, gregkh, linux-kernel

On Thu 19-07-18 13:51:27, Kirill Tkhai wrote:
> Currently, there are two flags only, so unsigned
> is more then enough. Also, move int seeks to keep
> these fields together.

a slightly more explanation _why_ would be really helpful. I suspect
this is to pack the structure better,

struct shrinker {
	long unsigned int          (*count_objects)(struct shrinker *, struct shrink_control *); /*     0     8 */
	long unsigned int          (*scan_objects)(struct shrinker *, struct shrink_control *); /*     8     8 */
	int                        seeks;                /*    16     4 */

	/* XXX 4 bytes hole, try to pack */

	long int                   batch;                /*    24     8 */
	long unsigned int          flags;                /*    32     8 */
	struct list_head           list;                 /*    40    16 */
	atomic_long_t *            nr_deferred;          /*    56     8 */
	/* --- cacheline 1 boundary (64 bytes) --- */

	/* size: 64, cachelines: 1, members: 7 */
	/* sum members: 60, holes: 1, sum holes: 4 */
};

suggests so. Which is a good thing.

> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>

other than that looks reasonable to me
Acked-by: Michal Hocko <mhocko@suse.com>
> ---
>  include/linux/shrinker.h |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
> index d58aaaed34a4..9443cafd1969 100644
> --- a/include/linux/shrinker.h
> +++ b/include/linux/shrinker.h
> @@ -63,9 +63,9 @@ struct shrinker {
>  	unsigned long (*scan_objects)(struct shrinker *,
>  				      struct shrink_control *sc);
>  
> -	int seeks;	/* seeks to recreate an obj */
>  	long batch;	/* reclaim batch size, 0 = default */
> -	unsigned long flags;
> +	int seeks;	/* seeks to recreate an obj */
> +	unsigned flags;
>  
>  	/* These are for internal use */
>  	struct list_head list;

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] mm: Keep int fields in struct shrink_control together
  2018-07-19 11:10   ` Michal Hocko
@ 2018-07-19 11:14     ` Kirill Tkhai
  0 siblings, 0 replies; 6+ messages in thread
From: Kirill Tkhai @ 2018-07-19 11:14 UTC (permalink / raw)
  To: Michal Hocko
  Cc: akpm, vdavydov.dev, penguin-kernel, chris, gregkh, linux-kernel

On 19.07.2018 14:10, Michal Hocko wrote:
> On Thu 19-07-18 13:51:19, Kirill Tkhai wrote:
>> gfp_t is of unsigned type, so let's move nid to keep
>> them together.
>
> I guess you meant unsigned int so you can fill up a hole in
> shrink_control and as such reduce the stack consumption a bit (at least
> on 64b).

Yes, I mean unsigned as synonym of unsigned int and this is the reason :)
 
> An output from pahole is usually a great tool to visualize this.
> 
> die__process_function: tag not supported (INVALID)!
> struct shrink_control {
> 	gfp_t                      gfp_mask;             /*     0     4 */
> 
> 	/* XXX 4 bytes hole, try to pack */
> 
> 	long unsigned int          nr_to_scan;           /*     8     8 */
> 	long unsigned int          nr_scanned;           /*    16     8 */
> 	int                        nid;                  /*    24     4 */
> 
> 	/* XXX 4 bytes hole, try to pack */
> 
> 	struct mem_cgroup *        memcg;                /*    32     8 */
> 
> 	/* size: 40, cachelines: 1, members: 5 */
> 	/* sum members: 32, holes: 2, sum holes: 8 */
> 	/* last cacheline: 40 bytes */
> };
>>>
>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> 
> Acked-by: Michal Hocko <mhocko@suse.com>
> 
>> ---
>>  include/linux/shrinker.h |    6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
>> index b154fd2b084c..d58aaaed34a4 100644
>> --- a/include/linux/shrinker.h
>> +++ b/include/linux/shrinker.h
>> @@ -12,6 +12,9 @@
>>  struct shrink_control {
>>  	gfp_t gfp_mask;
>>  
>> +	/* current node being shrunk (for NUMA aware shrinkers) */
>> +	int nid;
>> +
>>  	/*
>>  	 * How many objects scan_objects should scan and try to reclaim.
>>  	 * This is reset before every call, so it is safe for callees
>> @@ -26,9 +29,6 @@ struct shrink_control {
>>  	 */
>>  	unsigned long nr_scanned;
>>  
>> -	/* current node being shrunk (for NUMA aware shrinkers) */
>> -	int nid;
>> -
>>  	/* current memcg being shrunk (for memcg aware shrinkers) */
>>  	struct mem_cgroup *memcg;
>>  };
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-07-19 11:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-19 10:51 [PATCH 0/2] Reorderings in struct shrinker and struct shrink_control Kirill Tkhai
2018-07-19 10:51 ` [PATCH 1/2] mm: Keep int fields in struct shrink_control together Kirill Tkhai
2018-07-19 11:10   ` Michal Hocko
2018-07-19 11:14     ` Kirill Tkhai
2018-07-19 10:51 ` [PATCH 2/2] mm: Make flags of unsigned type in struct shrinker Kirill Tkhai
2018-07-19 11:14   ` Michal Hocko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.