On 12/27/2013 04:06 AM, Michael Dalton wrote: > On Mon, Dec 23, 2013 at 4:51 AM, Michael S. Tsirkin <mst@xxxxxxxxxx> wrote: >> OK so a high level benchmark shows it's worth it, >> but how well does the logic work? >> I think we should make the buffer size accessible in sysfs >> or debugfs, and look at it, otherwise we don't really know. >> > Exporting the size sounds good to me, it is definitely an > important metric and would give more visibility to the admin. > > Do you have a preference for implementation strategy? I was > thinking just add a DEVICE_ATTR to create a read-only sysfs file, > 'mergeable_rx_buffer_size', and return a space-separated list of the > current buffer size (computed from the average packet size) for each > receive queue. -EINVAL or a similar error could be returned if the > netdev was not configured for mergeable rx buffers. > >> I don't get the real motivation for this. >> >> We have skbs A,B,C sharing a page, with chunk D being unused. >> This randomly charges chunk D to an skb that ended up last >> in the page. >> Correct? >> Why does this make sense? > The intent of this code is to adjust the SKB true size for > the packet. We should completely use each packet buffer except > for the last buffer. For all buffers except the last buffer, it > should be the case that 'len' (bytes received) = buffer size. For > the last buffer, this code adjusts the truesize by comparing the > approximated buffer size with the bytes received into the buffer, > and adding the difference to the SKB truesize if the buffer size > is greater than the number of bytes received. But look like it does not take the 'hole' into account which may cause under estimation of trusize. > > We approximate the buffer size by using the last packet buffer size > from that same page, which as you have correctly noted may be a buffer > that belongs to a different packet on the same virtio-net device. This > buffer size should be very close to the actual buffer size because our > EWMA estimator uses a high weight (so the packet buffer size changes very > slowly) and there are only a handful packets on a page (even order-3). We can make this more accurate by using extra data structure to track the real buf size and using it as token. >> Why head_skb only? Why not full buffer size that comes from host? >> This is simply len. > Sorry, I believe this code fragment should be clearer. Basically, we > have a corner case in that for packets with size <= GOOD_COPY_LEN, there > are no frags because page_to_skb() already unref'd the page and the entire > packet contents are copied to skb->data. In this case, the SKB truesize > is already accurate and should not be updated (and it would be unsafe to > access page->private as page is already unref'd). > > I'll look at the above code again and cleanup (please let me know if you > have a preference) and/or add a comment to clarify. > > Best, > > Mike _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
