On Tue, Oct 13, 2009 at 12:57 AM, Vu Pham <[email protected]> wrote:
>
>
> Handling async local port events: port err, active, lid change...
> Upon local port err, it will set up timer srp_dev_loss_tmo seconds to
> reconnect. If local port active and there is timer, it will delete the timer
>
> Signed-off-by: Vu Pham <[email protected]>
>
> Index: ofed_kernel/drivers/infiniband/ulp/srp/ib_srp.c
> ===================================================================
> --- ofed_kernel.orig/drivers/infiniband/ulp/srp/ib_srp.c
> +++ ofed_kernel/drivers/infiniband/ulp/srp/ib_srp.c
> @@ -2071,6 +2120,77 @@
> return NULL;
> }
>
> +static void srp_event_handler(struct ib_event_handler *handler,
> + struct ib_event *event)
> +{
> + struct srp_device *srp_dev =
> + ib_get_client_data(event->device, &srp_client);
> + struct srp_host *host, *tmp_host;
> + struct srp_target_port *target, *tmp_target;
> +
> + if (!srp_dev || srp_dev->dev != event->device)
> + return;
> +
> + printk(KERN_WARNING PFX "ASYNC event= %d on device= %s\n",
> + event->event, srp_dev->dev->name);
> +
> + switch (event->event) {
> + case IB_EVENT_PORT_ERR:
> + list_for_each_entry_safe(host, tmp_host,
> + &srp_dev->dev_list, list) {
> + if (event->element.port_num == host->port) {
> + spin_lock(&host->target_lock);
Can srp_remove_work() be executed concurrently with
srp_event_handler() ? In that case the above code isn't safe and the
spin_lock(&host->target_lock) should be moved to just before
list_for_each_entry_safe(). The current implementation can trigger
reading deallocated memory.
> + list_for_each_entry_safe(target, tmp_target,
> + &host->target_list,
> list) {
> + unsigned long flags;
> +
> +
> spin_lock_irqsave(target->scsi_host->host_lock,
> + flags);
> + if (!target->qp_in_error &&
> + target->state == SRP_TARGET_LIVE)
> + srp_qp_err_add_timer(target,
> +
> srp_dev_loss_tmo);
> +
> spin_unlock_irqrestore(target->scsi_host->host_lock,
> + flags);
> + }
> + spin_unlock(&host->target_lock);
> + }
> + }
> + break;
> + case IB_EVENT_PORT_ACTIVE:
> + case IB_EVENT_LID_CHANGE:
> + case IB_EVENT_PKEY_CHANGE:
> + case IB_EVENT_SM_CHANGE:
> + list_for_each_entry_safe(host, tmp_host, &srp_dev->dev_list,
> + list) {
> + if (event->element.port_num == host->port) {
> + spin_lock(&host->target_lock);
A remark similar to the above: this code isn't safe against concurrent
calls of srp_remove_one().
> + list_for_each_entry_safe(target, tmp_target,
> + &host->target_list,
> list) {
> + unsigned long flags;
> +
> +
> spin_lock_irqsave(target->scsi_host->host_lock,
> + flags);
> + if
> (timer_pending(&target->qp_err_timer)
> + && !target->qp_in_error) {
> + shost_printk(KERN_WARNING PFX,
> +
> target->scsi_host,
> + "delete
> qp_in_err timer\n");
> +
> del_timer(&target->qp_err_timer);
> + }
> +
> spin_unlock_irqrestore(target->scsi_host->host_lock,
> + flags);
> + }
> + spin_unlock(&host->target_lock);
> + }
> + }
> + break;
> + default:
> + break;
> + }
> +
> +}
> +
> static void srp_add_one(struct ib_device *device)
> {
> struct srp_device *srp_dev;
> @@ -2116,6 +2228,11 @@
> if (IS_ERR(srp_dev->mr))
> goto err_pd;
>
> + INIT_IB_EVENT_HANDLER(&srp_dev->event_handler, srp_dev->dev,
> + srp_event_handler);
> + if (ib_register_event_handler(&srp_dev->event_handler))
> + goto err_pd;
> +
> memset(&fmr_param, 0, sizeof fmr_param);
> fmr_param.pool_size = SRP_FMR_POOL_SIZE;
> fmr_param.dirty_watermark = SRP_FMR_DIRTY_SIZE;
> @@ -2160,6 +2284,8 @@
>
> srp_dev = ib_get_client_data(device, &srp_client);
>
> + ib_unregister_event_handler(&srp_dev->event_handler);
> +
> list_for_each_entry_safe(host, tmp_host, &srp_dev->dev_list, list) {
> device_unregister(&host->dev);
> /*
> Index: ofed_kernel/drivers/infiniband/ulp/srp/ib_srp.h
> ===================================================================
> --- ofed_kernel.orig/drivers/infiniband/ulp/srp/ib_srp.h
> +++ ofed_kernel/drivers/infiniband/ulp/srp/ib_srp.h
> @@ -88,6 +88,7 @@ struct srp_device {
> struct ib_device *dev;
> struct ib_pd *pd;
> struct ib_mr *mr;
> + struct ib_event_handler event_handler;
> struct ib_fmr_pool *fmr_pool;
> int fmr_page_shift;
> int fmr_page_size;
>
N§²æìr¸yúèØb²X¬¶Ç§vØ^)Þº{.nÇ+·¥{±Ù{ayºÊÚë,j¢f£¢·hàz¹®w¥¢¸
¢·¦j:+v¨wèjØm¶ÿ¾«êçzZ+ùÝ¢j"ú!¶i