don't send the message if there was an issue in constructing it. Also allocate the fixed buffer with M_NOWAIT. It was overlooked in the various code movements during review.

[imp]

Summary

Noticed by: Shawn Webb

I think that kib@ wanted something more like this when things were locked. They are no longer locked, but I think this is the right thing to do...

[kib]

This makes the mechanism even less useful.

I made an (ignored) note in the previous review that unmount options can be changed by unmount, so they are not that interesting.

Options for mounted filesystem can be obtained from the fs itself using statfs(2) or like. It is more important for consumer to be notified that a mount was created or destroyed, than to provide a lot of secondary data. I would suggest to send some simpler message if large buffer cannot be allocated, e.g. fall back to small alloca().

[imp]

In D26122#579967, @kib wrote:

This makes the mechanism even less useful.

Yea, good point...

I made an (ignored) note in the previous review that unmount options can be changed by unmount, so they are not that interesting.

Yea, I included it to communicate forced vs non forced. Perhaps we should do that in other ways? Now I understand it better, I'm back with you...

Options for mounted filesystem can be obtained from the fs itself using statfs(2) or like. It is more important for consumer to be notified that a mount was created or destroyed, than to provide a lot of secondary data. I would suggest to send some simpler message if large buffer cannot be allocated, e.g. fall back to small alloca().

That's an interesting approach... I'll noodle through that and post a new version.... ah, the only question I have is how big is too big? If we can do 1k on the stack, maybe we should just do that all the time... if not, what's a good limit?

[cem]

In D26122#579979, @imp wrote:

That's an interesting approach... I'll noodle through that and post a new version.... ah, the only question I have is how big is too big? If we can do 1k on the stack, maybe we should just do that all the time... if not, what's a good limit?

We can probe remaining stack with GET_STACK_USAGE()... I don't know if/how much we need to leave for interrupts, and we need to leave a few bytes for whatever devctl_notify() does, but that at least gives us a heuristic for "is a large stack buffer reasonable?"

[kib]

In D26122#579979, @imp wrote:

In D26122#579967, @kib wrote:

This makes the mechanism even less useful.

Yea, good point...

I made an (ignored) note in the previous review that unmount options can be changed by unmount, so they are not that interesting.

Yea, I included it to communicate forced vs non forced. Perhaps we should do that in other ways? Now I understand it better, I'm back with you...

But forced unmount is passed as MNTK_UNMOUNTF, it is in mnt_kern_flags. MNT_FORCE is for forced rw mounts of dirty fs, and for forced remounts rw->ro. Yes, it very confusing.

Options for mounted filesystem can be obtained from the fs itself using statfs(2) or like. It is more important for consumer to be notified that a mount was created or destroyed, than to provide a lot of secondary data. I would suggest to send some simpler message if large buffer cannot be allocated, e.g. fall back to small alloca().

That's an interesting approach... I'll noodle through that and post a new version.... ah, the only question I have is how big is too big? If we can do 1k on the stack, maybe we should just do that all the time... if not, what's a good limit?

Assume that you have 256 bytes, this is sane value for all arches and in fact should be enough to pass fsid. 1k is not reasonable IMO.

[imp]

In D26122#580009, @cem wrote:

In D26122#579979, @imp wrote:

That's an interesting approach... I'll noodle through that and post a new version.... ah, the only question I have is how big is too big? If we can do 1k on the stack, maybe we should just do that all the time... if not, what's a good limit?

We can probe remaining stack with GET_STACK_USAGE()... I don't know if/how much we need to leave for interrupts, and we need to leave a few bytes for whatever devctl_notify() does, but that at least gives us a heuristic for "is a large stack buffer reasonable?"

The real answer is to pre-allocate. I have a better patch that does that in the works I'll post in a bit.