On 10/29/2015 07:41 PM, Lan Tianyu wrote:
On 2015年10月30日 00:17, Alexander Duyck wrote:
On 10/29/2015 01:33 AM, Lan Tianyu wrote:
On 2015年10月29日 14:58, Alexander Duyck wrote:
Your code was having to do a bunch of shuffling in order to get things
set up so that you could bring the interface back up. I would argue
that it may actually be faster at least on the bring-up to just drop the
old rings and start over since it greatly reduced the complexity and the
amount of device related data that has to be moved.
If give up the old ring after migration and keep DMA running before
stopping VCPU, it seems we don't need to track Tx/Rx descriptor ring and
just make sure that all Rx buffers delivered to stack has been migrated.
1) Dummy write Rx buffer before checking Rx descriptor to ensure packet
migrated first.
Don't dummy write the Rx descriptor. You should only really need to
dummy write the Rx buffer and you would do so after checking the
descriptor, not before. Otherwise you risk corrupting the Rx buffer
because it is possible for you to read the Rx buffer, DMA occurs, and
then you write back the Rx buffer and now you have corrupted the memory.
2) Make a copy of Rx descriptor and then use the copied data to check
buffer status. Not use the original descriptor because it won't be
migrated and migration may happen between two access of the Rx
descriptor.
Do not just blindly copy the Rx descriptor ring. That is a recipe for
disaster. The problem is DMA has to happen in a very specific order for
things to function correctly. The Rx buffer has to be written and then
the Rx descriptor. The problem is you will end up getting a read-ahead
on the Rx descriptor ring regardless of which order you dirty things in.
Sorry, I didn't say clearly.
I meant to copy one Rx descriptor when receive rx irq and handle Rx ring.
No, I understood what you are saying. My explanation was that it will
not work.
Current code in the ixgbevf_clean_rx_irq() checks status of the Rx
descriptor whether its Rx buffer has been populated data and then read
the packet length from Rx descriptor to handle the Rx buffer.
That part you have correct. However there are very explicit rules about
the ordering of the reads.
My idea is to do the following three steps when receive Rx buffer in the
ixgbevf_clean_rx_irq().
(1) dummy write the Rx buffer first,
You cannot dummy write the Rx buffer without first being given ownership
of it. In the driver this is handled in two phases. First we have to
read the DD bit to see if it is set. If it is we can take ownership of
the buffer. Second we have to either do a dma_sync_range_for_cpu or
dma_unmap_page call so that we can guarantee the data has been moved to
the buffer by the DMA API and that it knows it should no longer be
accessing it.
(2) make a copy of its Rx descriptor
This is not advisable. Unless you can guarantee you are going to only
read the descriptor after the DD bit is set you cannot guarantee that
you won't race with device DMA. The problem is you could have the
migration occur right in the middle of (2). If that occurs then you
will have valid status bits, but the rest of the descriptor would be
invalid data.
(3) Check the buffer status and get length from the copy.
I believe this is the assumption that is leading you down the wrong
path. You would have to read the status before you could do the copy.
You cannot do it after.
Migration may happen every time.
Happen between (1) and (2). If the Rx buffer has been populated data, VF
driver will not know that on the new machine because the Rx descriptor
isn't migrated. But it's still safe.
The part I think you are not getting is that DMA can occur between (1)
and (2). So if for example you were doing your dummy write while DMA
was occurring you pull in your value, DMA occurs, you write your value
and now you have corrupted an Rx frame by writing stale data back into it.
Happen between (2) and (3). The copy will be migrated to new machine
and Rx buffer is migrated firstly. If there is data in the Rx buffer,
VF driver still can handle the buffer without migrating Rx descriptor.
The next buffers will be ignored since we don't migrate Rx descriptor
for them. Their status will be not completed on the new machine.
You have kind of lost me on this part. Why do you believe there
statuses will not be completed? How are you going to prevent the Rx
descriptor ring from being migrated as it will be a dirty page by the
virtue of the fact that it is a bidirectional DMA mapping where the Rx
path provides new buffers and writes those addresses in while the device
is writing back the status bits and length back. This is kind of what I
was getting at. The Rx descriptor ring will show up as one of the
dirtiest spots on the driver since it is constantly being overwritten by
the CPU in ixgbevf_alloc_rx_buffers.
Anyway we are kind of getting side tracked and I really think the
solution you have proposed is kind of a dead-end.
What we have to do is come up with a solution that can deal with the
fact that you are racing against two different entities. You have to
avoid racing with the device, while at the same time you have to avoid
racing with the dirty page migration code. There are essentially 2
problems you have to solve.
1. Rx pages handed off to the stack must be marked as dirty. For now
your code seemed to address this via this snippet below from patch 12/12:
@@ -946,15 +949,17 @@ static struct sk_buff *ixgbevf_fetch_rx_buffer(struct ixgbevf_ring *rx_ring,
{
struct ixgbevf_rx_buffer *rx_buffer;
struct page *page;
+ u8 *page_addr;
rx_buffer = &rx_ring->rx_buffer_info[rx_ring->next_to_clean];
page = rx_buffer->page;
prefetchw(page);
- if (likely(!skb)) {
- void *page_addr = page_address(page) +
- rx_buffer->page_offset;
+ /* Mark page dirty */
+ page_addr = page_address(page) + rx_buffer->page_offset;
+ *page_addr = *page_addr;
+ if (likely(!skb)) {
/* prefetch first cache line of first page */
prefetch(page_addr);
#if L1_CACHE_BYTES < 128
It will work for now as a proof of concept, but I really would prefer to
see a solution that is driver agnostic. Maybe something that could take
care of it in the DMA API. For example if you were to use
"swiotlb=force" in the guest this code wouldn't even be necessary since
that forces bounce buffers which would mean your DMA mappings are dirty
pages anyway.
2. How to deal with a device that might be in the middle of an
interrupt routine when you decide to migrate. This is the bit I think
you might be focusing on a bit too much, and the current solutions you
have proposed will result in Rx data corruption in the generic case even
without migration. There are essentially 2 possible solutions that you
could explore.
2a. Have a VF device that is aware something is taking place and have
it yield via something like a PCI hot-plug pause request. I don't know
if the Linux kernel supports something like that now since pause support
in the OS is optional in the PCI hot-plug specification, but essentially
it would be a request to do a PM suspend. You would issue a hot-plug
pause and know when it is completed by the fact that the PCI Bus Master
bit is cleared in the VF. Then you complete the migration and in the
new guest you could issue a hot-plug event to restart operation.
2b. Come up with some sort of pseudo IOMMU interface the VF has to use
to map DMA, and provide an interface to quiesce the devices attached to
the VM so that DMA can no longer occur. Once you have disabled bus
mastering on the VF you could then go through and migrate all DMA mapped
pages. As far as resuming on the other side you would somehow need to
poke the VF to get it to realize the rings are no longer initialized and
the mailbox is out-of-sync. Once that happens the VF could reset and
resume operation.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
