On Thu, 20 Jul 2017 13:48:45 +0300
Dan Carpenter <dan.carpenter@xxxxxxxxxx> 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
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
