Re: [net-next PATCH 1/2] igbvf: add new driver to support 82576 virtualfunctions

[Alexander Duyck]

Andrew Morton wrote:
> On Tue, 17 Mar 2009 17:12:48 -0700 Alexander Duyck <alexander.h.duyck@xxxxxxxxx> wrote:
>
> > Thanks for all the comments.  I tried to incorporate most of them into 
> > the igbvf driver and also ended up porting some over to our other 
> > drivers, specifically igb since the igbvf driver copies much of the code.
> >
> > I have added my comments inline below.
> >
> > Thanks,
> >
> > Alex
> >
> > Andrew Morton wrote:
> > > On Tue, 10 Mar 2009 19:09:28 -0700 Jeff Kirsher <jeffrey.t.kirsher@xxxxxxxxx> wrote:
> > >
> > > > From: Alexander Duyck <alexander.h.duyck@xxxxxxxxx>
> > > >
> > > > This adds an igbvf driver to handle virtual functions provided
> > > > by the igb driver.
> > > The drive-by reader is now wondering what a "virtual function" is.
>
> ^^ this comment was missed.
>
> I was indirectly asking for an overview (preferably in the changelog) of
> what the whole patch actually does.

Sorry, while I missed the comment in my response I had gotten to 
addressing it in the next version.  I updated it to more thoroughly 
describe what the VF driver is doing.  I also included instructions on 
how to enable the VFs from the PF so that they can be tested.

> > > > +static int igbvf_set_ringparam(struct net_device *netdev,
> > > > +                               struct ethtool_ringparam *ring)
> > > > +{
> > > > +     struct igbvf_adapter *adapter = netdev_priv(netdev);
> > > > +     struct igbvf_ring *tx_ring, *tx_old;
> > > > +     struct igbvf_ring *rx_ring, *rx_old;
> > > > +     int err;
> > > > +
> > > > +     if ((ring->rx_mini_pending) || (ring->rx_jumbo_pending))
> > > > +             return -EINVAL;
> > > > +
> > > > +     while (test_and_set_bit(__IGBVF_RESETTING, &adapter->state))
> > > > +             msleep(1);
> > > No timeout needed here?  Interrupts might not be working, for example..
> > This bit isn't set in interrupt context.  This is always used out of 
> > interrupt context and is just to prevent multiple setting changes at the 
> > same time.
>
> Oh.  Can't use plain old mutex_lock()?

We have one or two spots that actually check to see if the bit is set 
and just report a warning instead of actually waiting on the bit to clear.

--
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/