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

[Alexander Duyck <alexander.h.duyck@intel.com>]

Andrew Morton wrote:
> On Tue, 17 Mar 2009 17:12:48 -0700 Alexander Duyck <alexander.h.duyck@intel.com> 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@intel.com> wrote:
>>>
>>>> From: Alexander Duyck <alexander.h.duyck@intel.com>
>>>>
>>>> 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.