From: Jonathan Toppins <jtoppins@redhat.com>
To: Joe Perches <joe@perches.com>, netdev@vger.kernel.org
Cc: Jay Vosburgh <j.vosburgh@gmail.com>,
Veaceslav Falico <vfalico@gmail.com>,
Andy Gospodarek <andy@greyhouse.net>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 2/2] bonding: combine netlink and console error messages
Date: Sun, 8 Aug 2021 22:07:11 -0400 [thread overview]
Message-ID: <b384c564-8467-1504-026c-5a437cad1a14@redhat.com> (raw)
In-Reply-To: <745ab8a85430ad4268a86b0957aa690c1a7a6d0f.camel@perches.com>
On 8/8/21 6:02 AM, Joe Perches wrote:
> On Sat, 2021-08-07 at 17:54 -0400, Jonathan Toppins wrote:
>> On 8/6/21 11:52 PM, Joe Perches wrote:
>>> On Fri, 2021-08-06 at 23:30 -0400, Jonathan Toppins wrote:
>>>> There seems to be no reason to have different error messages between
>>>> netlink and printk. It also cleans up the function slightly.
>>> []
>>>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>> []
>>>> +#define BOND_NL_ERR(bond_dev, extack, errmsg) do { \
>>>> + NL_SET_ERR_MSG(extack, errmsg); \
>>>> + netdev_err(bond_dev, "Error: " errmsg "\n"); \
>>>> +} while (0)
>>>> +
>>>> +#define SLAVE_NL_ERR(bond_dev, slave_dev, extack, errmsg) do { \
>>>> + NL_SET_ERR_MSG(extack, errmsg); \
>>>> + slave_err(bond_dev, slave_dev, "Error: " errmsg "\n"); \
>>>> +} while (0)
>>>
>>> If you are doing this, it's probably smaller object code to use
>>> "%s", errmsg
>>> as the errmsg string can be reused
>>>
>>> #define BOND_NL_ERR(bond_dev, extack, errmsg) \
>>> do { \
>>> NL_SET_ERR_MSG(extack, errmsg); \
>>> netdev_err(bond_dev, "Error: %s\n", errmsg); \
>>> } while (0)
>>>
>>> #define SLAVE_NL_ERR(bond_dev, slave_dev, extack, errmsg) \
>>> do { \
>>> NL_SET_ERR_MSG(extack, errmsg); \
>>> slave_err(bond_dev, slave_dev, "Error: %s\n", errmsg); \
>>> } while (0)
>>>
>>>
>>
>> I like the thought and would agree if not for how NL_SET_ERR_MSG is
>> coded. Unfortunately it does not appear as though doing the above change
>> actually generates smaller object code. Maybe I have incorrectly
>> interpreted something?
>
> No, it's because you are compiling allyesconfig or equivalent.
> Try defconfig with bonding.
>
>
$ git clean -dxf
$ git log -1 -p
commit 8985f8d3fa38bca5f5384f9210ed735d58fd94f2 (HEAD ->
upstream-bonding-cleanup)
Author: Jonathan Toppins <jtoppins@redhat.com>
Date: Sun Aug 8 21:45:14 2021 -0400
object code optimization
diff --git a/drivers/net/bonding/bond_main.c
b/drivers/net/bonding/bond_main.c
index 46b95175690b..e2903ae7cdab 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1714,12 +1714,12 @@ void bond_lower_state_changed(struct slave *slave)
#define BOND_NL_ERR(bond_dev, extack, errmsg) do { \
NL_SET_ERR_MSG(extack, errmsg); \
- netdev_err(bond_dev, "Error: " errmsg "\n"); \
+ netdev_err(bond_dev, "Error: %s\n", errmsg); \
} while (0)
#define SLAVE_NL_ERR(bond_dev, slave_dev, extack, errmsg) do { \
NL_SET_ERR_MSG(extack, errmsg); \
- slave_err(bond_dev, slave_dev, "Error: " errmsg "\n"); \
+ slave_err(bond_dev, slave_dev, "Error: %s\n", errmsg); \
} while (0)
/* enslave device <slave> to bond device <master> */
$ git log --oneline -2
8985f8d3fa38 (HEAD -> upstream-bonding-cleanup) object code optimization
e326bf8fd30f bonding: combine netlink and console error messages
$ make defconfig
HOSTCC scripts/basic/fixdep
[...]
*** Default configuration is based on 'x86_64_defconfig'
#
# configuration written to .config
#
$ grep "BONDING" .config
# CONFIG_BONDING is not set
$ make menuconfig
UPD scripts/kconfig/mconf-cfg
[...]
configuration written to .config
*** End of the configuration.
*** Execute 'make' to start the build or try 'make help'.
$ grep "BONDING" .config
CONFIG_BONDING=m
$ git rebase -i --exec "make drivers/net/bonding/bond_main.o; ls -l
drivers/net/bonding/bond_main.o" HEAD^^
Executing: make drivers/net/bonding/bond_main.o; ls -l
drivers/net/bonding/bond_main.o
SYNC include/config/auto.conf.cmd
[...]
CC /home/jtoppins/projects/linux-rhel7/tools/objtool/librbtree.o
LD /home/jtoppins/projects/linux-rhel7/tools/objtool/objtool-in.o
LINK /home/jtoppins/projects/linux-rhel7/tools/objtool/objtool
CC [M] drivers/net/bonding/bond_main.o
-rw-r--r--. 1 jtoppins jtoppins 131800 Aug 8 21:47
drivers/net/bonding/bond_main.o
Executing: make drivers/net/bonding/bond_main.o; ls -l
drivers/net/bonding/bond_main.o
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
DESCEND objtool
CC [M] drivers/net/bonding/bond_main.o
-rw-r--r--. 1 jtoppins jtoppins 131928 Aug 8 21:47
drivers/net/bonding/bond_main.o
Successfully rebased and updated refs/heads/upstream-bonding-cleanup.
$ gcc --version
gcc (GCC) 8.4.1 20200928 (Red Hat 8.4.1-1)
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
-Jon
next prev parent reply other threads:[~2021-08-09 2:07 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-07 3:30 [PATCH net-next 0/2] bonding: cleanup header file and error msgs Jonathan Toppins
2021-08-07 3:30 ` [PATCH net-next 1/2] bonding: remove extraneous definitions from bonding.h Jonathan Toppins
2021-08-07 3:30 ` [PATCH net-next 2/2] bonding: combine netlink and console error messages Jonathan Toppins
2021-08-07 3:52 ` Joe Perches
2021-08-07 21:54 ` Jonathan Toppins
2021-08-08 10:02 ` Joe Perches
2021-08-09 2:07 ` Jonathan Toppins [this message]
2021-08-09 5:05 ` Joe Perches
2021-08-09 17:17 ` Jonathan Toppins
2021-08-08 10:16 ` Leon Romanovsky
2021-08-09 1:42 ` Jonathan Toppins
2021-08-09 6:03 ` Leon Romanovsky
2021-08-10 6:40 ` [PATCH net-next v2 " Jonathan Toppins
2021-08-10 6:47 ` Leon Romanovsky
2021-08-11 2:53 ` [PATCH net-next v2 0/2] bonding: cleanup header file and error msgs Jonathan Toppins
2021-08-11 2:53 ` [PATCH net-next v2 1/2] bonding: remove extraneous definitions from bonding.h Jonathan Toppins
2021-08-11 2:53 ` [PATCH net-next v2 2/2] bonding: combine netlink and console error messages Jonathan Toppins
2021-08-11 3:27 ` Joe Perches
2021-08-11 12:49 ` Jakub Kicinski
2021-08-11 13:23 ` Joe Perches
2021-08-11 14:31 ` Jonathan Toppins
2021-08-11 22:10 ` [PATCH net-next v2 0/2] bonding: cleanup header file and error msgs patchwork-bot+netdevbpf
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=b384c564-8467-1504-026c-5a437cad1a14@redhat.com \
--to=jtoppins@redhat.com \
--cc=andy@greyhouse.net \
--cc=davem@davemloft.net \
--cc=j.vosburgh@gmail.com \
--cc=joe@perches.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=vfalico@gmail.com \
/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 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.