On Sun, Jul 10, 2011 at 4:14 PM, Joe Eykholt <[email protected]> wrote:
> On 7/9/11 7:46 PM, Hillf Danton wrote:
>>
>> When receiving data, if CRC error encountered we have to do stats for the
>> event.
>> It is PER CPU operation, and opeartions related to preempt could be
>> dropped.
>>
>> Any comment is appreciated.
>>
>> Thanks
>>
>> Signed-off-by: Hillf Danton<[email protected]>
>> ---
>> drivers/scsi/libfc/fc_fcp.c | 4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
>> index 9cd2149..e14fc3b 100644
>> --- a/drivers/scsi/libfc/fc_fcp.c
>> +++ b/drivers/scsi/libfc/fc_fcp.c
>> @@ -495,14 +495,14 @@ static void fc_fcp_recv_data(struct fc_fcp_pkt
>> *fsp, struct fc_frame *fp)
>>
>> if (~crc != le32_to_cpu(fr_crc(fp))) {
>> crc_err:
>> - stats = per_cpu_ptr(lport->dev_stats, get_cpu());
>> + stats = per_cpu_ptr(lport->dev_stats,
>> smp_processor_id());
>
> At this point you could be preempted, so ...
>
>> stats->ErrorFrames++;
>
> could be incrementing the ErrorFrames count for the CPU you *used* to be on,
> not the CPU you're actually running on, so, the get_cpu(), put_cpu() were
> necessary. It's not very often that you'll have CRC errors, probably, but
> even so, it's good to be correct when using per-CPU variables.
>
Yeah, CRC error is rare.
But the comment just above fc_fcp_recv() says it is in Soft IRQ context, then
there is no window opened for preempt, I think.
thanks,
Hillf
_______________________________________________
devel mailing list
[email protected]
https://lists.open-fcoe.org/mailman/listinfo/devel
