On Thu, Dec 26, 2013 at 12:06:23PM -0800, 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. Not too important really. Maybe an attribute per queue? Might be easier to parse and does not need tricky formatting. > > 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. > > 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). I'll need to ponder it, don't understand it exactly. > > 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
