* [PATCH 1/2] libfc: fix mem leak in fc_tm_done()
@ 2010-10-25 12:37 Hillf Danton
2010-10-25 12:42 ` [PATCH 2/2] " Hillf Danton
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Hillf Danton @ 2010-10-25 12:37 UTC (permalink / raw)
To: devel-s9riP+hp16TNLxjTenLetw; +Cc: linux-scsi-u79uwXL29TY76Z2rM5mHXA
There seems frame should get freed against memory leakage.
Signed-off-by: Hillf Danton <dhillf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
--- a/drivers/scsi/libfc/fc_fcp.c 2010-09-13 07:07:38.000000000 +0800
+++ b/drivers/scsi/libfc/fc_fcp.c 2010-10-25 20:31:04.000000000 +0800
@@ -1294,6 +1294,7 @@ static void fc_tm_done(struct fc_seq *se
*/
if (!fsp->seq_ptr || !fsp->wait_for_comp) {
spin_unlock_bh(&fsp->scsi_pkt_lock);
+ fc_frame_free(fp);
return;
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] libfc: fix mem leak in fc_tm_done()
2010-10-25 12:37 [PATCH 1/2] libfc: fix mem leak in fc_tm_done() Hillf Danton
@ 2010-10-25 12:42 ` Hillf Danton
2010-10-28 1:18 ` [Open-FCoE] [PATCH 1/2] " Robert Love
[not found] ` <AANLkTinAOy9+WPuKGro09Cu=xiLF+QMQ+JkioHNZao9n-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2 siblings, 0 replies; 8+ messages in thread
From: Hillf Danton @ 2010-10-25 12:42 UTC (permalink / raw)
To: devel; +Cc: linux-scsi
There seems fc_fcp_unlock_pkt should be issued against mem leak.
Signed-off-by: Hillf Danton <dhillf@gmail.com>
---
--- a/drivers/scsi/libfc/fc_fcp.c 2010-09-13 07:07:38.000000000 +0800
+++ b/drivers/scsi/libfc/fc_fcp.c 2010-10-25 20:32:18.000000000 +0800
@@ -1293,7 +1293,7 @@ static void fc_tm_done(struct fc_seq *se
* raced with eh timeout handler.
*/
if (!fsp->seq_ptr || !fsp->wait_for_comp) {
- spin_unlock_bh(&fsp->scsi_pkt_lock);
+ fc_fcp_unlock_pkt(fsp);
return;
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Open-FCoE] [PATCH 1/2] libfc: fix mem leak in fc_tm_done()
2010-10-25 12:37 [PATCH 1/2] libfc: fix mem leak in fc_tm_done() Hillf Danton
2010-10-25 12:42 ` [PATCH 2/2] " Hillf Danton
@ 2010-10-28 1:18 ` Robert Love
2010-10-28 13:47 ` Hillf Danton
[not found] ` <AANLkTinAOy9+WPuKGro09Cu=xiLF+QMQ+JkioHNZao9n-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2 siblings, 1 reply; 8+ messages in thread
From: Robert Love @ 2010-10-28 1:18 UTC (permalink / raw)
To: Hillf Danton; +Cc: devel, linux-scsi
On Mon, 2010-10-25 at 20:37 +0800, Hillf Danton wrote:
> There seems frame should get freed against memory leakage.
>
> Signed-off-by: Hillf Danton <dhillf@gmail.com>
> ---
>
> --- a/drivers/scsi/libfc/fc_fcp.c 2010-09-13 07:07:38.000000000 +0800
> +++ b/drivers/scsi/libfc/fc_fcp.c 2010-10-25 20:31:04.000000000 +0800
> @@ -1294,6 +1294,7 @@ static void fc_tm_done(struct fc_seq *se
> */
> if (!fsp->seq_ptr || !fsp->wait_for_comp) {
> spin_unlock_bh(&fsp->scsi_pkt_lock);
> + fc_frame_free(fp);
> return;
> }
> _______________________________________________
> devel mailing list
> devel@open-fcoe.org
> http://www.open-fcoe.org/mailman/listinfo/devel
Hi Hillf,
I'm not sure how you're generating your patches, but it seems that
many of them (not sure if it's all) do not have a trailing whitespace
line. So, when I try to apply the patch either using 'patch' or 'git-am'
it sees the first line of the mailing list footer as part of the patch.
So the, "____________________" line is considered part of the patch and
it causes the application of the patch to fail. Here's what happens when
I try to apply the mbox for this patch.
rwlove@fritz:/var/testspace/open-fcoe-initiator$ git-am
~/Desktop/1-2-libfc-fix-mem-leak-in-fc_tm_done.patch
Applying: libfc: fix mem leak in fc_tm_done()
fatal: corrupt patch at line 16
Patch failed at 0001 libfc: fix mem leak in fc_tm_done()
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
rwlove@fritz:/var/testspace/open-fcoe-initiator$ patch -p1 --dry-run
< .git/rebase-apply/0001
patching file drivers/scsi/libfc/fc_fcp.c
patch: **** malformed patch at line 28:
_______________________________________________
I'm not a patch formatting expert, but I think the lack of a
whitespace line at the end of the changes is the problem.
I'd suggest mailing the patch to yourself first and then try applying
the mbox with 'git-am', if that works then everything should be fine.
Also, I like to use stgit to generate and mail patches. Everyone has
their own development process, but I want to point that tool out to you
because it will take care of all of the formatting for you.
Thanks, //Rob
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] libfc: fix mem leak in fc_tm_done()
[not found] ` <AANLkTinAOy9+WPuKGro09Cu=xiLF+QMQ+JkioHNZao9n-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-10-28 1:24 ` Robert Love
2010-10-28 1:55 ` Mike Christie
0 siblings, 1 reply; 8+ messages in thread
From: Robert Love @ 2010-10-28 1:24 UTC (permalink / raw)
To: Hillf Danton
Cc: devel-s9riP+hp16TNLxjTenLetw, linux-scsi-u79uwXL29TY76Z2rM5mHXA
On Mon, 2010-10-25 at 20:37 +0800, Hillf Danton wrote:
> There seems frame should get freed against memory leakage.
>
> Signed-off-by: Hillf Danton <dhillf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>
> --- a/drivers/scsi/libfc/fc_fcp.c 2010-09-13 07:07:38.000000000 +0800
> +++ b/drivers/scsi/libfc/fc_fcp.c 2010-10-25 20:31:04.000000000 +0800
> @@ -1294,6 +1294,7 @@ static void fc_tm_done(struct fc_seq *se
> */
> if (!fsp->seq_ptr || !fsp->wait_for_comp) {
> spin_unlock_bh(&fsp->scsi_pkt_lock);
I wonder if this should be a fc_fcp_unlock_pkt(fsp) since we get the
lock with the fc_fcp_lock_pkt(fsp) call above.
> + fc_frame_free(fp);
> return;
> }
> _______________________________________________
> devel mailing list
> devel-s9riP+hp16TNLxjTenLetw@public.gmane.org
> http://www.open-fcoe.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] libfc: fix mem leak in fc_tm_done()
2010-10-28 1:24 ` Robert Love
@ 2010-10-28 1:55 ` Mike Christie
2010-10-28 13:43 ` [Open-FCoE] " Hillf Danton
0 siblings, 1 reply; 8+ messages in thread
From: Mike Christie @ 2010-10-28 1:55 UTC (permalink / raw)
To: Robert Love
Cc: Hillf Danton, devel-s9riP+hp16TNLxjTenLetw,
linux-scsi-u79uwXL29TY76Z2rM5mHXA
On 10/27/2010 08:24 PM, Robert Love wrote:
> On Mon, 2010-10-25 at 20:37 +0800, Hillf Danton wrote:
>> There seems frame should get freed against memory leakage.
>>
>> Signed-off-by: Hillf Danton<dhillf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>
>> --- a/drivers/scsi/libfc/fc_fcp.c 2010-09-13 07:07:38.000000000 +0800
>> +++ b/drivers/scsi/libfc/fc_fcp.c 2010-10-25 20:31:04.000000000 +0800
>> @@ -1294,6 +1294,7 @@ static void fc_tm_done(struct fc_seq *se
>> */
>> if (!fsp->seq_ptr || !fsp->wait_for_comp) {
>> spin_unlock_bh(&fsp->scsi_pkt_lock);
>
> I wonder if this should be a fc_fcp_unlock_pkt(fsp) since we get the
> lock with the fc_fcp_lock_pkt(fsp) call above.
>
Did I do that. I think you are right. Not sure what happened if it was me.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Open-FCoE] [PATCH 1/2] libfc: fix mem leak in fc_tm_done()
2010-10-28 1:55 ` Mike Christie
@ 2010-10-28 13:43 ` Hillf Danton
2010-10-28 14:56 ` Zou, Yi
0 siblings, 1 reply; 8+ messages in thread
From: Hillf Danton @ 2010-10-28 13:43 UTC (permalink / raw)
To: Mike Christie; +Cc: Robert Love, devel, linux-scsi
On Thu, Oct 28, 2010 at 9:55 AM, Mike Christie <michaelc@cs.wisc.edu> wrote:
> On 10/27/2010 08:24 PM, Robert Love wrote:
>>
>> On Mon, 2010-10-25 at 20:37 +0800, Hillf Danton wrote:
>>>
>>> There seems frame should get freed against memory leakage.
>>>
>>> Signed-off-by: Hillf Danton<dhillf@gmail.com>
>>> ---
>>>
>>> --- a/drivers/scsi/libfc/fc_fcp.c 2010-09-13 07:07:38.000000000
>>> +0800
>>> +++ b/drivers/scsi/libfc/fc_fcp.c 2010-10-25 20:31:04.000000000
>>> +0800
>>> @@ -1294,6 +1294,7 @@ static void fc_tm_done(struct fc_seq *se
>>> */
>>> if (!fsp->seq_ptr || !fsp->wait_for_comp) {
>>> spin_unlock_bh(&fsp->scsi_pkt_lock);
>>
>> I wonder if this should be a fc_fcp_unlock_pkt(fsp) since we get the
>> lock with the fc_fcp_lock_pkt(fsp) call above.
>>
According to one question a patch, two were delivered, please check them.
Here I want frame get freed. //Hillf
>
> Did I do that. I think you are right. Not sure what happened if it was me.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Open-FCoE] [PATCH 1/2] libfc: fix mem leak in fc_tm_done()
2010-10-28 1:18 ` [Open-FCoE] [PATCH 1/2] " Robert Love
@ 2010-10-28 13:47 ` Hillf Danton
0 siblings, 0 replies; 8+ messages in thread
From: Hillf Danton @ 2010-10-28 13:47 UTC (permalink / raw)
To: Robert Love; +Cc: devel, linux-scsi
On Thu, Oct 28, 2010 at 9:18 AM, Robert Love <robert.w.love@intel.com> wrote:
> On Mon, 2010-10-25 at 20:37 +0800, Hillf Danton wrote:
>> There seems frame should get freed against memory leakage.
>>
>> Signed-off-by: Hillf Danton <dhillf@gmail.com>
>> ---
>>
>> --- a/drivers/scsi/libfc/fc_fcp.c 2010-09-13 07:07:38.000000000 +0800
>> +++ b/drivers/scsi/libfc/fc_fcp.c 2010-10-25 20:31:04.000000000 +0800
>> @@ -1294,6 +1294,7 @@ static void fc_tm_done(struct fc_seq *se
>> */
>> if (!fsp->seq_ptr || !fsp->wait_for_comp) {
>> spin_unlock_bh(&fsp->scsi_pkt_lock);
>> + fc_frame_free(fp);
>> return;
>> }
>> _______________________________________________
>> devel mailing list
>> devel@open-fcoe.org
>> http://www.open-fcoe.org/mailman/listinfo/devel
>
> Hi Hillf,
>
> I'm not sure how you're generating your patches, but it seems that
> many of them (not sure if it's all) do not have a trailing whitespace
> line. So, when I try to apply the patch either using 'patch' or 'git-am'
> it sees the first line of the mailing list footer as part of the patch.
> So the, "____________________" line is considered part of the patch and
> it causes the application of the patch to fail. Here's what happens when
> I try to apply the mbox for this patch.
>
> rwlove@fritz:/var/testspace/open-fcoe-initiator$ git-am
> ~/Desktop/1-2-libfc-fix-mem-leak-in-fc_tm_done.patch
> Applying: libfc: fix mem leak in fc_tm_done()
> fatal: corrupt patch at line 16
> Patch failed at 0001 libfc: fix mem leak in fc_tm_done()
> When you have resolved this problem run "git am --resolved".
> If you would prefer to skip this patch, instead run "git am --skip".
> To restore the original branch and stop patching run "git am --abort".
> rwlove@fritz:/var/testspace/open-fcoe-initiator$ patch -p1 --dry-run
> < .git/rebase-apply/0001
> patching file drivers/scsi/libfc/fc_fcp.c
> patch: **** malformed patch at line 28:
> _______________________________________________
>
>
> I'm not a patch formatting expert, but I think the lack of a
> whitespace line at the end of the changes is the problem.
>
> I'd suggest mailing the patch to yourself first and then try applying
> the mbox with 'git-am', if that works then everything should be fine.
>
> Also, I like to use stgit to generate and mail patches. Everyone has
> their own development process, but I want to point that tool out to you
> because it will take care of all of the formatting for you.
>
> Thanks, //Rob
Sorry, get you in trouble again. More care will be taken. //Hillf
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [Open-FCoE] [PATCH 1/2] libfc: fix mem leak in fc_tm_done()
2010-10-28 13:43 ` [Open-FCoE] " Hillf Danton
@ 2010-10-28 14:56 ` Zou, Yi
0 siblings, 0 replies; 8+ messages in thread
From: Zou, Yi @ 2010-10-28 14:56 UTC (permalink / raw)
To: Hillf Danton, Mike Christie; +Cc: devel, linux-scsi
> On Thu, Oct 28, 2010 at 9:55 AM, Mike Christie <michaelc@cs.wisc.edu>
> wrote:
> > On 10/27/2010 08:24 PM, Robert Love wrote:
> >>
> >> On Mon, 2010-10-25 at 20:37 +0800, Hillf Danton wrote:
> >>>
> >>> There seems frame should get freed against memory leakage.
> >>>
> >>> Signed-off-by: Hillf Danton<dhillf@gmail.com>
> >>> ---
> >>>
> >>> --- a/drivers/scsi/libfc/fc_fcp.c 2010-09-13 07:07:38.000000000
> >>> +0800
> >>> +++ b/drivers/scsi/libfc/fc_fcp.c 2010-10-25 20:31:04.000000000
> >>> +0800
> >>> @@ -1294,6 +1294,7 @@ static void fc_tm_done(struct fc_seq *se
> >>> */
> >>> if (!fsp->seq_ptr || !fsp->wait_for_comp) {
> >>> spin_unlock_bh(&fsp->scsi_pkt_lock);
> >>
> >> I wonder if this should be a fc_fcp_unlock_pkt(fsp) since we get the
> >> lock with the fc_fcp_lock_pkt(fsp) call above.
> >>
> According to one question a patch, two were delivered, please check them.
>
> Here I want frame get freed. //Hillf
>
> >
> > Did I do that. I think you are right. Not sure what happened if it was
> me.
> >
I would think fc_frame_free() be called on all the if () { ..return } cases, the
caller won't do that from fc_exch_recv(), also, frame should be freed after unlock
the fsp:
{
if (IS_ERR(fp))
goto out;
if (fc_fcp_lock_pkt(fsp))
goto out;
if (!fsp->seq_ptr || !fsp->wait_for_comp)
goto out_unlock;
...
out_unlock:
fc_fcp_unlock_pkt(fsp);
out:
fc_frame_free(fp);
}
yi
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-10-28 14:56 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-25 12:37 [PATCH 1/2] libfc: fix mem leak in fc_tm_done() Hillf Danton
2010-10-25 12:42 ` [PATCH 2/2] " Hillf Danton
2010-10-28 1:18 ` [Open-FCoE] [PATCH 1/2] " Robert Love
2010-10-28 13:47 ` Hillf Danton
[not found] ` <AANLkTinAOy9+WPuKGro09Cu=xiLF+QMQ+JkioHNZao9n-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-10-28 1:24 ` Robert Love
2010-10-28 1:55 ` Mike Christie
2010-10-28 13:43 ` [Open-FCoE] " Hillf Danton
2010-10-28 14:56 ` Zou, Yi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.