[openib-general] Re: [PATCH] SRP: don't use TX IU after freei ng it

Tziporet Koren tziporet at mellanox.co.il
Sun Oct 30 22:50:05 PST 2005


Hi Roland,
When do you expect to apply the FMRs patch for SRP?

Thanks,
Tziporet

-----Original Message-----
From: Vu Pham [mailto:vuhuong at mellanox.com]
Sent: Tuesday, October 11, 2005 8:03 PM
To: Roland Dreier
Cc: kingman at storagegear.com; openib-general at openib.org
Subject: [openib-general] Re: [PATCH] SRP: don't use TX IU after freeing
it


Roland,
    Thanks or reviewing it.
    Responding to your feedback, I prepare new patch (attached)


> 
> Why put a pointer to struct list_head here instead of just a struct
> list_head?  If you just used the struct, then you wouldn't need this:
>

Done. Using struct list_head instead of pointer



>     > +	u16			in_use;
>     >  };
> 
> I can't find anywhere that the in_use flag is used.
>

Removed


>     > +static int srp_map_fmr(struct srp_target_port *target, struct
scatterlist *scat,
>     > +		       int sg_cnt, struct srp_request *req)
> 
>     [...]
> 
>     > +	return -ENOMEM;
> 
>     > +			} else if (fmr_cnt <= 0) {
> 
> fmr_cnt is unsigned so I think this is going to get you in trouble.
> Might as well make fmr_cnt a plain int to make things simpler.
> 

In previous patch, fmr_cnt was already declared as int

> Also, it might be good to try and add some more comments explaining
> srp_map_fmr() -- it would definitely help me review.
> 

I added some comments - Hope they help your review (instead 
of confusing you more :))

Signed-off-by: Vu Pham <vu at mellanox.com>




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/general/attachments/20051031/c2e668dd/attachment.html>


More information about the general mailing list