-----Original Message-----
From: Mike Christie [mailto:micha...@cs.wisc.edu] 
Sent: Wednesday, February 25, 2009 9:22 AM
To: Karen Xie
Cc: open-iscsi@googlegroups.com; linux-s...@vger.kernel.org;
james.bottom...@hansenpartnership.com
Subject: Re: [PATCH 2/2 2.6.29-rc] cxgb3i - add handling of chip reset

Karen Xie wrote:
> [PATCH 2/2 2.6.29-rc] cxgb3i - add support of chip reset
> 
> From: Karen Xie <k...@chelsio.com>
> 
> The cxgb3 driver would reset the chip due to an EEH event or a fatal
error
> and notifies the upper layer modules (iscsi, rdma).
> Upon receiving the notification the cxgb3i driver would 
> - close all the iscsi tcp connections and mark the associated sessions
as
>   failed due to connection error,
> - re-initialize its offload functions,
> - re-initialize the chip's ddp functions.
> 
> The iscsi error recovery mechanism will re-establish the tcp
connection after
> reset. 
> 
> This patch requires the the following cxgb3 patch in the linux-next
git tree
>
(http://git.kernel.org/?p=linux/kernel/git/sfr/linux-next.git;a=commitdi
ff;h=cb0bc205959bf8c60acae9c71f3da0597e756f8e).
> 


>  
>  static inline int ddp_find_unused_entries(struct cxgb3i_ddp_info
*ddp,
> @@ -439,14 +444,15 @@ EXPORT_SYMBOL_GPL(cxgb3i_ddp_tag_reserve);
>   * @tag: ddp tag
>   * ddp cleanup for a given ddp tag and release all the resources held
>   */
> -void cxgb3i_ddp_tag_release(struct t3cdev *tdev, u32 tag)
> +int cxgb3i_ddp_tag_release(struct t3cdev *tdev, u32 tag)
>  {
>       struct cxgb3i_ddp_info *ddp = tdev->ulp_iscsi;
>       u32 idx;
> +     int err = -EINVAL;
>  
>       if (!ddp) {
>               ddp_log_error("release ddp tag 0x%x, ddp NULL.\n", tag);
> -             return;
> +             return err;
>       }
>  
>       idx = (tag >> PPOD_IDX_SHIFT) & ddp->idx_mask;
> @@ -457,17 +463,26 @@ void cxgb3i_ddp_tag_release(struct t3cdev *tdev,
u32 tag)
>               if (!gl) {
>                       ddp_log_error("release ddp 0x%x, idx 0x%x, gl
NULL.\n",
>                                     tag, idx);
> -                     return;
> +                     return err;
>               }
>               npods = (gl->nelem + PPOD_PAGES_MAX - 1) >>
PPOD_PAGES_SHIFT;
> +             if (!npods) {
> +                     ddp_log_error("release ddp 0x%x, 0x%x, gl elem
%u.\n",
> +                                   tag, idx, gl->nelem);
> +                     return err;
> +             }
>               ddp_log_debug("ddp tag 0x%x, release idx 0x%x, npods
%u.\n",
>                             tag, idx, npods);
> -             clear_ddp_map(ddp, idx, npods);
> +             if (clear_ddp_map(ddp, tag, idx, npods) == npods)
> +                     err = 0;
>               ddp_unmark_entries(ddp, idx, npods);
>               cxgb3i_ddp_release_gl(gl, ddp->pdev);


If this fails, do we leak this memory?

[Karen] The only memory allocated is the gl structure, even if it is not
freed here, it will be freed at the cleanup of the ddp function.

When is ddp_release called? Would it be called to clean up in situati 
this could fail?
[Karen] it is called in cleanup_task().


> +/**
> + * cxgb3i_adapter_remove - release all the resources held and cleanup
any
> + *   h/w settings

The docbook comment for the function description should only be one 
line. I think you did this in some other places.

[Karen] Thanks, I will fix them.

>  
> -void cxgb3i_conn_closing(struct s3_conn *c3cn)
> +void cxgb3i_conn_closing(struct s3_conn *c3cn, int error)
>  {
>       struct iscsi_conn *conn;
>  
>       read_lock(&c3cn->callback_lock);
>       conn = c3cn->user_data;
> -     if (conn && c3cn->state != C3CN_STATE_ESTABLISHED)
> -             iscsi_conn_failure(conn, ISCSI_ERR_CONN_FAILED);
> +     if (conn && c3cn->state != C3CN_STATE_ESTABLISHED) {
> +             if (error)
> +                     iscsi_session_failure(conn->session,
> +                                             ISCSI_ERR_CONN_FAILED);
> +             else
> +                     iscsi_conn_failure(conn, ISCSI_ERR_CONN_FAILED);
> +     }
>       read_unlock(&c3cn->callback_lock);


If you have a ref to the conn, you can just use iscsi_conn_failure here.

I thought when you got the pci error event you were just going to use 
the host session iter and loop over the session and fail them.

[Karen] Yes, I could do it via iscsi_host_for_each_session() too.

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~----------~----~----~----~------~----~------~--~---

Reply via email to