On Fri, Jan 20, 2017 at 9:01 AM, Christopher Collins <[email protected]> wrote:
> On Thu, Jan 19, 2017 at 11:57:01PM -0800, Simon Ratner wrote: > > Thanks Chris, > > > > It appears to me that there is questionable benefit to having mbufs sized > > larger than the largest L2CAP fragment size (plus overhead), i.e. the 80 > > bytes that Will mentioned. Is that a reasonable statement, or am I > missing > > something? > > > > For incoming data, you always waste memory with larger mbufs, and for > > outgoing data host will take longer to free the memory (since you can't > > free the payload mbuf until the last fragment, as opposed to freeing > > smaller mbufs as you go), and you don't save on the number of copies in > the > > host. You will save something on mbuf allocations and mbuf header > overhead > > in the app as you are generating the payload, though. > > I agree with your analysis. This is just for BLE data, of course. If > you use msys for other things (e.g., an alternative to malloc for > internal use), that obviously has an affect on the optimum msys buffer > size. > > > When allocating mbufs for the payload, is there something I should do to > > reserve enough leading space for the ACL header to make sure host doesn't > > need to re-allocate it? > > Yes - you should use ble_hs_mbuf_att_pkt() to allocate all your mbufs. > This function ensures the resulting mbuf has sufficient leading space > for: > * ACL data header > * Basic L2CAP header > * Four bytes for an ATT command > And I assume the mbuf passed into gatt read handlers (ctxt->om) has been allocated that way? I only need to worry about the mbufs I allocate myself, such as for notify payload. > The last item (four bytes) is imprecise because different ATT commands > have different payload sizes, and this function doesn't know which > command you'll be sending. Four bytes is the most the host would never > need to prepend to application attribute data. > > We should expose some more functions from > net/nimble/host/src/ble_hs_mbuf.c that give the user more control over > the amount of leading space. This would be useful for when the > application wants to send an ATT command that doesn't need the full four > bytes. > > Just to be clear - if you allocate an mbuf that lacks sufficient leading > space (e.g., via os_msys_get_pkthdr()) and pass it to the host, the host > won't reallocate and copy the entire chain; it will just allocate a > single buffer and prepend it to the chain. This is still wasteful, but > it can be completely avoided by using the ble_hs_mbuf_att_pkt() > function. > > > Also, at least in theory, it sounds like you could size mbufs to match > the > > fragment exactly -- or pre-fragment the mbuf chain as you are generating > > the payload -- and have zero copies in the host. Could be useful in a > > low-memory situation, if the host was smart enough to take advantage of > > that? > > The host isn't smart enough :). The complication arises from the > "os_mbuf_pkthdr" that the leading buffer contains. The presence of this > header causes the first buffer to have less capacity than subsequent > buffers. The fragmentation procedure never frees the chain's leading > buffer. The assumption is that the data is packed in the mbuf chain, > and that the second buffer doesn't have sufficient leading space to > contain the pkthdr. > > A smarter procedure might check how much unused space the second buffer > contains. If there is sufficient room for the pkthdr, the procedure > would move the data to make room for the pkthdr at the front of the > buffer, then copy the pkthdr into the buffer. After this, the leading > buffer could be freed. This might be worth looking into. > > Thanks, > Chris >
