On Wed, Jul 20, 2016 at 11:07:57AM +0200, Daniel Borkmann wrote:
> On 07/19/2016 09:16 PM, Brenden Blanco wrote:
[...]
> >+    if (ring->xdp_prog)
> >+            bpf_prog_put(ring->xdp_prog);
> 
> Would be good if you also make this a READ_ONCE() here. I believe this is the
> only other spot in your set that has this 'direct' access (besides xchg() and
> READ_ONCE() from mlx4_en_process_rx_cq()). It would be mostly for consistency
> and to indicate that there's a more complex synchronization behind it. I'm 
> mostly
> worried that if it's not consistently used, people might copy this and not use
> the READ_ONCE() also in other spots where it matters, and thus add hard to 
> find
> bugs.
I can do that. My thinking was just that this is the cleanup path so the
code would have been superfluous. I think there were a few nits so I'll
collect those and clean them up.
> 
[...]

Reply via email to