[lkml]   [2011]   [Dec]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH v3 2/3] hvc_init(): Enforce one-time initialization.
    On (Mon) 12 Dec 2011 [11:11:55], Miche Baker-Harvey wrote:
    > So on a CONSOLE_PORT_ADD message, we would take the
    > (existing)ports_device::ports_lock, and for other control messages we
    > would justtake the (new) port::port_lock?  You are concerned that just
    > takingthe ports_lock for all control messages could be too
    > restrictive?  Iwouldn't have expected these messages to be frequent
    > occurrences, butI'll defer to your experience here.

    No, I mean we'll have to take the new port_lock() everywhere we
    currently take the port lock, plus in a few more places. I only
    suggest using port_lock() helper since we'll need a dependency on the
    portdev lock as well.

    > The CONSOLE_CONSOLE_PORT message calls hvc_alloc, which also
    > needsserialization.  That's in another one of these three patches; are
    > youthinking we could leave that patch be, or that we would we use
    > theport_lock for CONSOLE_CONSOLE_PORT?  Using the port_lock
    > wouldprovide the HVC serialization "for free" but it would be cleaner
    > if weput HVC related synchronization in hvc_console.c.

    Yes, definitely, since other users of hvc_console may get bitten in
    similar ways. However, I'm not too familiar with the hvc code, the
    people at linux-ppc can be of help.

    > On Thu, Dec 8, 2011 at 4:08 AM, Amit Shah <> wrote:
    > > On (Tue) 06 Dec 2011 [09:05:38], Miche Baker-Harvey wrote:
    > >> Amit,
    > >>
    > >> Ah, indeed.  I am not using MSI-X, so virtio_pci::vp_try_to_find_vqs()
    > >> calls vp_request_intx() and sets up an interrupt callback.  From
    > >> there, when an interrupt occurs, the stack looks something like this:
    > >>
    > >> virtio_pci::vp_interrupt()
    > >>   virtio_pci::vp_vring_interrupt()
    > >>     virtio_ring::vring_interrupt()
    > >>       vq->vq.callback()  <-- in this case, that's virtio_console::control_intr()
    > >>         workqueue::schedule_work()
    > >>           workqueue::queue_work()
    > >>             queue_work_on(get_cpu())  <-- queues the work on the current CPU.
    > >>
    > >> I'm not doing anything to keep multiple control message from being
    > >> sent concurrently to the guest, and we will take those interrupts on
    > >> any CPU. I've confirmed that the two instances of
    > >> handle_control_message() are occurring on different CPUs.
    > >
    > > So let's have a new helper, port_lock() that takes the port-specific
    > > spinlock.  There has to be a new helper, since the port lock should
    > > depend on the portdev lock being taken too.  For the port addition
    > > case, just the portdev lock should be taken.  For any other
    > > operations, the port lock should be taken.
    > >
    > > My assumption was that we would be able to serialise the work items,
    > > but that will be too restrictive.  Taking port locks sounds like a
    > > better idea.
    > >
    > > We'd definitely need the port lock in the control work handler.  We
    > > might need it in a few more places (like module removal), but we'll
    > > worry about that later.
    > >
    > > Does this sound fine?
    > >
    > >                Amit

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2011-12-12 20:27    [W:0.029 / U:24.848 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site