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

From: Zoltan Kiss
Date: Mon Dec 16 2013 - 10:21:52 EST


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/