Page MenuHomeFreeBSD

ext2fs: Dtrace support.
ClosedPublic

Authored by fsu on Apr 8 2019, 5:19 PM.

Details

Summary

Which logs were replaced by dtrace-probes:

  1. Misc printf's under DEBUG macro in the blocks allocation path.
  2. Different on-disk structures validation errors, now the filesystem will silently return EIO's.
  3. Misc checksum errors, same as above.

The only debug macro, which was leaved is EXT2FS_PRINT_EXTENTS.
It is impossible to replace it by dtrace-probes, because the additional logic is required to walk thru file extents.

The user still be able to see mount errors in the dmesg in case of:

  1. Filesystem features incompatibility.
  2. Superblock checksum error.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

fsu created this revision.Apr 8 2019, 5:19 PM
fsu retitled this revision from ext2fs: Drace support. to ext2fs: Dtrace support..Apr 8 2019, 5:20 PM

Very cool (but I am biased since I suggested the fusefs-dtrace changes as a starting point)!

Add as reviewers more objective DTrace experts and Kirk FYI as UFS is likely to follow in a near future :).

pfg added inline comments.Apr 8 2019, 7:27 PM
sys/fs/ext2fs/ext2_alloc.c
66

I see these ", ," a lot. Is the empty parameter fine or perhaps we should have some form of NULL there?

fsu added inline comments.Apr 9 2019, 7:39 AM
sys/fs/ext2fs/ext2_alloc.c
66

Nope, we should not. The argument could be empty.
See SDT_PROBE_DEFINE2 definition in the sdt.h.
BTW, this naming with 'trace' was borrowed from fusefs.

pfg accepted this revision.Apr 10 2019, 4:06 PM

I still hope to hear from the DTrace experts, but I like this very much. Thanks!

This revision is now accepted and ready to land.Apr 10 2019, 4:06 PM
fsu added a comment.Apr 10 2019, 6:12 PM
In D19848#426730, @pfg wrote:

I still hope to hear from the DTrace experts, but I like this very much. Thanks!

Ok, let's wait some time before landing.

gnn added a comment.Apr 11 2019, 2:26 PM

Some general comments.

  1. Can you move all the DEFINE statements to the top of the file rather than interspresing them throughout? I know you're defining them near use but the general way we have coded these is to put the DECLARES and DEFINES at the top of the files.
  1. The name "trace" is going to be a bit confusing to users as they already think they're tracing the system. You could drop the "trace" altogether and see if that makes sense.

Otherwise this looks like a great addition, and thank you for doing it.

fsu added a comment.Apr 12 2019, 7:03 AM
In D19848#426959, @gnn wrote:

Some general comments.

  1. Can you move all the DEFINE statements to the top of the file rather than interspresing them throughout? I know you're defining them near use but the general way we have coded these is to put the DECLARES and DEFINES at the top of the files.

Ok, I will add this change before landing.

  1. The name "trace" is going to be a bit confusing to users as they already think they're tracing the system. You could drop the "trace" altogether and see if that makes sense.

I prefer to leave it as is to keep consistency with fusefs dtrace probes naming. See: https://reviews.freebsd.org/D19667

Otherwise this looks like a great addition, and thank you for doing it.

gnn accepted this revision.Apr 13 2019, 11:46 PM

OK, I'm happy to work out this "trace" thing later.

Please commit.

This revision was automatically updated to reflect the committed changes.