Page MenuHomeFreeBSD

fusefs: First take on exterrorizing
ClosedPublic

Authored by arrowd on Jun 13 2025, 10:41 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Oct 13, 12:23 PM
Unknown Object (File)
Sun, Oct 12, 11:47 PM
Unknown Object (File)
Sun, Oct 12, 11:47 PM
Unknown Object (File)
Sun, Oct 12, 11:47 PM
Unknown Object (File)
Sun, Oct 12, 11:47 PM
Unknown Object (File)
Sun, Oct 12, 11:47 PM
Unknown Object (File)
Sun, Oct 12, 11:47 PM
Unknown Object (File)
Sun, Oct 12, 11:47 PM
Subscribers

Details

Summary

I'm pretty confident with my changes to fuse_device.c, but
fuse_vfsops.c wasn't that obvious.

Test Plan

make buildkernel

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/fs/fuse/fuse_device.c
198
369

The point is to have different error messages for same error codes but different places.

410
411

Continuation line should have indent +4 spaces.

416

Also same note about indent, there and below about [].

asomers requested changes to this revision.Jun 13 2025, 3:55 PM

This is an interesting new feature. But how do we use it? There aren't any examples yet, or any man page. We could easily add test cases for the mount and kqueue errors to the fusefs test suite.

sys/fs/fuse/fuse_device.c
412

"SET_ERROR0" includes the "ktrexterr" dtrace probe. That makes this custom SDT probe redundant. I think you should delete it. Also, delete every other "trace" probe that's adjacent to SET_ERROR0. The fusefs "trace" probes aren't very useful anyway. They're leftover from the original GSoC contribution. I rarely use them.

462
530

It's no longer a "Linux" errno by this point.

580

Because ohead.unique == 0, we aren't looking at an error code. We're looking at a notification code. And it isn't necessarily unknown; it could be a fallthrough from one of the unimplemented codes.

sys/fs/fuse/fuse_vfsops.c
412
This revision now requires changes to proceed.Jun 13 2025, 3:55 PM
sys/fs/fuse/fuse_device.c
412

There is no dtrace probe in SET_ERRORX(). It provides ktrace tracing only, at least ATM.

Removal of the SDT probes should be a separate commit. It might be that Gleb should just not bother to de-dup the string literals, instead copy-pasting them from SDT to SET_ERROR() for this change.

sys/fs/fuse/fuse_device.c
412

What about fbt:kernel:ktrexterr:entry ?

sys/fs/fuse/fuse_device.c
412

I do not object, but it should be made by somebody else, not me. Feel free to add it.

arrowd marked 8 inline comments as done.
  • Address comments
  • Make use of SET_ERROR*() becoming an expression

I'm going to submit another diff that removes trace probes after finishing with exterrorizing.

sys/fs/fuse/fuse_device.c
369

Yes, but I don't understand the code well enough to discern between these 3 ENODEV returns. Maybe @asomers can help to reformulate these exterrors?

On the other hand, this is still more descriptive than just returning ENODEV without any exterror.

There is also D50845 waiting for review.

sys/fs/fuse/fuse_device.c
198

and remove the next line.

arrowd marked an inline comment as done.
  • Address comments
asomers requested changes to this revision.Jun 16 2025, 4:49 PM
asomers added inline comments.
sys/fs/fuse/fuse_vfsops.c
592–595

It looks to me like you rebased your code? There should be another EXTERROR here.

This revision now requires changes to proceed.Jun 16 2025, 4:49 PM
sys/fs/fuse/fuse_vfsops.c
592–595

It looks to me like you rebased your code?

Yes, I did that to make use of EXTERROR.

There should be another EXTERROR here.

Should I deduplicate the error string via static char array here?

sys/fs/fuse/fuse_vfsops.c
592–595

It looks to me like you rebased your code?

Yes, I did that to make use of EXTERROR.

There should be another EXTERROR here.

Should I deduplicate the error string via static char array here?

Yes please. Unlike the dtrace probe, the fuse_warn will not be going away.

  • exterrorize one more errno

This change is ok with me. But I would still like to see a man page and some usage examples from userland.

This revision is now accepted and ready to land.Jun 16 2025, 6:20 PM

What man page? The information on how to read exterrors is located in kdumps manpage.

What man page? The information on how to read exterrors is located in kdumps manpage.

Where? That man page hasn't been changed since 2022. And uexterr_gettext and exterrctl need man pages.

Where? That man page hasn't been changed since 2022.

Ah, sorry, it is in ktrace man page: 96f4be881e8e9e0cb9a6ad2cd9f17f4440983600

And uexterr_gettext and exterrctl need man pages.

Yes, but this is orthogonal to this diff, I think?

What man page? The information on how to read exterrors is located in kdumps manpage.

Where? That man page hasn't been changed since 2022. And uexterr_gettext and exterrctl need man pages.

We did not finished with stabilizing the kernel interface yet, and not much started looking at the proper userspace facilities. It might end up built-in into err()/warn() so uexterr_gettext() would be left as some internal function. I simply did not considered that we are there yet.

For the exterrctl(2), the man page would be eventually added, but with the warning similar to that for thr*(2) and sigfastblock(2) that the interface is for C runtime and should not be used by the app code.

In D50831#1161443, @kib wrote:

We did not finished with stabilizing the kernel interface yet, and not much started looking at the proper userspace facilities. It might end up built-in into err()/warn() so uexterr_gettext() would be left as some internal function. I simply did not considered that we are there yet.

For the exterrctl(2), the man page would be eventually added, but with the warning similar to that for thr*(2) and sigfastblock(2) that the interface is for C runtime and should not be used by the app code.

So is it intended that kdump/ktrace an err/warn will be the primary way to view these messages?

In D50831#1161443, @kib wrote:

We did not finished with stabilizing the kernel interface yet, and not much started looking at the proper userspace facilities. It might end up built-in into err()/warn() so uexterr_gettext() would be left as some internal function. I simply did not considered that we are there yet.

For the exterrctl(2), the man page would be eventually added, but with the warning similar to that for thr*(2) and sigfastblock(2) that the interface is for C runtime and should not be used by the app code.

So is it intended that kdump/ktrace an err/warn will be the primary way to view these messages?

Probably warn/err when I enable them. Ktrace is indeed already there, but it cannot be the primary user interface.
I do not want to do the more intrusive pass on userspace to avoid the needs to iterate, as I have to do for vm/vm_mmap.c in kernel.

This revision was automatically updated to reflect the committed changes.
markj added inline comments.
sys/fs/fuse/fuse_device.c
600

Was the change from EINVAL intentional? It is causing one of the fusefs tests to fail.

sys/fs/fuse/fuse_device.c
600

No, it is a copy-paste mistake. I can push a fix if you approve.

sys/fs/fuse/fuse_device.c
600

You do not need approval. Just commit it.