Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1374425 > unrolled thread

[PATCH] drivers/infiniband/hw/nes/nes_verbs.c: Deinline nes_free_qp_mem, save 1072 bytes

Started byDenys Vlasenko <dvlasenk@redhat.com>
First post2016-04-08 21:00 +0200
Last post2016-04-11 07:00 +0200
Articles 8 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] drivers/infiniband/hw/nes/nes_verbs.c: Deinline nes_free_qp_mem, save 1072 bytes Denys Vlasenko <dvlasenk@redhat.com> - 2016-04-08 21:00 +0200
    [PATCH] fs/gfs2/glock.c: Deinline do_error, save 1856 bytes Denys Vlasenko <dvlasenk@redhat.com> - 2016-04-08 21:00 +0200
      Re: [PATCH] fs/gfs2/glock.c: Deinline do_error, save 1856 bytes Bob Peterson <rpeterso@redhat.com> - 2016-04-08 21:20 +0200
        Re: [PATCH] fs/gfs2/glock.c: Deinline do_error, save 1856 bytes Denys Vlasenko <dvlasenk@redhat.com> - 2016-04-08 21:50 +0200
      Re: [PATCH] fs/gfs2/glock.c: Deinline do_error, save 1856 bytes Bob Peterson <rpeterso@redhat.com> - 2016-04-12 18:50 +0200
    [PATCH] lockdep: Deinline register_lock_class, save 2328 bytes Denys Vlasenko <dvlasenk@redhat.com> - 2016-04-08 21:10 +0200
      [tip:locking/core] locking/lockdep: Deinline register_lock_class(),  save 2328 bytes tip-bot for Denys Vlasenko <tipbot@zytor.com> - 2016-04-13 13:40 +0200
    Re: [PATCH] drivers/infiniband/hw/nes/nes_verbs.c: Deinline  nes_free_qp_mem, save 1072 bytes Leon Romanovsky <leon@leon.nu> - 2016-04-11 07:00 +0200

#1374425 — [PATCH] drivers/infiniband/hw/nes/nes_verbs.c: Deinline nes_free_qp_mem, save 1072 bytes

FromDenys Vlasenko <dvlasenk@redhat.com>
Date2016-04-08 21:00 +0200
Subject[PATCH] drivers/infiniband/hw/nes/nes_verbs.c: Deinline nes_free_qp_mem, save 1072 bytes
Message-ID<rlJNn-8ev-7@gated-at.bofh.it>
This function compiles to 550 bytes of machine code.
Three callsites, all in nes_create_qp.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Faisal Latif <faisal.latif@intel.com>
CC: Doug Ledford <dledford@redhat.com>
CC: linux-rdma@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
 drivers/infiniband/hw/nes/nes_verbs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
index fba69a3..5f48d08 100644
--- a/drivers/infiniband/hw/nes/nes_verbs.c
+++ b/drivers/infiniband/hw/nes/nes_verbs.c
@@ -981,7 +981,7 @@ static int nes_setup_mmap_qp(struct nes_qp *nesqp, struct nes_vnic *nesvnic,
 /**
  * nes_free_qp_mem() is to free up the qp's pci_alloc_consistent() memory.
  */
-static inline void nes_free_qp_mem(struct nes_device *nesdev,
+static void nes_free_qp_mem(struct nes_device *nesdev,
 		struct nes_qp *nesqp, int virt_wqs)
 {
 	unsigned long flags;
-- 
2.1.0

[toc] | [next] | [standalone]

#1374426 — [PATCH] fs/gfs2/glock.c: Deinline do_error, save 1856 bytes

FromDenys Vlasenko <dvlasenk@redhat.com>
Date2016-04-08 21:00 +0200
Subject[PATCH] fs/gfs2/glock.c: Deinline do_error, save 1856 bytes
Message-ID<rlJNo-8ev-21@gated-at.bofh.it>
In reply to#1374425
This function compiles to 522 bytes of machine code.

Error paths are not very time critical.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Steven Whitehouse <swhiteho@redhat.com>
CC: Bob Peterson <rpeterso@redhat.com>
CC: cluster-devel@redhat.com
CC: linux-kernel@vger.kernel.org
---
 fs/gfs2/glock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 6539131..c3d5172 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -218,7 +218,7 @@ static void gfs2_holder_wake(struct gfs2_holder *gh)
  *
  */
 
-static inline void do_error(struct gfs2_glock *gl, const int ret)
+static void do_error(struct gfs2_glock *gl, const int ret)
 {
 	struct gfs2_holder *gh, *tmp;
 
-- 
2.1.0

[toc] | [prev] | [next] | [standalone]

#1374437 — Re: [PATCH] fs/gfs2/glock.c: Deinline do_error, save 1856 bytes

FromBob Peterson <rpeterso@redhat.com>
Date2016-04-08 21:20 +0200
SubjectRe: [PATCH] fs/gfs2/glock.c: Deinline do_error, save 1856 bytes
Message-ID<rlK6J-aw-7@gated-at.bofh.it>
In reply to#1374426
----- Original Message -----
> This function compiles to 522 bytes of machine code.
> 
> Error paths are not very time critical.
> 
> Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
> CC: Steven Whitehouse <swhiteho@redhat.com>
> CC: Bob Peterson <rpeterso@redhat.com>
> CC: cluster-devel@redhat.com
> CC: linux-kernel@vger.kernel.org
> ---
>  fs/gfs2/glock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
> index 6539131..c3d5172 100644
> --- a/fs/gfs2/glock.c
> +++ b/fs/gfs2/glock.c
> @@ -218,7 +218,7 @@ static void gfs2_holder_wake(struct gfs2_holder *gh)
>   *
>   */
>  
> -static inline void do_error(struct gfs2_glock *gl, const int ret)
> +static void do_error(struct gfs2_glock *gl, const int ret)
>  {
>  	struct gfs2_holder *gh, *tmp;
>  
> --
> 2.1.0
> 
> 

Hi Denys,

The name is misleading. Function do_error() isn't really an error path.
Its job is to "fail" all the holders for a glock that are doing a "try" lock
in cases where trying the lock has been determined to have failed.

Is there a reason why you want to trade memory for speed? Are you
optimizing for memory on an embedded device or something?
I guess I have no fundamental problem in adding this patch, but perhaps
Steve or someone can offer a second opinion before I do.

Regards,

Bob Peterson
Red Hat File Systems

[toc] | [prev] | [next] | [standalone]

#1374443 — Re: [PATCH] fs/gfs2/glock.c: Deinline do_error, save 1856 bytes

FromDenys Vlasenko <dvlasenk@redhat.com>
Date2016-04-08 21:50 +0200
SubjectRe: [PATCH] fs/gfs2/glock.c: Deinline do_error, save 1856 bytes
Message-ID<rlKzM-mK-1@gated-at.bofh.it>
In reply to#1374437
On 04/08/2016 09:17 PM, Bob Peterson wrote:
> ----- Original Message -----
>> This function compiles to 522 bytes of machine code.
>>
> Is there a reason why you want to trade memory for speed? Are you
> optimizing for memory on an embedded device or something?

Yes. I did a scan for really large inlines and this function came up.
If you feel it is indeed performance critical, please ignore my patch.

[toc] | [prev] | [next] | [standalone]

#1377010 — Re: [PATCH] fs/gfs2/glock.c: Deinline do_error, save 1856 bytes

FromBob Peterson <rpeterso@redhat.com>
Date2016-04-12 18:50 +0200
SubjectRe: [PATCH] fs/gfs2/glock.c: Deinline do_error, save 1856 bytes
Message-ID<rn9FL-22N-7@gated-at.bofh.it>
In reply to#1374426
----- Original Message -----
> This function compiles to 522 bytes of machine code.
> 
> Error paths are not very time critical.
> 
> Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
> CC: Steven Whitehouse <swhiteho@redhat.com>
> CC: Bob Peterson <rpeterso@redhat.com>
> CC: cluster-devel@redhat.com
> CC: linux-kernel@vger.kernel.org
> ---
>  fs/gfs2/glock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
> index 6539131..c3d5172 100644
> --- a/fs/gfs2/glock.c
> +++ b/fs/gfs2/glock.c
> @@ -218,7 +218,7 @@ static void gfs2_holder_wake(struct gfs2_holder *gh)
>   *
>   */
>  
> -static inline void do_error(struct gfs2_glock *gl, const int ret)
> +static void do_error(struct gfs2_glock *gl, const int ret)
>  {
>  	struct gfs2_holder *gh, *tmp;
>  
> --
> 2.1.0
> 
> 

Hi,

Thanks. This is now applied to the for-next branch of the linux-gfs2 tree:
https://git.kernel.org/cgit/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs/gfs2?h=for-next&id=a527b38e1475211b67eb59b3fadb40689f035529

Regards,

Bob Peterson
Red Hat File Systems

[toc] | [prev] | [next] | [standalone]

#1374429 — [PATCH] lockdep: Deinline register_lock_class, save 2328 bytes

FromDenys Vlasenko <dvlasenk@redhat.com>
Date2016-04-08 21:10 +0200
Subject[PATCH] lockdep: Deinline register_lock_class, save 2328 bytes
Message-ID<rlJX3-7e-9@gated-at.bofh.it>
In reply to#1374425
This function compiles to 1328 bytes of machine code. Three callsites.

Registering a new lock class is definitely not *that* time-critical to inline it.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Ingo Molnar <mingo@redhat.com>
CC: linux-kernel@vger.kernel.org
---
 kernel/locking/lockdep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 53ab2f8..ffad373 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -708,7 +708,7 @@ look_up_lock_class(struct lockdep_map *lock, unsigned int subclass)
  * yet. Otherwise we look it up. We cache the result in the lock object
  * itself, so actual lookup of the hash should be once per lock object.
  */
-static inline struct lock_class *
+static struct lock_class *
 register_lock_class(struct lockdep_map *lock, unsigned int subclass, int force)
 {
 	struct lockdep_subclass_key *key;
-- 
2.1.0

[toc] | [prev] | [next] | [standalone]

#1377788 — [tip:locking/core] locking/lockdep: Deinline register_lock_class(), save 2328 bytes

Fromtip-bot for Denys Vlasenko <tipbot@zytor.com>
Date2016-04-13 13:40 +0200
Subject[tip:locking/core] locking/lockdep: Deinline register_lock_class(), save 2328 bytes
Message-ID<rnrjk-x5-15@gated-at.bofh.it>
In reply to#1374429
Commit-ID:  c003ed928962a55eb446e78c544b1d7c4f6cb88a
Gitweb:     http://git.kernel.org/tip/c003ed928962a55eb446e78c544b1d7c4f6cb88a
Author:     Denys Vlasenko <dvlasenk@redhat.com>
AuthorDate: Fri, 8 Apr 2016 20:58:46 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 13 Apr 2016 10:06:13 +0200

locking/lockdep: Deinline register_lock_class(), save 2328 bytes

This function compiles to 1328 bytes of machine code. Three callsites.

Registering a new lock class is definitely not *that* time-critical to inline it.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
Link: http://lkml.kernel.org/r/1460141926-13069-5-git-send-email-dvlasenk@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/locking/lockdep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index ed94109..7cc43ef 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -708,7 +708,7 @@ look_up_lock_class(struct lockdep_map *lock, unsigned int subclass)
  * yet. Otherwise we look it up. We cache the result in the lock object
  * itself, so actual lookup of the hash should be once per lock object.
  */
-static inline struct lock_class *
+static struct lock_class *
 register_lock_class(struct lockdep_map *lock, unsigned int subclass, int force)
 {
 	struct lockdep_subclass_key *key;

[toc] | [prev] | [next] | [standalone]

#1375492 — Re: [PATCH] drivers/infiniband/hw/nes/nes_verbs.c: Deinline nes_free_qp_mem, save 1072 bytes

FromLeon Romanovsky <leon@leon.nu>
Date2016-04-11 07:00 +0200
SubjectRe: [PATCH] drivers/infiniband/hw/nes/nes_verbs.c: Deinline nes_free_qp_mem, save 1072 bytes
Message-ID<rmC79-6h-7@gated-at.bofh.it>
In reply to#1374425

[Multipart message — attachments visible in raw view] — view raw

On Fri, Apr 08, 2016 at 08:58:42PM +0200, Denys Vlasenko wrote:
> This function compiles to 550 bytes of machine code.
> Three callsites, all in nes_create_qp.

I agree with you, the functions which calls below and after this
function are not optimized for speed and there is no need to inline
this function.

I have two requests from you.
1)
Can you please change title to be more convenient?
[PATCH] drivers/infiniband/hw/nes/nes_verbs.c: Deinline nes_free_qp_mem, save 1072 bytes
--->
[PATCH] IB/nes: Deinline nes_free_qp_mem

2) Add bloat-o-meter output to the commit message.

And after that feel free to add my RB tag.

Reviewed-By: Leon Romanovsky <leonro@mellanox.com>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web