On Wed, Oct 20, 2010 at 1:29 AM, Joe Eykholt <jeykholt@xxxxxxxxx> wrote:
>
> On 10/19/10 7:17 AM, Hillf Danton wrote:
>> There seems exchs should get released in cases that FC_RJT_NONE is
>> returned, or they will no longer get freed.
>>
>> It also looks a typo like the following,
>>
>> - Â Â Â Â Â Â Â Â fc_seq_lookup_recip(lport, ema->mp, fp) != FC_RJT_NONE)
>> + Â Â Â Â Â Â Â Â fc_seq_lookup_recip(lport, ema->mp, fp) == FC_RJT_NONE)
>
> True.
>
>>
>> but I am not sure.
>>
>> Signed-off-by: Hillf Danton <dhillf@xxxxxxxxx>
>> ---
>>
>> --- a/drivers/scsi/libfc/fc_exch.c  Â2010-09-13 07:07:38.000000000 +0800
>> +++ b/drivers/scsi/libfc/fc_exch.c  Â2010-10-19 21:48:22.000000000 +0800
>> @@ -3,6 +3,9 @@
>> Â * Copyright(c) 2008 Red Hat, Inc. ÂAll rights reserved.
>> Â * Copyright(c) 2008 Mike Christie
>> Â *
>> + * Copyright(c) Oct 19, 2010 Hillf Danton
>> + * Â Â Â Â Â Â Â Â Â Â Â Â Â minor fix of memory leakage
>> + *
>
> Thanks for fixing the bug, but I don't think you should add a copyright.
Node
> I didn't add a copyright when I created the bug.
The cool comment I ever seen, haha.
And the true patch will be prepared and delivered soon.
thanks //Hillf
>> Â * This program is free software; you can redistribute it and/or modify it
>> Â * under the terms and conditions of the GNU General Public License,
>> Â * version 2, as published by the Free Software Foundation.
>> @@ -1246,9 +1249,16 @@ static struct fc_seq *fc_seq_assign(stru
>> Â Â Â fr_seq(fp) = NULL;
>>
>> Â Â Â list_for_each_entry(ema, &lport->ema_list, ema_list)
>> - Â Â Â Â Â Â if ((!ema->match || ema->match(fp)) &&
>> - Â Â Â Â Â Â Â Â fc_seq_lookup_recip(lport, ema->mp, fp) != FC_RJT_NONE)
>> - Â Â Â Â Â Â Â Â Â Â break;
>> + Â Â Â Â Â Â if (! ema->match || ema->match(fp))
>> + Â Â Â Â Â Â Â Â Â Â if (fc_seq_lookup_recip(lport, ema->mp, fp) !=
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â FC_RJT_NONE)
>
> Should continue in this case ... we hope to find the exchange in another
> exchange manager.
>
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
>> + Â Â Â Â Â Â Â Â Â Â else {
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â struct fc_seq *seq = fr_seq(fp);
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â struct fc_exch *exch = fc_seq_exch(seq);
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â fc_exch_release(exch);
>
> I want the sequence to be held by the frame, so don't do the release.
> In this case we should break, since we found the frame.
>
> This function is currently only used by target modules. ÂA later
> patch exposes a release so they can release the sequence/exchange.
>
> So, my original code was right except for the != vs == bug.
> That turns out not to cause major problems if the first exchange
> manager assigns the exchange, which is why I didn't see the bug.
>
>> + Â Â Â Â Â Â Â Â Â Â }
>> +
>> Â Â Â return fr_seq(fp);
>> Â}
>
> By the way, I think patches to libfc, libfcoe, and fcoe should go to
> the open-fcoe list only, not to SCSI. ÂRobert Love as the open-fcoe
> maintainer will forward them after testing to the SCSI list.
>
> Â Â Â ÂRegards,
> Â Â Â ÂJoe
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
