> From: David Miller [mailto:davem@xxxxxxxxxxxxx] > Sent: Thursday, August 24, 2017 18:20 > > +#define VMBUS_PKT_TRAILER (sizeof(u64)) > > This is not the packet trailer, it's the size of the packet trailer. Thanks! I'll change it to VMBUS_PKT_TRAILER_SIZE. > > + /* Have we sent the zero-length packet (FIN)? */ > > + unsigned long fin_sent; > > Why does this need to be atomic? Why can't a smaller simpler It doesn't have to be. It was originally made for a quick workaround. Thanks! I should do it in the right way now. > mechanism be used to make sure hvs_shutdown() only performs > hvs_send_data() call once on the channel? I'll change "fin_sent" to bool, and avoid test_and_set_bit(). I'll add lock_sock/release_sock() in hvs_shutdown() like this: static int hvs_shutdown(struct vsock_sock *vsk, int mode) { ... lock_sock(sk); hvs = vsk->trans; if (hvs->fin_sent) goto out; send_buf = (struct hvs_send_buf *)&hdr; (void)hvs_send_data(hvs->chan, send_buf, 0); hvs->fin_sent = true; out: release_sock(sk); return 0; } > > +static inline bool is_valid_srv_id(const uuid_le *id) > > +{ > > + return !memcmp(&id->b[4], &srv_id_template.b[4], sizeof(uuid_le) - > 4); > > +} > > Do not use the inline function attribute in *.c code. Let the > compiler decide. OK. Will remove all the inline usages. > > + *((u32 *)&h->vm_srv_id) = vsk->local_addr.svm_port; > > + *((u32 *)&h->host_srv_id) = vsk->remote_addr.svm_port; > > There has to be a better way to express this. I may need to define a uinon here. Let me try it. > And if this is partially initializing vm_srv_id, at a minimum > endianness needs to be taken into account. I may need to use cpu_to_le32(). Let me check it. _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel