On Wed, Mar 06, 2013 at 02:16:30PM +0800, Asias He wrote: > +static struct tcm_vhost_evt *tcm_vhost_allocate_evt(struct vhost_scsi *vs, > + u32 event, u32 reason) > +{ > + struct tcm_vhost_evt *evt; > + > + if (atomic_read(&vs->vs_events_nr) > VHOST_SCSI_MAX_EVENT) > + return NULL; > + > + evt = kzalloc(sizeof(*evt), GFP_KERNEL); > + > + if (evt) { > + atomic_inc(&vs->vs_events_nr); This looks suspicious: checking vs_events_nr > VHOST_SCSI_MAX_EVENT first and then incrementing later isn't atomic! > +static int tcm_vhost_hotunplug(struct tcm_vhost_tpg *tpg, struct se_lun *lun) > +{ > + struct vhost_scsi *vs = tpg->vhost_scsi; > + > + mutex_lock(&tpg->tv_tpg_mutex); > + vs = tpg->vhost_scsi; > + mutex_unlock(&tpg->tv_tpg_mutex); > + if (!vs) > + return -EOPNOTSUPP; > + > + if (!tcm_vhost_check_feature(vs, VIRTIO_SCSI_F_HOTPLUG)) > + return -EOPNOTSUPP; > + > + return tcm_vhost_send_evt(vs, tpg, lun, > + VIRTIO_SCSI_T_TRANSPORT_RESET, > + VIRTIO_SCSI_EVT_RESET_REMOVED); > +} tcm_vhost_hotplug() and tcm_vhost_hotunplug() are the same function except for VIRTIO_SCSI_EVT_RESET_RESCAN vs VIRTIO_SCSI_EVT_RESET_REMOVED. That can be passed in as an argument and the code duplication can be eliminated. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html