Page MenuHomeFreeBSD

geom_part: Dispatch to partitions to create providers and aliases
AcceptedPublic

Authored by cem on Thu, May 21, 3:58 AM.

Details

Summary

This allows partitions to create additional aliases of their own. The
default method implementations preserve the existing behavior.

No functional change.

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 31228
Build 28878: arc lint + arc unit

Event Timeline

cem requested review of this revision.Thu, May 21, 3:58 AM
cem created this revision.
cem updated this revision to Diff 72103.Fri, May 22, 4:50 AM

Kobj dispatch is on first method argument; correct method argument order to put
table first. In retrospect this is obvious but kobj doesn't detect this nicely
at either compile or runtime.

cem added a reviewer: jhb.
jhb added a comment.Fri, May 22, 3:34 PM

Ah, your comment is that effectively "*this" has to be the first argument to kobj methods? Yes, that is true. Not sure how one would detect that at runtime in KOBJ_DISPATCH or the like though. Note that we tend to just aggregate kobj_class_t as the first member of whatever we use as the first argument, no inheritance (since it's C) so I don't think you can do much in the way of compile time checking?

cem added a comment.Fri, May 22, 4:54 PM
In D24938#549502, @jhb wrote:

Ah, your comment is that effectively "*this" has to be the first argument to kobj methods? Yes, that is true.

Right. In retrospect, obvious, but I was perhaps being silly trying to preserve argument order so registers didn't need to shuffle and did not think about where dispatch happened.

Not sure how one would detect that at runtime in KOBJ_DISPATCH or the like though.

I don't think we can generally do it at runtime. We could have some INVARIANTS-only kludge like a sentinel marker that is unlikely to match for non-KOBJs.

Note that we tend to just aggregate kobj_class_t as the first member of whatever we use as the first argument, no inheritance (since it's C) so I don't think you can do much in the way of compile time checking?

Well, if we didn't cast firstvar to kobj_t, the compiler would actually check the ->ops member existed. For some reason we already don't do this for STATICMETHODs, but it isn't clear to me why we do it at all.

And if KOBJOPLOOKUP() was an inline function instead of a macro (which, it can be, aside from OP##_##desc pasting), the type of the ops would be checked as well.

imp accepted this revision.Sat, May 23, 1:17 AM
In D24938#549385, @cem wrote:

Kobj dispatch is on first method argument; correct method argument order to put
table first. In retrospect this is obvious but kobj doesn't detect this nicely
at either compile or runtime.

Correct. If you create a method that doesn't have a kobj as its first argument, you're going to have a bad time.

Though STATIC methods should be fine... but STATIC methods are a bit special in other ways :( I am surprised we don't catch it better, but looking at the casts involved and our standard practice, it looks like it would be hard to improve unless we mandated all the kobj objects have a kobj first named in a particular way, then macros could come to the rescue w/o casts... but that's its own kind of poison.

In any event, these changes look good to my eye.

sys/geom/part/g_part_if.m
82

It's unfortunate these two functions are so close to each other, yet unsharable.

This revision is now accepted and ready to land.Sat, May 23, 1:17 AM
cem added inline comments.Sat, May 23, 3:35 AM
sys/geom/part/g_part_if.m
82

The first N lines could be merged to a small subroutine. Not a big deal to me one way or the other. We save ~3 lines and then add at least as many to make a subroutine.