Make SET_ERRORX() an expression evaluating to the error number
ClosedPublic
Actions

Authored by kib on Fri, Jun 13, 5:36 PM.

Details

Reviewers

Commits

rG4e207e3849d4: exterr: make SET_ERRORX() macros an expression evaluating to the errno
rG947bff6a04ed: vm/vm_mmap.c: simplify code by using the SET_ERRNOX() values

Summary

Suggested by: markj

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kib created this revision.Fri, Jun 13, 5:36 PM

Herald added a subscriber: imp. · View Herald TranscriptFri, Jun 13, 5:36 PM

kib requested review of this revision.Fri, Jun 13, 5:36 PM

Thanks.

sys/sys/exterrvar.h
51 ↗	(On Diff #156984)	Perhaps this should be a proper function which returns `error`? I'm not sure. I wonder if inlining will result in significant .text bloat.

arrowd added a child revision: D50831: fusefs: First take on exterrorizing.Fri, Jun 13, 5:45 PM

kib added inline comments.Fri, Jun 13, 5:48 PM

sys/sys/exterrvar.h
51 ↗	(On Diff #156984)	Wouldn't setting up args for the call consume approximately the same amount of code? I also wanted to make mmsg store a dead store if the BLOAT option is not enabled.

markj added inline comments.Fri, Jun 13, 6:12 PM

sys/sys/exterrvar.h
51 ↗	(On Diff #156984)	Maybe, but I'd expect a function call to be a bit smaller: it will usually be a tail call, and then the inline code doesn't need to fetch curthread. We would need to try it and see.

kib added inline comments.Fri, Jun 13, 6:28 PM

sys/sys/exterrvar.h

51 ↗

(On Diff #156984)

So for existing calls of vm/vm_map.c

Macro
     text     data       bss        dec        hex   filename
5873740   599651   3595968   10069359   0x99a56f   sys/amd64/compile/X/kernel

Function
     text     data       bss        dec        hex   filename
  5872671   599651   3595968   10068290   0x99a142   sys/amd64/compile/X/kernel

% rg SET_ERROR sys/vm sys/kern | wc -l
      25

The difference is 1K for 25 places.

In D50836#1160355, @markj wrote:

Thanks.

Oops, I meant to accept.

sys/sys/exterrvar.h
51 ↗	(On Diff #156984)	Seems worthwhile? In either case it may be worthwhile to annotate the check with __predict_false so that the body of the macro is moved to the end of the function.