Skip to content

Conversation

@PaulZ-98
Copy link
Contributor

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build was run locally and any changes were pushed
  • Lint has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

There is no whatis command in sdb.

Issue Number: 119

What is the new behavior?

sdb> whatis 0xfffffe06596e21b8
0xfffffe06596e21b8 is allocated from dmu_buf_impl_t
sdb> dbuf |head 1 |deref |member db_buf |whatis
0xffff8e804368ac80 is allocated from arc_buf_t

Does this introduce a breaking change?

  • Yes
  • No

Other information

@codecov-io
Copy link

codecov-io commented Mar 15, 2021

Codecov Report

Merging #267 (899e486) into master (db1b967) will increase coverage by 0.13%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #267      +/-   ##
==========================================
+ Coverage   87.51%   87.65%   +0.13%     
==========================================
  Files          62       63       +1     
  Lines        2523     2551      +28     
==========================================
+ Hits         2208     2236      +28     
  Misses        315      315              
Impacted Files Coverage Δ
sdb/commands/linux/whatis.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db1b967...899e486. Read the comment docs.

@PaulZ-98 PaulZ-98 force-pushed the sdb_whatis branch 3 times, most recently from fce08f9 to 460170a Compare March 15, 2021 21:28
@PaulZ-98
Copy link
Contributor Author

Looks like the CI is just waiting. I had a good run, made a few changes and re-pushed a few consecutive times, and it went into the waiting state. Can I trigger it to re-run? I already tried a trivial commit/push to change the commit hash.

@ahrens
Copy link
Contributor

ahrens commented Mar 16, 2021

@PaulZ-98 In my experience, force-pushing a different commit (e.g. with the timestamp changed) has kicked off a new test run. So if that isn't working I'm not sure what to do. @sdimitro can probably help when he gets back from vacation later this week.

Copy link
Contributor

@ahrens ahrens left a comment

Choose a reason for hiding this comment

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

Nice functionality, thanks for contributing this!

@PaulZ-98 PaulZ-98 force-pushed the sdb_whatis branch 5 times, most recently from ef08683 to 18a7787 Compare March 17, 2021 13:56
@PaulZ-98
Copy link
Contributor Author

drgn has modified its required version of libelf to 0.176. It looks like Ubuntu bionic only goes up to 0.170.

@PaulZ-98 PaulZ-98 force-pushed the sdb_whatis branch 6 times, most recently from a76d632 to 3163e75 Compare March 17, 2021 15:58
@PaulZ-98
Copy link
Contributor Author

I edited install-drgn.sh to work around the libelf version problem (git checkout older drgn). But that's not likely to be the ultimate solution.

Copy link
Contributor

@sdimitro sdimitro left a comment

Choose a reason for hiding this comment

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

This is great and thank you for working on this!

Have you given any thoughts on how we can also check if the address provided is part of an independent zfs spl-cache? No worries if you haven't we can still go on with just linux-owned cache files. I was just curious.

Comment on lines 28 to 36
def lookup_in_kmem(obj: drgn.Object) -> None:
pfn = mm.virt_to_pfn(sdb.get_prog(), obj)
page = mm.pfn_to_page(pfn)
try:
cache = page.slab_cache
cache_nm = cache.name.string_().decode('utf-8')
print(f"{hex(int(obj))} is allocated from {cache_nm}")
except drgn.FaultError:
print(f"{hex(int(obj))} does not map to a kmem_cache")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should do a couple of changes to this helper if we are to truly make it re-usable:
[1] Move it to sdb/commands/linux/internal/slub_helpers.py
[2] substitute the mm alias with the conventions of the above helper file (e.g. import virt_to_pfn and pfn_to_page at the top of that file and use them without mm).
[3] Change the functions signature:

def lookup_cache_by_address(obj: drgn.Object) -> Optional[drgn.Object]:

[4] Move the printing to the sdb command itself and have that helper method return the cache as a drgn.Object. If we get a FaultError because the page is not part of the cache we return None and the caller/command needs to handle it.

@PaulZ-98
Copy link
Contributor Author

PaulZ-98 commented Mar 18, 2021

This is great and thank you for working on this!

Have you given any thoughts on how we can also check if the address provided is part of an independent zfs spl-cache? No worries if you haven't we can still go on with just linux-owned cache files. I was just curious.

I had started down the road of building up a list of all the caches with their starting address and size, to see if the requested address fell within one of the ranges, and was thus from that kmem_cache. I wonder if there is a better way to see if the address is in a specific zfs spl-cache instead of a linux cache.

sudo apt install bison flex libelf-dev libdw-dev libomp5 libomp-dev

git clone https://github.com/osandov/drgn.git

Copy link
Contributor

Choose a reason for hiding this comment

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

Unneeded whitespace change

@sdimitro sdimitro merged commit 39ec94f into delphix:master Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants