[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH net-next v2 1/9] xen-netback: Introduce TX grant map definitions



On 13/12/13 19:14, Wei Liu wrote:
+       spin_lock_irqsave(&vif->dealloc_lock, flags);
> >>+  do {
> >>+          pending_idx = ubuf->desc;
> >>+          ubuf = (struct ubuf_info *) ubuf->ctx;
> >>+          index = pending_index(vif->dealloc_prod);
> >>+          vif->dealloc_ring[index] = pending_idx;
> >>+          /* Sync with xenvif_tx_action_dealloc:
> >>+           * insert idx then incr producer.
> >>+           */
> >>+          smp_wmb();
> >>+          vif->dealloc_prod++;
> >>+  } while (ubuf);
> >>+  wake_up(&vif->dealloc_wq);
> >>+  spin_unlock_irqrestore(&vif->dealloc_lock, flags);
[...]
> >
> >>+          smp_rmb();
> >>+
> >>+          while (dc != dp) {
> >>+                  pending_idx =
> >>+                          vif->dealloc_ring[pending_index(dc++)];
> >>+
> >>+                  /* Already unmapped? */
> >>+                  if (vif->grant_tx_handle[pending_idx] ==
> >>+                          NETBACK_INVALID_HANDLE) {
> >>+                          netdev_err(vif->dev,
> >>+                                  "Trying to unmap invalid handle! "
> >>+                                  "pending_idx: %x\n", pending_idx);
> >>+                          continue;
> >>+                  }
> >
> >Should this be BUG_ON? AIUI this kthread should be the only one doing
> >unmap, right?
>The NAPI instance can do it as well if it is a small packet fits
>into PKT_PROT_LEN. But still this scenario shouldn't really happen,
>I was just not sure we have to crash immediately. Maybe handle it as
>a fatal error and destroy the vif?
>
It depends. If this is within the trust boundary, i.e. everything at the
stage should have been sanitized then we should BUG_ON because there's
clearly a bug somewhere in the sanitization process, or in the
interaction of various backend routines.

My understanding is that crashing should be avoided if we can bail out somehow. At this point there is clearly a bug in netback somewhere, something unmapped that page before it should have happened, or at least that array get corrupted somehow. However there is a chance that xenvif_fatal_tx_err() can contain the issue, and the rest of the system can go unaffected.

Zoli

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.