Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

CORTX-33743: Analyze and fix memory leaks #2088

Merged
merged 33 commits into from
Aug 31, 2022

Conversation

swapnil-seagate
Copy link
Contributor

CORTX-33743: Analyse and fix memory leaks found in glibc when running s3 IOs with valgrind

Problem statement:
Valgrind reported few leaks in the code, where strdup was used for allocating memory to the duplicate string.

Fix Description:
Analyzed the code statically and freed up the variables allocated memory dynamically during the execution.
Attaching the valrind report with the code changes, where no leak is found wrt the leaks described of the JIRA ticket.

Coding

Checklist for Author

  • [ y] Coding conventions are followed and code is consistent

Testing

Checklist for Author

  • Unit and System Tests are added
  • Test Cases cover Happy Path, Non-Happy Path and Scalability
  • Testing was performed with RPM

Impact Analysis

Checklist for Author/Reviewer/GateKeeper

  • Interface change (if any) are documented
  • Side effects on other features (deployment/upgrade)
  • Dependencies on other component(s)

Review Checklist

Checklist for Author

  • [ y] JIRA number/GitHub Issue added to PR
  • [ y] PR is self reviewed
  • [ y] Jira and state/status is updated and JIRA is updated with PR link
  • [ y] Check if the description is clear and explained

Documentation

Checklist for Author

  • Changes done to WIKI / Confluence page / Quick Start Guide

@rkothiya
Copy link
Contributor

Jenkins CI Result : Motr#1623

Motr Test Summary

Test ResultCountInfo
❌Failed2
📁

04motr-single-node/49motr-rpc-cancel
01motr-single-node/00userspace-tests

🏁Skipped32
📁

01motr-single-node/28sys-kvs
01motr-single-node/35m0singlenode
01motr-single-node/04initscripts
01motr-single-node/37protocol
02motr-single-node/51kem
02motr-single-node/20rpc-session-cancel
02motr-single-node/10pver-assign
02motr-single-node/21fsync-single-node
02motr-single-node/13dgmode-io
02motr-single-node/14poolmach
02motr-single-node/11m0t1fs
02motr-single-node/26motr-user-kernel-tests
02motr-single-node/08spiel
03motr-single-node/06conf
03motr-single-node/36spare-reservation
04motr-single-node/34sns-repair-1n-1f
04motr-single-node/08spiel-sns-repair-quiesce
04motr-single-node/28sys-kvs-kernel
04motr-single-node/11m0t1fs-rconfc-fail
04motr-single-node/08spiel-sns-repair
04motr-single-node/19sns-repair-abort
04motr-single-node/22sns-repair-ios-fail
05motr-single-node/18sns-repair-quiesce
05motr-single-node/12fwait
05motr-single-node/16sns-repair-multi
05motr-single-node/07mount-fail
05motr-single-node/15sns-repair-single
05motr-single-node/23sns-abort-quiesce
05motr-single-node/17sns-repair-concurrent-io
05motr-single-node/07mount
05motr-single-node/07mount-multiple
05motr-single-node/12fsync

✔️Passed43
📁

01motr-single-node/43m0crate
01motr-single-node/05confgen
01motr-single-node/06hagen
01motr-single-node/52motr-singlenode-sanity
01motr-single-node/01net
01motr-single-node/01kernel-tests
01motr-single-node/03console
01motr-single-node/02rpcping
02motr-single-node/07m0d-fatal
02motr-single-node/67fdmi-plugin-multi-filters
02motr-single-node/53clusterusage-alert
02motr-single-node/41motr-conf-update
03motr-single-node/61sns-repair-motr-1n-1f
03motr-single-node/72spiel-sns-motr-repair-quiesce
03motr-single-node/08spiel-multi-confd
03motr-single-node/69sns-repair-motr-quiesce
03motr-single-node/62sns-repair-motr-mf
03motr-single-node/70sns-failure-after-repair-quiesce
03motr-single-node/63sns-repair-motr-1k-1f
03motr-single-node/60sns-repair-motr-1f
03motr-single-node/66sns-repair-motr-abort-quiesce
03motr-single-node/24motr-dix-repair-lookup-insert-spiel
03motr-single-node/68sns-repair-motr-shutdown
03motr-single-node/64sns-repair-motr-ios-fail
03motr-single-node/71spiel-sns-motr-repair
03motr-single-node/24motr-dix-repair-lookup-insert-m0repair
03motr-single-node/04sss
03motr-single-node/65sns-repair-motr-abort
04motr-single-node/73motr-io-small-disks
04motr-single-node/48motr-raid0-io
04motr-single-node/74motr-di-corruption-detection
04motr-single-node/25m0kv
04motr-single-node/44motr-rm-lock-cc-io
04motr-single-node/45motr-rmw
05motr-single-node/23dix-repair-m0repair
05motr-single-node/43motr-sync-replication
05motr-single-node/42motr-utils
05motr-single-node/45motr-sns-repair-N-1
05motr-single-node/40motr-dgmode
05motr-single-node/23dix-repair-quiesce-m0repair
05motr-single-node/23spiel-dix-repair-quiesce
05motr-single-node/44motr-sns-repair
05motr-single-node/23spiel-dix-repair

Total77🔗

CppCheck Summary

   Cppcheck: No new warnings found 👍

ha/entrypoint.c Outdated Show resolved Hide resolved
motr/setup.c Outdated Show resolved Hide resolved
ha/entrypoint.c Outdated Show resolved Hide resolved
motr/setup.c Outdated Show resolved Hide resolved
swapnil-seagate and others added 6 commits August 30, 2022 12:16
… s3 IOs with valgrind

Problem statement:
Valgrind reported few leaks in the code, where strdup was used for allocating memory to the duplicate string.

Fix Description:
Analyzed the code statically and freed up the variables allocated memory dynamically during the execution.
Attaching the valrind report with the code changes, where no leak is found wrt the leaks described of the JIRA ticket.

Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
[fenced-code-flag] -> added name of programming language after 3 backquotes ```

Signed-off-by: Pratik Patil <pratik.k.patil@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
Problem:
Unable to load gdb-extensions.py. Failing with syntax errors

Solution:
Introduce parenthesis for "print" function. Replace "long" with "int".
Removed macro definition as gdb.execute() is throwing errors.
User need to manually define macros at gdb command prompt.
Added comments for the same inside the file.

Signed-off-by: Naga Kishore Kommuri <nagakishore.kommuri@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
Problem:
Codacy warning, "shellcheck: Tips depend on target shell and
yours is unknown.

Solution:
Add a shebang or a 'shell' directive"

Signed-off-by: Venkateswarlu Payidimarry <venkateswarlu.payidimarry@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
This patch fixes some of the codacy warnings.
warning fixed : "Double quote to prevent globing and words splitting".

Signed-off-by: alfhad <fahadshah2411@gmail.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
)

Problem:
We see 1688 occurrences of the pattern, "Double quote to prevent globing and word splitting".
And issues like Use $(...) notation instead of legacy backticked ....

Solution:
We are putting the variable references in double-quotes.
Also fixed issues like Use $(...) notation instead of legacy backticked ....

Signed-off-by: Gaurav Gaur <gaurav.gaur@seagate.com>
Co-authored-by: Venkatesh Balagani <venkatesh.balagani@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
rkothiya and others added 17 commits August 30, 2022 12:16
This patch fixes some of the codacy warnings.
warning fixed : "Double quote to prevent globing and words splitting".

Signed-off-by: Rinku Kothiya <rinku.kothiya@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
This patch fixes some of the codacy warnings.
warning fixed : "Double quote to prevent globing and words splitting".

Signed-off-by: Rinku Kothiya <rinku.kothiya@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
This patch fixes some of the codacy warnings.
warning fixed : "Double quote to prevent globing and words splitting".

Signed-off-by: Rinku Kothiya <rinku.kothiya@seagate.com>
Co-authored-by: Pradeep Kumbhre <pradeep.kumbhre@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
This patch fixes some of the codacy warnings.
warning fixed : "Double quote to prevent globing and words splitting".

Signed-off-by: Rinku Kothiya <rinku.kothiya@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
…eagate#2004)

* async_disconnecting is used to check if waiting is needed.

* No need to call session_put() in rm_remote_free().
  Because at this moment, maybe some other requests are still in progress.

Signed-off-by: Hua Huang <hua.huang@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
Problem:
Getting following codacy warnings
Useless cat. Consider `cmd < file | ...`

Solution:
Removed unnecessary cat from the command.

Signed-off-by: Rinku Kothiya <rinku.kothiya@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
This patch fixes some of the codacy warnings.
warning fixed : "Double quote to prevent globing and words splitting".

Signed-off-by: alfhad <fahadshah2411@gmail.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
This patch fixes some of the codacy warnings.
warning fixed : "Double quote to prevent globing and words splitting".

Signed-off-by: alfhad <fahadshah2411@gmail.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
…le (Seagate#2059)

Problem:
The addb client logs in rgw container is generating in wrong directory (rgw_debug).
This "rgw_debug" directories meant for rgw crash files only. Our expectation, addb
client logs should be resides in "addb_files-0x7200000000000001:0x13" directory.

Solution:
It's required to set an environment for M0_CLIENT_ADDB_DIR on cortx-rgw-integration
repository in src/rgw/setup/rgw.py at config stage.

In cortx-motr, client_init.c will make use of M0_CLIENT_ADDB_DIR environment variable
to store addb client log.

Signed-off-by: Rahul Kumar <rahul.kumar@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
Problem:
43motr-sync-replication ST fails.
be-ut failure in emap and io-nw-xfer-ut failure in target_ioreq_seg_add fails.

Solution:
43motr-sync-replication fixed for N < K by shifting goff by (K-N)*offset.
be-ut failure in emap is fixed by changing EXTMAP_UT_UNIT_SIZE to 16 from 10 as it should be in range of power 2.
io-nw-xfer-ut is fixed by allocating ti_goff_ivec.

Signed-off-by: Rajat Patil <rajat.r.patil@seagate.com>
Co-authored-by: Vidyadhar Pinglikar <vidyadhar.pinglikar@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
Problem:
With the previous job if one of the test failed then
rest of the jobs were not executed.

Solution:
Running tests individually instead of a group.
Ignoring any failures that occur.

Signed-off-by: Rinku Kothiya <rinku.kothiya@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
…eagate#2105)

Signed-off-by: Mehul Joshi <mehul.joshi@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
Pulled latest code and addressed review comments.

Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
Signed-off-by: Swapnil Chaudhary <swapnil.chaudhary@seagate.com>
@swapnil-seagate swapnil-seagate force-pushed the CORTX-33743_FIX_MEMORY_LEAK branch from 641d64f to 5ab2d68 Compare August 30, 2022 18:17
@cla-bot
Copy link

cla-bot bot commented Aug 30, 2022

Thanks for your contribution!
The CLA bot has flagged your contribution as not having a Contributor License Agreement
in place. Note that this is not needed in the overwhelming majority of instances and this warning will usually be ignored.
The code reviewers will make a determination and may ask you to sign a CLA or may choose to ignore this warning.
More information about this can be found here.

@cla-bot cla-bot bot removed the cla-signed label Aug 30, 2022
@cla-bot
Copy link

cla-bot bot commented Aug 30, 2022

Thanks for your contribution!
The CLA bot has flagged your contribution as not having a Contributor License Agreement
in place. Note that this is not needed in the overwhelming majority of instances and this warning will usually be ignored.
The code reviewers will make a determination and may ask you to sign a CLA or may choose to ignore this warning.
More information about this can be found here.

@swapnil-seagate
Copy link
Contributor Author

Latest valgrind report.
valgrind.txt

@rkothiya
Copy link
Contributor

Jenkins CI Result : Motr#1671

Motr Test Summary

Test ResultCountInfo
❌Failed2
📁

04motr-single-node/49motr-rpc-cancel
01motr-single-node/00userspace-tests

🏁Skipped32
📁

01motr-single-node/28sys-kvs
01motr-single-node/35m0singlenode
01motr-single-node/04initscripts
01motr-single-node/37protocol
02motr-single-node/51kem
02motr-single-node/20rpc-session-cancel
02motr-single-node/10pver-assign
02motr-single-node/21fsync-single-node
02motr-single-node/13dgmode-io
02motr-single-node/14poolmach
02motr-single-node/11m0t1fs
02motr-single-node/26motr-user-kernel-tests
02motr-single-node/08spiel
03motr-single-node/06conf
03motr-single-node/36spare-reservation
04motr-single-node/34sns-repair-1n-1f
04motr-single-node/08spiel-sns-repair-quiesce
04motr-single-node/28sys-kvs-kernel
04motr-single-node/11m0t1fs-rconfc-fail
04motr-single-node/08spiel-sns-repair
04motr-single-node/19sns-repair-abort
04motr-single-node/22sns-repair-ios-fail
05motr-single-node/18sns-repair-quiesce
05motr-single-node/12fwait
05motr-single-node/16sns-repair-multi
05motr-single-node/07mount-fail
05motr-single-node/15sns-repair-single
05motr-single-node/23sns-abort-quiesce
05motr-single-node/17sns-repair-concurrent-io
05motr-single-node/07mount
05motr-single-node/07mount-multiple
05motr-single-node/12fsync

✔️Passed43
📁

01motr-single-node/43m0crate
01motr-single-node/05confgen
01motr-single-node/06hagen
01motr-single-node/52motr-singlenode-sanity
01motr-single-node/01net
01motr-single-node/01kernel-tests
01motr-single-node/03console
01motr-single-node/02rpcping
02motr-single-node/07m0d-fatal
02motr-single-node/67fdmi-plugin-multi-filters
02motr-single-node/53clusterusage-alert
02motr-single-node/41motr-conf-update
03motr-single-node/61sns-repair-motr-1n-1f
03motr-single-node/72spiel-sns-motr-repair-quiesce
03motr-single-node/08spiel-multi-confd
03motr-single-node/69sns-repair-motr-quiesce
03motr-single-node/62sns-repair-motr-mf
03motr-single-node/70sns-failure-after-repair-quiesce
03motr-single-node/63sns-repair-motr-1k-1f
03motr-single-node/60sns-repair-motr-1f
03motr-single-node/66sns-repair-motr-abort-quiesce
03motr-single-node/24motr-dix-repair-lookup-insert-spiel
03motr-single-node/68sns-repair-motr-shutdown
03motr-single-node/64sns-repair-motr-ios-fail
03motr-single-node/71spiel-sns-motr-repair
03motr-single-node/24motr-dix-repair-lookup-insert-m0repair
03motr-single-node/04sss
03motr-single-node/65sns-repair-motr-abort
04motr-single-node/73motr-io-small-disks
04motr-single-node/48motr-raid0-io
04motr-single-node/74motr-di-corruption-detection
04motr-single-node/25m0kv
04motr-single-node/44motr-rm-lock-cc-io
04motr-single-node/45motr-rmw
05motr-single-node/23dix-repair-m0repair
05motr-single-node/43motr-sync-replication
05motr-single-node/42motr-utils
05motr-single-node/45motr-sns-repair-N-1
05motr-single-node/40motr-dgmode
05motr-single-node/23dix-repair-quiesce-m0repair
05motr-single-node/23spiel-dix-repair-quiesce
05motr-single-node/44motr-sns-repair
05motr-single-node/23spiel-dix-repair

Total77🔗

CppCheck Summary

   Cppcheck: No new warnings found 👍

motr/setup.c Outdated Show resolved Hide resolved
@cla-bot
Copy link

cla-bot bot commented Aug 31, 2022

Thanks for your contribution!
The CLA bot has flagged your contribution as not having a Contributor License Agreement
in place. Note that this is not needed in the overwhelming majority of instances and this warning will usually be ignored.
The code reviewers will make a determination and may ask you to sign a CLA or may choose to ignore this warning.
More information about this can be found here.

@rkothiya rkothiya merged commit 9b5d685 into Seagate:main Aug 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.