On Thu, 20 Jul 2017 13:48:45 +0300
Dan Carpenter <dan.carpen...@oracle.com> wrote:

> Hello Stephen Hemminger,
> 
> The patch 15e1674de747: "vmbus: add prefetch to ring buffer iterator"
> from Jun 25, 2017, leads to the following static checker warning:
> 
>       drivers/hv/ring_buffer.c:366 hv_pkt_iter_first()
>       warn: 'desc' can't be NULL.
> 
> drivers/hv/ring_buffer.c
>    357  struct vmpacket_descriptor *hv_pkt_iter_first(struct vmbus_channel 
> *channel)
>    358  {
>    359          struct hv_ring_buffer_info *rbi = &channel->inbound;
>    360          struct vmpacket_descriptor *desc;
>    361  
>    362          if (hv_pkt_iter_avail(rbi) < sizeof(struct 
> vmpacket_descriptor))
>    363                  return NULL;
>    364  
>    365          desc = hv_get_ring_buffer(rbi) + rbi->priv_read_index;
>    366          if (desc)
>    367                  prefetch((char *)desc + (desc->len8 << 3));
>    368  
>    369          return desc;
>    370  }
>    371  EXPORT_SYMBOL_GPL(hv_pkt_iter_first);
> 
> This static checker warning is pretty bogus, but it's actually correct
> this time (like a stopped clock is right twice a day).  The first test
> ensures that there is enough space in the ring bugger so it can't return
> NULL.
> 
> How does that ring buffer work without any sort of locking??  It feels
> like you should have to say how much data you want to get in
> hv_get_ring_buffer() so that someone else doesn't come and claim it
> between the check for if it's avaible and the get.


You are right the NULL check is unnecessary here.
The ring buffer has no locking because it is serialized in other ways.
For example in networking, there can only be one NAPI instance per channel.
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to