In D55122#1260106, @ziaee wrote:

Should we (then) mention execution order as well?

I don't know, is there anything special about the execution order? I've never had a complicated enough newsyslog setup to run into this problem.

Stupid question: Should we (then) mention execution order as well?

In D54519#1257928, @des wrote:

In D54519#1257714, @michaelo wrote:

Let me rephrase: The version banner should be complete in terms of information regards of the args passed to device_printf. At runtime the passed dev does not know (ahead of time) that the string mrsas/mfi is contained in the version banner. How can this assumption be made at compile time?

Right. Perhaps the kernel should provide a function whose purpose is to print a message to the console on behalf of the driver and make sure that the message is accompanied by the driver name so the user can identify where the message comes from. We could call it something like driver_printf(). Or even better, we can include not only the driver name but the device number as well, and call it device_printf(). Would that satisfy your concern?

Drop redundant terms

In D54519#1257776, @ziaee wrote:

I don't understand. What assumption are we making at compile time?

In D54519#1257713, @des wrote:

In D54519#1257568, @michaelo wrote:

The point I was trying to make is that the dev is not aware at that time how driver banner looks like. From that PoV the banner would look incomplete. WDYT?

I have no idea what any of that means.

In D54519#1257565, @des wrote:

The conceptual change is correct, probes should be silent.

In D54519#1257560, @ziaee wrote:

I do very much approve of the conceptual change, and this code seems clear and good to me, but I am too junior at C programming to be the final word on mechanical changes to drivers. I asked some senior colleagues to take a look.

@des, is this acceptable for you?

@ziaee Can you formally approve then? @emaste, @imp any objections?

In D54619#1256076, @markmi_dsl-only.net wrote:

Well, folks that do not use poudriere(-devel) have been locally putting back ar=${AR} in some places so that their make/Makefile or portmaster style of working can find the right ar. An example is:

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=292544

for which the original editing for the original problem also had an ar=${AR} removal. Some folks then reported build errors tied to ar commands that failed to recognize command line syntax used during the port build --and later reported that putting back the ar=${AR} got things to build again for them.

Has make/Makefile or portmaster based build testing been done for this context?

Note: I use poudriere-devel .

I used the code logic from install(1).

This has been implicitly fixed by 78c933ad0ae2d48b627179958af56aedf623d97a.

@imp A followup review to D54843 to use comma consistently.

In D54843#1255708, @imp wrote:

Comma is preferred.

Please pick for 14 and 13 as well. It should apply cleanly. I am still on 13 and will put in into production immediately.

In D54619#1255317, @siva wrote:

Anything further for this review?

@jrm Please don't forget to cherry-pick.

Fix typo

In D54843#1254166, @ziaee wrote:

This is fine with me, but I would wait for @imp or even anyone someone else from Loader like @kevans or @tsoome. It sounds like he might not want to declare that space works and then they're committed to supporting that, or making things consistent towards that.

@imp as requested...

In D54842#1253816, @ziaee wrote:

Many tools expect git commit titles to be 50 or less characters

Incorporate comments

In D54842#1253714, @emaste wrote:

To expand on my comment - the reason I think it's worth mentioning that is to make it clear we haven't made a choice to ignore this tunable with UEFI, it's that it just doesn't apply with UEFI.

Folks, any opinion?

@mikael Any objections?

@des Please take another look. Do you still want to see the pointer gymnastics?

In D54519#1252062, @ziaee wrote:

I agree with this from a doc principle but I don't know enough from a C standpoint, I'd like @emaste or @imp to check it.

I ran this and it looks good now:

root@deblndw014x:~ # dmesg | grep mrsas0
mrsas0: <AVAGO Thunderbolt SAS Controller> port 0x8000-0x80ff mem 0xdfa60000-0xdfa63fff,0xdfa00000-0xdfa3ffff irq 26 at device 0.0 on pci1
mrsas0: FW now in Ready state
mrsas0: Using MSI-X with 16 number of vectors
mrsas0: FW supports <16> MSIX vector,Online CPU 32 Current MSIX <16>
mrsas0: mrsas_init_adapter: sc->reply_q_depth 0x7e0,sc->request_alloc_sz 0x1f78, sc->reply_alloc_sz 0x3f00,sc->io_frames_alloc_sz 0x3f100
mrsas0: max sge: 0x46, max chain frame size: 0x400, max fw cmd: 0x3ef sc->chain_frames_alloc_sz: 0xfbc00
mrsas0: Issuing IOC INIT command to FW.
mrsas0: IOC INIT response received from FW.

Please add also MFC after one week or so. I don't see it in the summary.

Checked two days in a row. Looks good to me now. Thank you!

LGTM

This looks decent now. I will install this on a few machines and wait until Monday. Thanks!

Rebase against main

What about the documentation part? It does not mention mit-devel at all.

Closing this in favor of @jrm's simpler solution.

I am fine with that. Thank you for the hard work!

Since there was no agreement whether the author's email address should be updated for a newer one, I am going to clone this one.

There is a typo in the commit message: libbbhyve.

There is a typo in the commit message: libbbhyve.

In D54606#1248653, @jrm wrote:

I think the feeling of being ignored is fairly common in the project. There is just more work to be done than time to do it all, and people often expend their energy on things that interest them.

Address style(9) fixes

Apply style(9) fixes

don't fiddle with SUNPATHLEN

In D54606#1248645, @jrm wrote:

@michaelo, I believe I've addressed all your concerns, and I've performed some light testing with manually created logs. Are you able and willing to do any testing with your real logs for the three scripts?

In D54606#1247636, @jrm wrote:

Michael, I took another look at your review, and it dawned on me that we don't need to know which timestamp format is in use. We only need patterns that match either format. Rather than trying to explain things in the other review or take it over, I thought I would first create a separate review for you to consider.
I haven't thoroughly tested this version because I don't have adequate logs.

What do you think?

In D54361#1247638, @jrm wrote:

Michael, take a look at https://reviews.freebsd.org/D54606, which might be a cleaner approach.