Re: [PATCH] iwlwifi: pcie: make array prop static, shrinks object size

Joe Perches
2018-06-11T19:40:55Z

(adding Greg KH)

On Mon, 2018-06-11 at 18:15 +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Don't populate the read-only array 'prop' on the stack but
> instead make it static. Makes the object code smaller by 20 bytes:
>
> Before:
> text data bss dec hex filename
> 71659 14614 576 86849 15341 trans.o
>
> After:
> text data bss dec hex filename
> 71479 14774 576 86829 1532d trans.o
>
> (gcc version 7.3.0 x86_64)
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> index 7229991ae70d..c4626ebe5da1 100644
> --- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> +++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> @@ -1946,7 +1946,7 @@ static void iwl_trans_pcie_removal_wk(struct work_struct *wk)
> struct iwl_trans_pcie_removal *removal > container_of(wk, struct iwl_trans_pcie_removal, work);
> struct pci_dev *pdev = removal->pdev;
> - char *prop[] = {"EVENT=INACCESSIBLE", NULL};
> + static char *prop[] = {"EVENT=INACCESSIBLE", NULL};
>
> dev_err(&pdev->dev, "Device gone - attempting removal\n");
> kobject_uevent_env(&pdev->dev.kobj, KOBJ_CHANGE, prop);

Now what is happening is that prop is being reloaded
each invocation with the constant addresses of the strings.

It seems the prototype and function for kobject_uevent_env
should change as well to avoid this.

Perhaps this should become:
---
drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 2 +-
include/linux/kobject.h | 2 +-
lib/kobject_uevent.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index 7229991ae70d..6668a8aad22e 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -1946,7 +1946,7 @@ static void iwl_trans_pcie_removal_wk(struct work_struct *wk)
struct iwl_trans_pcie_removal *removal container_of(wk, struct iwl_trans_pcie_removal, work);
struct pci_dev *pdev = removal->pdev;
- char *prop[] = {"EVENT=INACCESSIBLE", NULL};
+ static const char * const prop[] = {"EVENT=INACCESSIBLE", NULL};

dev_err(&pdev->dev, "Device gone - attempting removal\n");
kobject_uevent_env(&pdev->dev.kobj, KOBJ_CHANGE, prop);
diff --git a/include/linux/kobject.h b/include/linux/kobject.h
index 7f6f93c3df9c..9f5cf553dd1e 100644
--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -217,7 +217,7 @@ extern struct kobject *firmware_kobj;

int kobject_uevent(struct kobject *kobj, enum kobject_action action);
int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
- char *envp[]);
+ const char * const envp[]);
int kobject_synth_uevent(struct kobject *kobj, const char *buf, size_t count);

__printf(2, 3)
diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
index 63d0816ab23b..9107989a0cc8 100644
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -452,7 +452,7 @@ static void zap_modalias_env(struct kobj_uevent_env *env)
* corresponding error when it fails.
*/
int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
- char *envp_ext[])
+ const char * const envp_ext[])
{
struct kobj_uevent_env *env;
const char *action_string = kobject_actions[action];