User Details
- User Since
- Mar 11 2014, 8:46 PM (554 w, 2 d)
Yesterday
@kp are you ok with this change? I think m_unshare can still be resolved orthogonally from the rest of this series.
Wed, Oct 23
Hmmm, if the logic is the same for all IRQ resources ever allocated on intrng architectures, then doing it in the wrapper might indeed be simplest.
@mav does this version look ok? It still works for me with the basic 'newfs -E' test in a VM.
Presumably the gpiobus driver needs to call into subr_intr.c for those platforms then? (And that needs to happen in gpiobus.c, this is not ACPI specific)
So the acpi bus will need some way to recognize special DMAR devices (e.g. a custom ivar) in acpi_get_domain(), and then for DMAR devices it needs to call some other helper routine instead of acpi_parse_pxm() that looks up the domain.
Tue, Oct 22
Yes, the acpi_get_domain() would have to recognize dmar devices and do special handling. Today it just handles _PXM methods on Device objects, and if a device doesn't have a _PXM method it just punts. Presumably the DMAR devices are not Device nodes in the ACPI namespace and thus don't have a _PXM method?
Eh, this doesn't seem correct on the surface. How are the resources being allocated? If they are being allocated via gpiobus_alloc_resource(), then it is wrong to pass those resources up to some random other device driver in the tree and ask it to activate a resource it didn't create and doesn't know how to manage. Do you have more details on "that wasn't working"? Do you get a panic?
Mon, Oct 21
Correct synchronous command completion handling
There are also various races (I think) with the CIFS being changed by the guest while a request is in flight. We really should be caching the CIFS for the duration of a command. The issue there though is that CIFS is variable-sized. :( We could at least cache the common header though I think which would probably handle all of the races I can see.
This fixes a regression from the previous fix. With current main if you just boot a VM with an AHCI attached disk backed by a zvol (so supports TRIM) and do newfs -E /dev/ada0 the guest FreeBSD kernel hangs in a loop of AHCI timeouts as mav@ worried in the previous review. I hadn't expected that the previous review was actually broken, but my guess is the added done >= sizeof(buf) - 8 check was wrong. It probably should have been '>' instead as if you get a full 512 byte block, it will break from the loop before the last valid entry and never send a reply leading to the hang.
Historically I had wanted the 'maxcpus' tunable to be one of the possible settings at creation time (and an nvlist is how I would have passed in such settings). However, I think I've mostly made that requirement OBE in practice.
This looks ok to me, though I'm not intimately familiar with all of iflib.
I'm not opposed to that necessarily. We discussed this a bit more in the review at D46948 if you'd like to chime in there.
Fri, Oct 18
Thu, Oct 17
Wed, Oct 16
I've found this useful as you can now see which drivers fail to set a description (among other things). :)
I don't think the parent can interfere here as DEVICE_ATTACH calls the child method directly without giving the parent driver a chance to interpose.
Humm, I posit that pf(4) will never need to look into the content of unmapped mbufs. They will always be "plain" packet data (either data from a call to write(2) or sendfile(2)) and never headers inserted by any protocol layer. You can also get them for things like iSCSI and NVMe PDUs, but again only for the data portion, not even for PDU headers (which are in plain mbufs). The reason being that protocol headers have to be constructed, and the code that constructs the headers is going to want a mapped mbuf that it can write into. It could be that what we say here is that m_unshare() does not un-share unmapped mbufs but instead just bumps the refcount on the backing mbuf. We could certainly add a new helper of sorts that is something like m_mapped(m, off, len) that pf(4) could use before calls to m_pullup(). However, it is also true that nothing should be reading the unencrypted data in NIC TLS mbufs (where the NIC does the encryption so the data stays unencrypted all the way down the stack).
@olce Rock Ridge extensions support extended modes like sticky bits on ISO images. msdosfs has no way to store such bits and the permissions are all synthetic for msdosfs.
Tue, Oct 15
Adjust ()s to quiet warnings
So would ya'll rather me fix m_unshare()? It's clearly not being used today on unmapped mbufs today as it would be panicking when it used the pointers returned from mtod(), so I wouldn't be able to test it.
Ping
Mon, Oct 14
I don't think we need the taskqueue. It's probably just a design copied from the Intel drivers, and I don't think it makes much sense for those either. The other thing that can be nice to do though when making this change is to instead build a temporary list of packets linked via m_nextpkt (mbufq works for this) and pass an entire batch to if_input. This lets you avoid dropping the lock as often.
Fri, Oct 4
Thu, Oct 3
Is the issue you mentioned previously that you are worried that some drivers might not support concurrent requests on a session? I can't recall if all drivers are ready for this. ccr(4) is at least, though it will perform better if you use separate sessions (Chelsio NICs have multiple crypto engines and each session is tied to a specific engine).
Wed, Oct 2
Ping @asomers
Ok, I'll drop it to 32 for commit then.
I can fix the type mismatch during commit. I have not looked to see if other stacks are affected.
Mon, Sep 30
Switch to old_len
You might try doing this again after my other series lands. In this case though I think you'd need M_WRITABLE_EXTPG.
Looks kind of similar. I have it as a downstream diff in CheriBSD as otherwise it don't compile at all. (CheriBSD doesn't let you mix and match user vs kernel pointers willy-nilly as they are different types.) I guess I'll have to deal with the merge conflict when merging your other change.
This is fine for now. I do think long term we want to figure out the minimum set of registers to preserve for each arch and zero the rest.