linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Vaneet Narang <v.narang@samsung.com>
To: Michal Hocko <mhocko@kernel.org>,
	Maninder Singh <maninder1.s@samsung.com>
Cc: "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	AMIT SAHRAWAT <a.sahrawat@samsung.com>
Subject: RE:(2) [PATCH 1/1] mm/vmscan.c: change prototype for shrink_page_list
Date: Wed, 29 Apr 2020 18:59:40 +0530	[thread overview]
Message-ID: <20200429132940epcms5p81e75e09469b62253ff512222c568556f@epcms5p8> (raw)
In-Reply-To: <20200429130912.GE28637@dhcp22.suse.cz>

Hi Michal, 

>> >
>> >Acked-by: Michal Hocko <mhocko@suse.com>
>> >
>> >Is there any reason to move declarations here?
>> >
>> 
>> "unsigned int ret" was changed mistakenely, sending V2.
>> and "unsigned int nr_reclaimed" is changed to remove hole.
 
>Could you be more specific? Have you seen a better stack allocation
>footprint?

We didn't check stack allocation footprint, we did changes just by looking into the code.
we thought changing reclaimed type from long to int on 64 bit platform will add 
hole of 4 bytes between two stack variables nr_reclaimed & nr_taken.

So we tried to remove that hole by packing it with bool.

 	unsigned long nr_scanned;                 --> Size and alignment 8 byte for long 
-	unsigned long nr_reclaimed = 0;           --> Changing to int will make its size as 4  
 	unsigned long nr_taken;                   --> nr_taken needs alignment of 8 so will add hole.
 	struct reclaim_stat stat;
 	int file = is_file_lru(lru);
 	enum vm_event_item item;
 	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
 	struct zone_reclaim_stat *reclaim_stat = &lruvec->reclaim_stat;
+	unsigned int nr_reclaimed = 0;            --> So moving to this place to pack it along with bool 
 	bool stalled = false;


Overall stack footprint might not change as compiler makes function stack pointer as 16 byte aligned but we did this 
as we normally follow this coding convention when defining structures or stack variables. 

Thanks & Regards,
Vaneet Narang
 
 

 


  parent reply	other threads:[~2020-04-29 13:29 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20200429055946epcas5p2d5faf2b320913d59a4a8380cb017053c@epcas5p2.samsung.com>
2020-04-29  5:59 ` [PATCH 1/1] mm/vmscan.c: change prototype for shrink_page_list Maninder Singh
2020-04-29 12:09   ` Michal Hocko
     [not found]   ` <CGME20200429055946epcas5p2d5faf2b320913d59a4a8380cb017053c@epcms5p6>
2020-04-29 12:53     ` Maninder Singh
2020-04-29 13:09       ` [PATCH " Michal Hocko
     [not found]       ` <CGME20200429055946epcas5p2d5faf2b320913d59a4a8380cb017053c@epcms5p8>
2020-04-29 13:29         ` Vaneet Narang [this message]
2020-04-29 13:35           ` (2) " Michal Hocko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200429132940epcms5p81e75e09469b62253ff512222c568556f@epcms5p8 \
    --to=v.narang@samsung.com \
    --cc=a.sahrawat@samsung.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=maninder1.s@samsung.com \
    --cc=mhocko@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).