Skip to content

osd: initialise m_interval_start#38959

Merged
neha-ojha merged 1 commit intoceph:masterfrom
badone:wip-valgrind-tracker-48918
Jan 19, 2021
Merged

osd: initialise m_interval_start#38959
neha-ojha merged 1 commit intoceph:masterfrom
badone:wip-valgrind-tracker-48918

Conversation

@badone
Copy link
Contributor

@badone badone commented Jan 19, 2021

Fixes: https://tracker.ceph.com/issues/48918

Signed-off-by: Brad Hubbard bhubbard@redhat.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@badone
osd: initialise m_interval_start
45f36bb 
Fixes: https://tracker.ceph.com/issues/48918

Signed-off-by: Brad Hubbard <bhubbard@redhat.com>
@github-actions github-actions bot added the core label Jan 19, 2021
@tchaikov tchaikov requested a review from ronen-fr January 19, 2021 08:24
ronen-fr
ronen-fr approved these changes Jan 19, 2021
View reviewed changes
Copy link
Contributor

@ronen-fr ronen-fr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it is always initialized when scrub starts (on on_init() for a primary, and on the Op for the replica), I didn't think we should add the extra initialization. But no harm in doing that.

@tchaikov
Copy link
Contributor

tchaikov commented Jan 19, 2021

As it is always initialized when scrub starts (on on_init() for a primary, and on the Op for the replica), I didn't think we should add the extra initialization. But no harm in doing that.

i disagree. it's to silence the warning from valgrind. without this change, we could read from an uninitialized memory when calling PgScrubber::dump(). please see /a/nojha-2021-01-18_15:39:11-rados-wip-ifed-fix-48776-distro-basic-smithi/5799399/remote/smithi194/log/valgrind/osd.1.log.gz for the stacktrace.

@ronen-fr
Copy link
Contributor

ronen-fr commented Jan 19, 2021

As it is always initialized when scrub starts (on on_init() for a primary, and on the Op for the replica), I didn't think we should add the extra initialization. But no harm in doing that.

i disagree. it's to silence the warning from valgrind. without this change, we could read from an uninitialized memory when calling PgScrubber::dump(). please see /a/nojha-2021-01-18_15:39:11-rados-wip-ifed-fix-48776-distro-basic-smithi/5799399/remote/smithi194/log/valgrind/osd.1.log.gz for the stacktrace.

I stand corrected. Thanks.

@neha-ojha
Copy link
Member

neha-ojha commented Jan 19, 2021

https://pulpito.ceph.com/nojha-2021-01-19_21:55:26-rados:verify-wip-badone-testing-distro-basic-smithi/

@neha-ojha neha-ojha merged commit 0c3433d into ceph:master Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tchaikov tchaikov tchaikov approved these changes

@neha-ojha neha-ojha neha-ojha approved these changes

@ronen-fr ronen-fr ronen-fr approved these changes

Assignees

No one assigned

Labels

bug-fix core needs-qa

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@badone @tchaikov @ronen-fr @neha-ojha