[Cluster-devel] [PATCH] fs/gfs2/glock.c: Deinline do_error, save 1856 bytes

[Cluster-devel] [PATCH] fs/gfs2/glock.c: Deinline do_error, save 1856 bytes

[Denys Vlasenko]

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 at redhat.com
CC: linux-kernel at 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

[Cluster-devel] [PATCH] fs/gfs2/glock.c: Deinline do_error, save 1856 bytes

[Bob Peterson]

----- 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 at redhat.com
> CC: linux-kernel at 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

[Cluster-devel] [PATCH] fs/gfs2/glock.c: Deinline do_error, save 1856 bytes

[Denys Vlasenko]

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.

[Cluster-devel] [PATCH] fs/gfs2/glock.c: Deinline do_error, save 1856 bytes

[Bob Peterson]

----- 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 at redhat.com
> CC: linux-kernel at 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