Re: [PATCH 2.6.9-rc2 7/8] S2io: NAPI fix

[Jeff Garzik <jgarzik@pobox.com>]

Raghavendra Koushik wrote:
> Hi Jeff,
> 
> My comments inline.
> 
> 
>>-----Original Message-----
>>From: Jeff Garzik [mailto:jgarzik@pobox.com]
>>Sent: Thursday, October 14, 2004 8:00 AM
>>To: ravinandan.arakali@s2io.com
>>Cc: 'Francois Romieu'; netdev@oss.sgi.com; leonid.grossman@s2io.com;
>>raghavendra.koushik@s2io.com; rapuru.sriram@s2io.com
>>Subject: Re: [PATCH 2.6.9-rc2 7/8] S2io: NAPI fix
>>
>>Comments on patch #7:
>>
>>1) Can you prove that put_lock is really necessary, and not covered by
>>other methods of synchronization?  Typically the preferred model is that
>>your RX process requires _no_ spinlocks, and instead you use the net
>>stack API to ensure when your RX process is, and is not, running.
>>
> 
> <KSK>
> 	Lets say, CPU0 after processing s2io_isr, schedules s2io_tasklet.
> The tasklet in turn calls fill_rx_buffer which will replenish skbs into Rx
> descriptors and start modifying the put index of the Rx ring, at the same
> time if there is another Rx interrupt being processed (rx_intr_handler) on
> CPU1, then it will try to read put index of the ring which can result in
> synchronization problem. So I created this new variable put_pos to track the
> absolute value of the put index and make sure all its accesses are between
> spin locks. The same problem cannot happen if NAPI is used since the
> s2io_poll and fill_rx_buffer calls are serialized.


Have you actually benchmarked this on SMP?

It seems to me that you pay a penalty cross-CPU traffic with this scheme?

	Jeff