Paths

Table of Contentst

-
lang/
-
Makefile
2/5
rust-bootstrap/
2/5
Makefile
2/5
distinfo
-
pkg-descr
-
rust/
-
Makefile
-
distinfo

lang/rust-bootstrap: Cross-compile Rust bootstraps for all archs
ClosedPublic
Actions

Authored by tobik on Jan 11 2020, 9:09 AM.

Details

Reviewers

pkubaj
jbeich
mikael

Group Reviewers

rust
portmgr

Commits

rP527099: New port: lang/rust-bootstrap

Summary

Goals are:

Putting the cost of building them on the FreeBSD project instead of volunteers
Creating bootstraps for only supported releases instead of using whatever upstream builds them on (currently 10.3-RELEASE)
Having the ability to easily patch them when it becomes necessary

Adding portmgr for approval of flavors.

Test Plan

Does lang/rust build with the new bootstraps?

amd64, i386: yes, tested by @tobik
aarch64: yes, tested by @garga (thanks!)
armv7: yes, tested by @mikael.urankar_gmail.com (thanks!)
armv6: untested
powerpc64 elfv1: yes. tested on ref12-ppc64 by @tobik, also tested by @pkubaj
powerpc64 elfv2: yes, tested by @pkubaj (thanks!)

Diff Detail

Repository

rP FreeBSD ports repository

Lint

No Lint Coverage

Unit

No Test Coverage

Build Status

Buildable 28908
Build 26899: arc lint + arc unit

Event Timeline

tobik created this revision.Jan 11 2020, 9:09 AM

Herald added a subscriber: mat. · View Herald TranscriptJan 11 2020, 9:09 AM

Harbormaster completed remote builds in B28605: Diff 66615.Jan 11 2020, 9:09 AM

Remove empty dir...

Harbormaster completed remote builds in B28606: Diff 66616.Jan 11 2020, 9:09 AM

Drop unused compiler-rt from distinfo

Harbormaster completed remote builds in B28607: Diff 66617.Jan 11 2020, 9:16 AM

Rebase

Harbormaster completed remote builds in B28608: Diff 66619.Jan 11 2020, 9:21 AM

Tested to build on both elfv1 and elfv2.

This revision is now accepted and ready to land.Jan 14 2020, 2:23 PM

They finally fixed their cross-compile issue, I hope it won't break in the future.
It's ok on armv7, I'm afraid I won't have time to test on armv6 or aarch64 (but I see no reason it would break).

tobik edited the test plan for this revision. (Show Details)Jan 16 2020, 9:20 AM

garga added a subscriber: garga.Jan 16 2020, 1:32 PM

I tried to build lang/rust on native aarch64 builder with this patch applied, under a 11.3-STABLE jail and got:

running: /wrkdirs/usr/ports/lang/rust/work/rustc-1.40.0-src/build/aarch64-unknown-freebsd/stage0/bin/cargo build --manifest-path /wrkdirs/usr/ports/lang/rust/work/rustc-1.40.0-src/src/bootstrap/Cargo.toml --verbose --frozen
/wrkdirs/usr/ports/lang/rust/work/rustc-1.40.0-src/build/aarch64-unknown-freebsd/stage0/bin/cargo: Undefined symbol "lstat@FBSD_1.5"
Traceback (most recent call last):
  File "/wrkdirs/usr/ports/lang/rust/work/rustc-1.40.0-src/x.py", line 11, in <module>
    bootstrap.main()
  File "/wrkdirs/usr/ports/lang/rust/work/rustc-1.40.0-src/src/bootstrap/bootstrap.py", line 909, in main
    bootstrap(help_triggered)
  File "/wrkdirs/usr/ports/lang/rust/work/rustc-1.40.0-src/src/bootstrap/bootstrap.py", line 880, in bootstrap
    build.build_bootstrap()
  File "/wrkdirs/usr/ports/lang/rust/work/rustc-1.40.0-src/src/bootstrap/bootstrap.py", line 675, in build_bootstrap
    run(args, env=env, verbose=self.verbose)
  File "/wrkdirs/usr/ports/lang/rust/work/rustc-1.40.0-src/src/bootstrap/bootstrap.py", line 141, in run
    raise RuntimeError(err)
RuntimeError: failed to run: /wrkdirs/usr/ports/lang/rust/work/rustc-1.40.0-src/build/aarch64-unknown-freebsd/stage0/bin/cargo build --manifest-path /wrkdirs/usr/ports/lang/rust/work/rustc-1.40.0-src/src/bootstrap/Cargo.toml --verbose --frozen
*** Error code 1

Would it be possible to have a bootstrap compatible with FreeBSD 11?

In D23133#509306, @garga wrote:

Would it be possible to have a bootstrap compatible with FreeBSD 11?

You'll need to change _RUST_FBSD_VER_aarch64 to 11.3. Be aware that there are bugs in rtld on aarch64 11.3 (not sure if the bug is fixed in 11-stable), you'll need a ld-elf.so.1 from 12.1 or 12.stable (see bug https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=228892 for details)

aarch64: Target 11.3-RELEASE (bootstrap already updated but give it an hour to appear on mirrors)
Add COMPAT_FREEBSD11 canary to lang/rust-bootstrap too

This revision now requires review to proceed.Jan 18 2020, 9:34 AM

Harbormaster completed remote builds in B28753: Diff 66963.Jan 18 2020, 9:34 AM

Rebase

Harbormaster completed remote builds in B28754: Diff 66964.Jan 18 2020, 9:57 AM

@garga So is it ok for you now?

In D23133#511441, @tobik wrote:

@garga So is it ok for you now?

Unfortunately our ThunderX builder is out of service for maintenance and I won't be able to test it until next week. It is still broken for crossbuild, right?

In D23133#511524, @garga wrote:

In D23133#511441, @tobik wrote:

@garga So is it ok for you now?

Unfortunately our ThunderX builder is out of service for maintenance and I won't be able to test it until next week.

Ok. Let me know how it goes.

It is still broken for crossbuild, right?

Yes, unfortunately the build will just hang with qemu-user-static at some point.

tobik mentioned this in rP524013: lang/rust: Apply some of the patches for lang/rust-bootstrap (D23133).Jan 25 2020, 5:41 AM

Rebase

Harbormaster completed remote builds in B28908: Diff 67274.Jan 25 2020, 5:44 AM

...

Harbormaster completed remote builds in B28909: Diff 67276.Jan 25 2020, 5:47 AM

tobik mentioned this in D23385: lang/rust: Update to 1.41.0.Jan 27 2020, 9:27 PM

It fails on armv6, 11.3-RELEASE. I used to build the bootstrap for 11.3, it's 12.1 with this port. I'm not against it but we should restrict lang/rust accordingly. I'd prefer keeping support for 11.3 though.

running: /usr/ports/lang/rust/work/rustc-1.41.0-src/build/armv6-unknown-freebsd/stage0/bin/cargo build --manifest-path /usr/ports/lang/rust/work/rustc-1.41.0-src/src/bootstrap/Cargo.toml --verbose --frozen
/usr/ports/lang/rust/work/rustc-1.41.0-src/build/armv6-unknown-freebsd/stage0/bin/cargo: Undefined symbol "lstat@FBSD_1.5"
/usr/ports/lang/rust/work/rustc-1.41.0-src/build/armv6-unknown-freebsd/stage0/bin/cargo: ELF 32-bit LSB shared object, ARM, EABI5 version 1 (FreeBSD), dynamically linked, interpreter /libexec/ld-elf.so.1, for FreeBSD 12.1, FreeBSD-style, with debug_info, not stripped

Something is odd, it seems the armv6 sysroot is for armv7:

readelf -A /usr/ports/lang/rust-bootstrap/work-armv6/lib/libcrypt.so.5
Attribute Section: aeabi
File Attributes
  Tag_CPU_arch: ARM v7
  Tag_CPU_arch_profile: Application Profile
  Tag_ARM_ISA_use: Yes
  Tag_THUMB_ISA_use: 32-bit Thumb
  Tag_FP_arch: VFPv3
  Tag_Advanced_SIMD_arch: NEONv1
  Tag_CPU_unaligned_access: Not allowed

Yeah, FreeBSD-11.3-RELEASE-arm-armv6.tar.xz and FreeBSD-12.1-RELEASE-arm-armv7.tar.xz were accidentally the same file... I'm rebuilding the armv6 bootstrap now.

Actually build armv6 bootstrap for armv6

Harbormaster completed remote builds in B28996: Diff 67430.Jan 28 2020, 7:48 PM

In D23133#511525, @tobik wrote:

In D23133#511524, @garga wrote:

In D23133#511441, @tobik wrote:

@garga So is it ok for you now?

Unfortunately our ThunderX builder is out of service for maintenance and I won't be able to test it until next week.

Ok. Let me know how it goes.

It built fine! Thanks!

tobik edited the test plan for this revision. (Show Details)Jan 29 2020, 8:25 AM

Rebase and update to 1.41.0

Harbormaster completed remote builds in B29051: Diff 67528.Jan 30 2020, 8:07 PM

Lock lang/rust version to same version as lang/rust-bootstrap

Harbormaster completed remote builds in B29052: Diff 67529.Jan 30 2020, 8:09 PM

Can someone from portmgr approve the flavors? It would be nice if this could be committed before Rust 1.42.0 (2020-03-12).

Ping?

So, this needs lang/rust to build, and, lang/rust needs it to build? Isn't there some sort of circular dependency?

lang/rust-bootstrap/Makefile
81	To be honest, I would rather you used a few `.if` than this hardly readable magic.
95–97	There are no patches, is this needed? Also, could the patch be named just like the flavor, it would be much more simpler.

In D23133#523804, @mat wrote:

So, this needs lang/rust to build, and, lang/rust needs it to build? Isn't there some sort of circular dependency?

Yes, but there is no circular dependency in the ports tree sense. rust-bootstrap creates the bootstraps used for building the next stable release of lang/rust. I then upload the binaries somewhere before the next Rust release, so that lang/rust can fetch and use them. lang/rust will never depend directly on lang/rust-bootstrap.

lang/rust-bootstrap/Makefile
81	Fair enough. Would you also be fine with something like _RUST_LLVM_TARGET= ${_RUST_LLVM_TARGET_${FLAVOR}} _RUST_LLVM_TARGET_armv6= ARM _RUST_LLVM_TARGET_armv7= ARM ...
95–97	There are patches. It reuses the ones from lang/rust.

Make _RUST_LLVM_TARGET more readable

Harbormaster completed remote builds in B29604: Diff 68806.Feb 25 2020, 3:23 PM

Accept the flavors, while I am here.

This revision was not accepted when it landed; it landed in state Needs Review.Feb 25 2020, 6:18 PM

Closed by commit rP527099: New port: lang/rust-bootstrap (authored by tobik). · Explain Why

This revision was automatically updated to reflect the committed changes.

tobik added a commit: rP527099: New port: lang/rust-bootstrap.

Revision Contents
Changeset List

Path

Size

lang/

1 line

223 lines

19 lines

7 lines

rust/

Makefile

6 lines

distinfo

86 lines

Diff 67274

View Options

lang/Makefile

Show First 20 Lines • Show All 335 Lines • ▼ Show 20 Lines	for (n = 0; n < G_N_EVENTREFS; n++) {
break;		break;
}		}
}		}
if (TAILQ_EMPTY(&g_events))		if (TAILQ_EMPTY(&g_events))
wakeup(&g_pending_events);		wakeup(&g_pending_events);
mtx_unlock(&g_eventlock);		mtx_unlock(&g_eventlock);
}		}

static int		struct g_event *
g_post_event_x(g_event_t func, void arg, int flag, int wuflag, struct g_event **epp, va_list ap)		g_alloc_event(int flag)
		jhbUnsubmitted Not Done Inline Actions Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it always, but my other case in `g_vfs_done()` is optional and I will need to be able to free it in `g_vfs_close()` when a g_vfs object is freed without having its underlying storage destroyed. You could also perhaps forward declare 'struct g_event' in geom.h while leaving it opaque so that you could have static type-checking for the new functions (g_alloc_event, g_free_event, g_post_event_storage). jhb: Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
		impAuthorUnsubmitted Done Inline Actions Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it always, but my other case in `g_vfs_done()` is optional and I will need to be able to free it in `g_vfs_close()` when a g_vfs object is freed without having its underlying storage destroyed. geom_disk just posts the event, it doesn't wait for it. As such, it doesn't need to free it. In other waiters, we already expect them to g_free() the memory. This is no different than that. I'm unfamiliar with your needs here, though... Can you explain it more, since I can't puzzle out why you'd need it there... You could also perhaps forward declare 'struct g_event' in geom.h while leaving it opaque so that you could have static type-checking for the new functions (g_alloc_event, g_free_event, g_post_event_storage). That's a good idea. imp: > Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
		jhbUnsubmitted Not Done Inline Actions Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it always, but my other case in `g_vfs_done()` is optional and I will need to be able to free it in `g_vfs_close()` when a g_vfs object is freed without having its underlying storage destroyed. geom_disk just posts the event, it doesn't wait for it. As such, it doesn't need to free it. In other waiters, we already expect them to g_free() the memory. This is no different than that. I'm unfamiliar with your needs here, though... Can you explain it more, since I can't puzzle out why you'd need it there... g_vfs won't always post the event is the problem. The event I'm preallocating is for this code at the end of `g_vfs_done`: if (destroy) g_post_event(g_vfs_destroy, cp, M_WAITOK, NULL); That is conditional as it only kicks in if a disk is destroyed out from under a filesystem. If you just unmount a filesystem while the disk is still present, the event won't be used. For that latter case where the event isn't used, I need a way to free the event in g_vfs_close(). Now, perhaps what I can do instead is allocate the event in g_vfs_orphan(), I'm just not sure if it's save to do M_WAITOK in g_vfs_orphan vs pre-allocating in g_vfs_open() where M_WAITOK is definitely safe. (Though it looks like it might be the case that I can sleep in g_vfs_orphan since it is always called with the topology lock held which I think is an sx lock?) Oh, I think due to the opaque use of void * I was worried you couldn't just use g_free() directly, but I guess it would be ok to use g_free() directly for an event allocated by g_alloc_event(). If you document this in a manpage maybe note that an event allocated by g_alloc_event() can be freed via g_free() if it isn't posted. jhb: > > Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
{		{
struct g_event *ep;		KASSERT(flag == M_WAITOK \|\| flag == M_NOWAIT,
		("Wrong flag to g_alloc_event"));

		return (g_malloc(sizeof(struct g_event), flag \| M_ZERO));
		}

		static void
		g_post_event_ep_va(g_event_t func, void arg, int wuflag,
		struct g_event *ep, va_list ap)
		{
void *p;		void *p;
u_int n;		u_int n;

g_trace(G_T_TOPOLOGY, "g_post_event_x(%p, %p, %d, %d)",
func, arg, flag, wuflag);
KASSERT(wuflag == 0 \|\| wuflag == EV_WAKEUP,
("Wrong wuflag in g_post_event_x(0x%x)", wuflag));
ep = g_malloc(sizeof *ep, flag \| M_ZERO);
if (ep == NULL)
return (ENOMEM);
ep->flag = wuflag;		ep->flag = wuflag;
for (n = 0; n < G_N_EVENTREFS; n++) {		for (n = 0; n < G_N_EVENTREFS; n++) {
p = va_arg(ap, void *);		p = va_arg(ap, void *);
if (p == NULL)		if (p == NULL)
break;		break;
g_trace(G_T_TOPOLOGY, " ref %p", p);		g_trace(G_T_TOPOLOGY, " ref %p", p);
ep->ref[n] = p;		ep->ref[n] = p;
}		}
KASSERT(p == NULL, ("Too many references to event"));		KASSERT(p == NULL, ("Too many references to event"));
ep->func = func;		ep->func = func;
ep->arg = arg;		ep->arg = arg;
mtx_lock(&g_eventlock);		mtx_lock(&g_eventlock);
TSHOLD("GEOM events");		TSHOLD("GEOM events");
TAILQ_INSERT_TAIL(&g_events, ep, events);		TAILQ_INSERT_TAIL(&g_events, ep, events);
mtx_unlock(&g_eventlock);		mtx_unlock(&g_eventlock);
wakeup(&g_wait_event);		wakeup(&g_wait_event);
if (epp != NULL)
*epp = ep;
curthread->td_pflags \|= TDP_GEOM;		curthread->td_pflags \|= TDP_GEOM;
thread_lock(curthread);		thread_lock(curthread);
curthread->td_flags \|= TDF_ASTPENDING;		curthread->td_flags \|= TDF_ASTPENDING;
thread_unlock(curthread);		thread_unlock(curthread);
		}

		void
		g_post_event_ep(g_event_t func, void arg, struct g_event *ep, ...)
		jhbUnsubmitted Not Done Inline Actions Why did you expose `wuflag` in this API? I would perhaps instead just have a new `g_waitfor_event_storage` if someone needs the `EV_WAKEUP` variant so that it mirrors the `g_post_event`/`g_waitfor_event` API. The 'waitfor' variant could be added in the future when it is needed. Actually, we probably won't ever need the 'waitfor' variant as if you can sleep, you can likely likely use `M_WAITOK` to alloc the event in the first place. I'm somewhat surprised 'waitfor' even takes a malloc flag. Indeed, every caller uses `M_WAITOK`. jhb: Why did you expose `wuflag` in this API? I would perhaps instead just have a new…
		impAuthorUnsubmitted Done Inline Actions I'll just remove the wuflag. It was mostly because g_post_event_x took it. But your logic makes sense. imp: I'll just remove the wuflag. It was mostly because g_post_event_x took it. But your logic makes…
		{
		va_list ap;

		va_start(ap, ep);
		g_post_event_ep_va(func, arg, 0, ep, ap);
		va_end(ap);
		}


		static int
		g_post_event_x(g_event_t func, void arg, int flag, int wuflag, struct g_event **epp, va_list ap)
		{
		struct g_event *ep;

		g_trace(G_T_TOPOLOGY, "g_post_event_x(%p, %p, %d, %d)",
		func, arg, flag, wuflag);
		KASSERT(wuflag == 0 \|\| wuflag == EV_WAKEUP,
		("Wrong wuflag in g_post_event_x(0x%x)", wuflag));
		ep = g_alloc_event(flag);
		if (ep == NULL)
		return (ENOMEM);
		if (epp != NULL)
		*epp = ep;
		g_post_event_ep_va(func, arg, wuflag, ep, ap);
return (0);		return (0);
}		}

int		int
g_post_event(g_event_t func, void arg, int flag, ...)		g_post_event(g_event_t func, void arg, int flag, ...)
{		{
va_list ap;		va_list ap;
int i;		int i;
▲ Show 20 Lines • Show All 59 Lines • Show Last 20 Lines

View Options

lang/rust-bootstrap/

Show First 20 Lines • Show All 335 Lines • ▼ Show 20 Lines	for (n = 0; n < G_N_EVENTREFS; n++) {
break;		break;
}		}
}		}
if (TAILQ_EMPTY(&g_events))		if (TAILQ_EMPTY(&g_events))
wakeup(&g_pending_events);		wakeup(&g_pending_events);
mtx_unlock(&g_eventlock);		mtx_unlock(&g_eventlock);
}		}

static int		struct g_event *
g_post_event_x(g_event_t func, void arg, int flag, int wuflag, struct g_event **epp, va_list ap)		g_alloc_event(int flag)
		jhbUnsubmitted Not Done Inline Actions Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it always, but my other case in `g_vfs_done()` is optional and I will need to be able to free it in `g_vfs_close()` when a g_vfs object is freed without having its underlying storage destroyed. You could also perhaps forward declare 'struct g_event' in geom.h while leaving it opaque so that you could have static type-checking for the new functions (g_alloc_event, g_free_event, g_post_event_storage). jhb: Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
		impAuthorUnsubmitted Done Inline Actions Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it always, but my other case in `g_vfs_done()` is optional and I will need to be able to free it in `g_vfs_close()` when a g_vfs object is freed without having its underlying storage destroyed. geom_disk just posts the event, it doesn't wait for it. As such, it doesn't need to free it. In other waiters, we already expect them to g_free() the memory. This is no different than that. I'm unfamiliar with your needs here, though... Can you explain it more, since I can't puzzle out why you'd need it there... You could also perhaps forward declare 'struct g_event' in geom.h while leaving it opaque so that you could have static type-checking for the new functions (g_alloc_event, g_free_event, g_post_event_storage). That's a good idea. imp: > Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
		jhbUnsubmitted Not Done Inline Actions Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it always, but my other case in `g_vfs_done()` is optional and I will need to be able to free it in `g_vfs_close()` when a g_vfs object is freed without having its underlying storage destroyed. geom_disk just posts the event, it doesn't wait for it. As such, it doesn't need to free it. In other waiters, we already expect them to g_free() the memory. This is no different than that. I'm unfamiliar with your needs here, though... Can you explain it more, since I can't puzzle out why you'd need it there... g_vfs won't always post the event is the problem. The event I'm preallocating is for this code at the end of `g_vfs_done`: if (destroy) g_post_event(g_vfs_destroy, cp, M_WAITOK, NULL); That is conditional as it only kicks in if a disk is destroyed out from under a filesystem. If you just unmount a filesystem while the disk is still present, the event won't be used. For that latter case where the event isn't used, I need a way to free the event in g_vfs_close(). Now, perhaps what I can do instead is allocate the event in g_vfs_orphan(), I'm just not sure if it's save to do M_WAITOK in g_vfs_orphan vs pre-allocating in g_vfs_open() where M_WAITOK is definitely safe. (Though it looks like it might be the case that I can sleep in g_vfs_orphan since it is always called with the topology lock held which I think is an sx lock?) Oh, I think due to the opaque use of void * I was worried you couldn't just use g_free() directly, but I guess it would be ok to use g_free() directly for an event allocated by g_alloc_event(). If you document this in a manpage maybe note that an event allocated by g_alloc_event() can be freed via g_free() if it isn't posted. jhb: > > Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
{		{
struct g_event *ep;		KASSERT(flag == M_WAITOK \|\| flag == M_NOWAIT,
		("Wrong flag to g_alloc_event"));

		return (g_malloc(sizeof(struct g_event), flag \| M_ZERO));
		}

		static void
		g_post_event_ep_va(g_event_t func, void arg, int wuflag,
		struct g_event *ep, va_list ap)
		{
void *p;		void *p;
u_int n;		u_int n;

g_trace(G_T_TOPOLOGY, "g_post_event_x(%p, %p, %d, %d)",
func, arg, flag, wuflag);
KASSERT(wuflag == 0 \|\| wuflag == EV_WAKEUP,
("Wrong wuflag in g_post_event_x(0x%x)", wuflag));
ep = g_malloc(sizeof *ep, flag \| M_ZERO);
if (ep == NULL)
return (ENOMEM);
ep->flag = wuflag;		ep->flag = wuflag;
for (n = 0; n < G_N_EVENTREFS; n++) {		for (n = 0; n < G_N_EVENTREFS; n++) {
p = va_arg(ap, void *);		p = va_arg(ap, void *);
if (p == NULL)		if (p == NULL)
break;		break;
g_trace(G_T_TOPOLOGY, " ref %p", p);		g_trace(G_T_TOPOLOGY, " ref %p", p);
ep->ref[n] = p;		ep->ref[n] = p;
}		}
KASSERT(p == NULL, ("Too many references to event"));		KASSERT(p == NULL, ("Too many references to event"));
ep->func = func;		ep->func = func;
ep->arg = arg;		ep->arg = arg;
mtx_lock(&g_eventlock);		mtx_lock(&g_eventlock);
TSHOLD("GEOM events");		TSHOLD("GEOM events");
TAILQ_INSERT_TAIL(&g_events, ep, events);		TAILQ_INSERT_TAIL(&g_events, ep, events);
mtx_unlock(&g_eventlock);		mtx_unlock(&g_eventlock);
wakeup(&g_wait_event);		wakeup(&g_wait_event);
if (epp != NULL)
*epp = ep;
curthread->td_pflags \|= TDP_GEOM;		curthread->td_pflags \|= TDP_GEOM;
thread_lock(curthread);		thread_lock(curthread);
curthread->td_flags \|= TDF_ASTPENDING;		curthread->td_flags \|= TDF_ASTPENDING;
thread_unlock(curthread);		thread_unlock(curthread);
		}

		void
		g_post_event_ep(g_event_t func, void arg, struct g_event *ep, ...)
		jhbUnsubmitted Not Done Inline Actions Why did you expose `wuflag` in this API? I would perhaps instead just have a new `g_waitfor_event_storage` if someone needs the `EV_WAKEUP` variant so that it mirrors the `g_post_event`/`g_waitfor_event` API. The 'waitfor' variant could be added in the future when it is needed. Actually, we probably won't ever need the 'waitfor' variant as if you can sleep, you can likely likely use `M_WAITOK` to alloc the event in the first place. I'm somewhat surprised 'waitfor' even takes a malloc flag. Indeed, every caller uses `M_WAITOK`. jhb: Why did you expose `wuflag` in this API? I would perhaps instead just have a new…
		impAuthorUnsubmitted Done Inline Actions I'll just remove the wuflag. It was mostly because g_post_event_x took it. But your logic makes sense. imp: I'll just remove the wuflag. It was mostly because g_post_event_x took it. But your logic makes…
		{
		va_list ap;

		va_start(ap, ep);
		g_post_event_ep_va(func, arg, 0, ep, ap);
		va_end(ap);
		}


		static int
		g_post_event_x(g_event_t func, void arg, int flag, int wuflag, struct g_event **epp, va_list ap)
		{
		struct g_event *ep;

		g_trace(G_T_TOPOLOGY, "g_post_event_x(%p, %p, %d, %d)",
		func, arg, flag, wuflag);
		KASSERT(wuflag == 0 \|\| wuflag == EV_WAKEUP,
		("Wrong wuflag in g_post_event_x(0x%x)", wuflag));
		ep = g_alloc_event(flag);
		if (ep == NULL)
		return (ENOMEM);
		if (epp != NULL)
		*epp = ep;
		g_post_event_ep_va(func, arg, wuflag, ep, ap);
return (0);		return (0);
}		}

int		int
g_post_event(g_event_t func, void arg, int flag, ...)		g_post_event(g_event_t func, void arg, int flag, ...)
{		{
va_list ap;		va_list ap;
int i;		int i;
▲ Show 20 Lines • Show All 59 Lines • Show Last 20 Lines

View Options

lang/rust-bootstrap/Makefile

Show First 20 Lines • Show All 335 Lines • ▼ Show 20 Lines	for (n = 0; n < G_N_EVENTREFS; n++) {
break;		break;
}		}
}		}
if (TAILQ_EMPTY(&g_events))		if (TAILQ_EMPTY(&g_events))
wakeup(&g_pending_events);		wakeup(&g_pending_events);
mtx_unlock(&g_eventlock);		mtx_unlock(&g_eventlock);
}		}

static int		struct g_event *
g_post_event_x(g_event_t func, void arg, int flag, int wuflag, struct g_event **epp, va_list ap)		g_alloc_event(int flag)
		jhbUnsubmitted Not Done Inline Actions Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it always, but my other case in `g_vfs_done()` is optional and I will need to be able to free it in `g_vfs_close()` when a g_vfs object is freed without having its underlying storage destroyed. You could also perhaps forward declare 'struct g_event' in geom.h while leaving it opaque so that you could have static type-checking for the new functions (g_alloc_event, g_free_event, g_post_event_storage). jhb: Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
		impAuthorUnsubmitted Done Inline Actions Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it always, but my other case in `g_vfs_done()` is optional and I will need to be able to free it in `g_vfs_close()` when a g_vfs object is freed without having its underlying storage destroyed. geom_disk just posts the event, it doesn't wait for it. As such, it doesn't need to free it. In other waiters, we already expect them to g_free() the memory. This is no different than that. I'm unfamiliar with your needs here, though... Can you explain it more, since I can't puzzle out why you'd need it there... You could also perhaps forward declare 'struct g_event' in geom.h while leaving it opaque so that you could have static type-checking for the new functions (g_alloc_event, g_free_event, g_post_event_storage). That's a good idea. imp: > Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
		jhbUnsubmitted Not Done Inline Actions Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it always, but my other case in `g_vfs_done()` is optional and I will need to be able to free it in `g_vfs_close()` when a g_vfs object is freed without having its underlying storage destroyed. geom_disk just posts the event, it doesn't wait for it. As such, it doesn't need to free it. In other waiters, we already expect them to g_free() the memory. This is no different than that. I'm unfamiliar with your needs here, though... Can you explain it more, since I can't puzzle out why you'd need it there... g_vfs won't always post the event is the problem. The event I'm preallocating is for this code at the end of `g_vfs_done`: if (destroy) g_post_event(g_vfs_destroy, cp, M_WAITOK, NULL); That is conditional as it only kicks in if a disk is destroyed out from under a filesystem. If you just unmount a filesystem while the disk is still present, the event won't be used. For that latter case where the event isn't used, I need a way to free the event in g_vfs_close(). Now, perhaps what I can do instead is allocate the event in g_vfs_orphan(), I'm just not sure if it's save to do M_WAITOK in g_vfs_orphan vs pre-allocating in g_vfs_open() where M_WAITOK is definitely safe. (Though it looks like it might be the case that I can sleep in g_vfs_orphan since it is always called with the topology lock held which I think is an sx lock?) Oh, I think due to the opaque use of void * I was worried you couldn't just use g_free() directly, but I guess it would be ok to use g_free() directly for an event allocated by g_alloc_event(). If you document this in a manpage maybe note that an event allocated by g_alloc_event() can be freed via g_free() if it isn't posted. jhb: > > Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
{		{
struct g_event *ep;		KASSERT(flag == M_WAITOK \|\| flag == M_NOWAIT,
		("Wrong flag to g_alloc_event"));

		return (g_malloc(sizeof(struct g_event), flag \| M_ZERO));
		}

		static void
		g_post_event_ep_va(g_event_t func, void arg, int wuflag,
		struct g_event *ep, va_list ap)
		{
void *p;		void *p;
u_int n;		u_int n;

g_trace(G_T_TOPOLOGY, "g_post_event_x(%p, %p, %d, %d)",
func, arg, flag, wuflag);
KASSERT(wuflag == 0 \|\| wuflag == EV_WAKEUP,
("Wrong wuflag in g_post_event_x(0x%x)", wuflag));
ep = g_malloc(sizeof *ep, flag \| M_ZERO);
if (ep == NULL)
return (ENOMEM);
ep->flag = wuflag;		ep->flag = wuflag;
for (n = 0; n < G_N_EVENTREFS; n++) {		for (n = 0; n < G_N_EVENTREFS; n++) {
p = va_arg(ap, void *);		p = va_arg(ap, void *);
if (p == NULL)		if (p == NULL)
break;		break;
g_trace(G_T_TOPOLOGY, " ref %p", p);		g_trace(G_T_TOPOLOGY, " ref %p", p);
ep->ref[n] = p;		ep->ref[n] = p;
}		}
KASSERT(p == NULL, ("Too many references to event"));		KASSERT(p == NULL, ("Too many references to event"));
ep->func = func;		ep->func = func;
ep->arg = arg;		ep->arg = arg;
mtx_lock(&g_eventlock);		mtx_lock(&g_eventlock);
TSHOLD("GEOM events");		TSHOLD("GEOM events");
TAILQ_INSERT_TAIL(&g_events, ep, events);		TAILQ_INSERT_TAIL(&g_events, ep, events);
mtx_unlock(&g_eventlock);		mtx_unlock(&g_eventlock);
wakeup(&g_wait_event);		wakeup(&g_wait_event);
if (epp != NULL)
*epp = ep;
curthread->td_pflags \|= TDP_GEOM;		curthread->td_pflags \|= TDP_GEOM;
thread_lock(curthread);		thread_lock(curthread);
curthread->td_flags \|= TDF_ASTPENDING;		curthread->td_flags \|= TDF_ASTPENDING;
thread_unlock(curthread);		thread_unlock(curthread);
		}

		void
		g_post_event_ep(g_event_t func, void arg, struct g_event *ep, ...)
		jhbUnsubmitted Not Done Inline Actions Why did you expose `wuflag` in this API? I would perhaps instead just have a new `g_waitfor_event_storage` if someone needs the `EV_WAKEUP` variant so that it mirrors the `g_post_event`/`g_waitfor_event` API. The 'waitfor' variant could be added in the future when it is needed. Actually, we probably won't ever need the 'waitfor' variant as if you can sleep, you can likely likely use `M_WAITOK` to alloc the event in the first place. I'm somewhat surprised 'waitfor' even takes a malloc flag. Indeed, every caller uses `M_WAITOK`. jhb: Why did you expose `wuflag` in this API? I would perhaps instead just have a new…
		impAuthorUnsubmitted Done Inline Actions I'll just remove the wuflag. It was mostly because g_post_event_x took it. But your logic makes sense. imp: I'll just remove the wuflag. It was mostly because g_post_event_x took it. But your logic makes…
		{
		va_list ap;

		va_start(ap, ep);
		g_post_event_ep_va(func, arg, 0, ep, ap);
		va_end(ap);
		}


		static int
		g_post_event_x(g_event_t func, void arg, int flag, int wuflag, struct g_event **epp, va_list ap)
		{
		struct g_event *ep;

		g_trace(G_T_TOPOLOGY, "g_post_event_x(%p, %p, %d, %d)",
		func, arg, flag, wuflag);
		KASSERT(wuflag == 0 \|\| wuflag == EV_WAKEUP,
		("Wrong wuflag in g_post_event_x(0x%x)", wuflag));
		ep = g_alloc_event(flag);
		if (ep == NULL)
		return (ENOMEM);
		if (epp != NULL)
		*epp = ep;
		g_post_event_ep_va(func, arg, wuflag, ep, ap);
return (0);		return (0);
}		}

int		int
g_post_event(g_event_t func, void arg, int flag, ...)		g_post_event(g_event_t func, void arg, int flag, ...)
{		{
va_list ap;		va_list ap;
int i;		int i;
▲ Show 20 Lines • Show All 59 Lines • Show Last 20 Lines

View Options

lang/rust-bootstrap/distinfo

Show First 20 Lines • Show All 335 Lines • ▼ Show 20 Lines	for (n = 0; n < G_N_EVENTREFS; n++) {
break;		break;
}		}
}		}
if (TAILQ_EMPTY(&g_events))		if (TAILQ_EMPTY(&g_events))
wakeup(&g_pending_events);		wakeup(&g_pending_events);
mtx_unlock(&g_eventlock);		mtx_unlock(&g_eventlock);
}		}

static int		struct g_event *
g_post_event_x(g_event_t func, void arg, int flag, int wuflag, struct g_event **epp, va_list ap)		g_alloc_event(int flag)
		jhbUnsubmitted Not Done Inline Actions Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it always, but my other case in `g_vfs_done()` is optional and I will need to be able to free it in `g_vfs_close()` when a g_vfs object is freed without having its underlying storage destroyed. You could also perhaps forward declare 'struct g_event' in geom.h while leaving it opaque so that you could have static type-checking for the new functions (g_alloc_event, g_free_event, g_post_event_storage). jhb: Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
		impAuthorUnsubmitted Done Inline Actions Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it always, but my other case in `g_vfs_done()` is optional and I will need to be able to free it in `g_vfs_close()` when a g_vfs object is freed without having its underlying storage destroyed. geom_disk just posts the event, it doesn't wait for it. As such, it doesn't need to free it. In other waiters, we already expect them to g_free() the memory. This is no different than that. I'm unfamiliar with your needs here, though... Can you explain it more, since I can't puzzle out why you'd need it there... You could also perhaps forward declare 'struct g_event' in geom.h while leaving it opaque so that you could have static type-checking for the new functions (g_alloc_event, g_free_event, g_post_event_storage). That's a good idea. imp: > Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
		jhbUnsubmitted Not Done Inline Actions Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it always, but my other case in `g_vfs_done()` is optional and I will need to be able to free it in `g_vfs_close()` when a g_vfs object is freed without having its underlying storage destroyed. geom_disk just posts the event, it doesn't wait for it. As such, it doesn't need to free it. In other waiters, we already expect them to g_free() the memory. This is no different than that. I'm unfamiliar with your needs here, though... Can you explain it more, since I can't puzzle out why you'd need it there... g_vfs won't always post the event is the problem. The event I'm preallocating is for this code at the end of `g_vfs_done`: if (destroy) g_post_event(g_vfs_destroy, cp, M_WAITOK, NULL); That is conditional as it only kicks in if a disk is destroyed out from under a filesystem. If you just unmount a filesystem while the disk is still present, the event won't be used. For that latter case where the event isn't used, I need a way to free the event in g_vfs_close(). Now, perhaps what I can do instead is allocate the event in g_vfs_orphan(), I'm just not sure if it's save to do M_WAITOK in g_vfs_orphan vs pre-allocating in g_vfs_open() where M_WAITOK is definitely safe. (Though it looks like it might be the case that I can sleep in g_vfs_orphan since it is always called with the topology lock held which I think is an sx lock?) Oh, I think due to the opaque use of void * I was worried you couldn't just use g_free() directly, but I guess it would be ok to use g_free() directly for an event allocated by g_alloc_event(). If you document this in a manpage maybe note that an event allocated by g_alloc_event() can be freed via g_free() if it isn't posted. jhb: > > Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
{		{
struct g_event *ep;		KASSERT(flag == M_WAITOK \|\| flag == M_NOWAIT,
		("Wrong flag to g_alloc_event"));

		return (g_malloc(sizeof(struct g_event), flag \| M_ZERO));
		}

		static void
		g_post_event_ep_va(g_event_t func, void arg, int wuflag,
		struct g_event *ep, va_list ap)
		{
void *p;		void *p;
u_int n;		u_int n;

g_trace(G_T_TOPOLOGY, "g_post_event_x(%p, %p, %d, %d)",
func, arg, flag, wuflag);
KASSERT(wuflag == 0 \|\| wuflag == EV_WAKEUP,
("Wrong wuflag in g_post_event_x(0x%x)", wuflag));
ep = g_malloc(sizeof *ep, flag \| M_ZERO);
if (ep == NULL)
return (ENOMEM);
ep->flag = wuflag;		ep->flag = wuflag;
for (n = 0; n < G_N_EVENTREFS; n++) {		for (n = 0; n < G_N_EVENTREFS; n++) {
p = va_arg(ap, void *);		p = va_arg(ap, void *);
if (p == NULL)		if (p == NULL)
break;		break;
g_trace(G_T_TOPOLOGY, " ref %p", p);		g_trace(G_T_TOPOLOGY, " ref %p", p);
ep->ref[n] = p;		ep->ref[n] = p;
}		}
KASSERT(p == NULL, ("Too many references to event"));		KASSERT(p == NULL, ("Too many references to event"));
ep->func = func;		ep->func = func;
ep->arg = arg;		ep->arg = arg;
mtx_lock(&g_eventlock);		mtx_lock(&g_eventlock);
TSHOLD("GEOM events");		TSHOLD("GEOM events");
TAILQ_INSERT_TAIL(&g_events, ep, events);		TAILQ_INSERT_TAIL(&g_events, ep, events);
mtx_unlock(&g_eventlock);		mtx_unlock(&g_eventlock);
wakeup(&g_wait_event);		wakeup(&g_wait_event);
if (epp != NULL)
*epp = ep;
curthread->td_pflags \|= TDP_GEOM;		curthread->td_pflags \|= TDP_GEOM;
thread_lock(curthread);		thread_lock(curthread);
curthread->td_flags \|= TDF_ASTPENDING;		curthread->td_flags \|= TDF_ASTPENDING;
thread_unlock(curthread);		thread_unlock(curthread);
		}

		void
		g_post_event_ep(g_event_t func, void arg, struct g_event *ep, ...)
		jhbUnsubmitted Not Done Inline Actions Why did you expose `wuflag` in this API? I would perhaps instead just have a new `g_waitfor_event_storage` if someone needs the `EV_WAKEUP` variant so that it mirrors the `g_post_event`/`g_waitfor_event` API. The 'waitfor' variant could be added in the future when it is needed. Actually, we probably won't ever need the 'waitfor' variant as if you can sleep, you can likely likely use `M_WAITOK` to alloc the event in the first place. I'm somewhat surprised 'waitfor' even takes a malloc flag. Indeed, every caller uses `M_WAITOK`. jhb: Why did you expose `wuflag` in this API? I would perhaps instead just have a new…
		impAuthorUnsubmitted Done Inline Actions I'll just remove the wuflag. It was mostly because g_post_event_x took it. But your logic makes sense. imp: I'll just remove the wuflag. It was mostly because g_post_event_x took it. But your logic makes…
		{
		va_list ap;

		va_start(ap, ep);
		g_post_event_ep_va(func, arg, 0, ep, ap);
		va_end(ap);
		}


		static int
		g_post_event_x(g_event_t func, void arg, int flag, int wuflag, struct g_event **epp, va_list ap)
		{
		struct g_event *ep;

		g_trace(G_T_TOPOLOGY, "g_post_event_x(%p, %p, %d, %d)",
		func, arg, flag, wuflag);
		KASSERT(wuflag == 0 \|\| wuflag == EV_WAKEUP,
		("Wrong wuflag in g_post_event_x(0x%x)", wuflag));
		ep = g_alloc_event(flag);
		if (ep == NULL)
		return (ENOMEM);
		if (epp != NULL)
		*epp = ep;
		g_post_event_ep_va(func, arg, wuflag, ep, ap);
return (0);		return (0);
}		}

int		int
g_post_event(g_event_t func, void arg, int flag, ...)		g_post_event(g_event_t func, void arg, int flag, ...)
{		{
va_list ap;		va_list ap;
int i;		int i;
▲ Show 20 Lines • Show All 59 Lines • Show Last 20 Lines