From owner-freebsd-arch  Fri Dec  1 16: 5:11 2000
Delivered-To: freebsd-arch@freebsd.org
Received: from implode.root.com (root.com [209.102.106.178])
	by hub.freebsd.org (Postfix) with ESMTP
	id 8CE2B37B400; Fri,  1 Dec 2000 16:05:07 -0800 (PST)
Received: from implode.root.com (localhost [127.0.0.1])
	by implode.root.com (8.8.8/8.8.5) with ESMTP id QAA14320;
	Fri, 1 Dec 2000 16:01:42 -0800 (PST)
Message-Id: <200012020001.QAA14320@implode.root.com>
To: Andrew Gallatin <gallatin@cs.duke.edu>,
	Bosko Milekic <bmilekic@technokratis.com>,
	"Kenneth D. Merry" <ken@kdm.org>, arch@FreeBSD.ORG,
	alfred@FreeBSD.ORG
Subject: Re: zero copy code review 
In-reply-to: Your message of "Fri, 01 Dec 2000 15:26:19 PST."
             <200012012326.PAA14154@implode.root.com> 
From: David Greenman <dg@root.com>
Reply-To: dg@root.com
Date: Fri, 01 Dec 2000 16:01:41 -0800
Sender: owner-freebsd-arch@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG

>> > 	In your code, you do deal with the possibility of the MGETHDR
>> >   returning NULL (you check for it) and you set ENOBUFS in that case and
>> >   jump to the "errorpath" label. But, before using MGETHDR, you allocate an
>> >   sf_buf (in sf) and it just so happens that the code beyond "errorpath"
>> >   does not take care of freeing the sf_buf you allocated before even
>> >   trying to allocate the mbuf.
>>
>>I see your point.  This was copied, (bug for bug ;-), from sendfile itself.
>>Look at line 1700 or so of kern/uipc_syscalls.c..  This bug should
>>probaby be fixed there too..
>
>   Oops. The original assumption (and code that I wrote) was that M_WAIT
>_cannot_ return a NULL pointer. This was changed in FreeBSD recently, and
>as you mentioned, the code added in rev 1.65 that now checks for it in
>sendfile doesn't do complete cleanup in this case. It definately should
>be fixed so that the sf_buf is freed as well.

   Followup...the attached patch should fix the problem.

-DG

David Greenman
Co-founder, The FreeBSD Project - http://www.freebsd.org
President, TeraSolutions, Inc. - http://www.terasolutions.com
Pave the road of life with opportunities.


Index: uipc_syscalls.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.65.2.3
diff -c -r1.65.2.3 uipc_syscalls.c
*** uipc_syscalls.c	2000/08/16 19:20:31	1.65.2.3
--- uipc_syscalls.c	2000/12/01 23:54:19
***************
*** 1628,1633 ****
--- 1630,1636 ----
  		MGETHDR(m, M_WAIT, MT_DATA);
  		if (m == NULL) {
  			error = ENOBUFS;
+ 			sf_buf_free((void *)sf->kva, PAGE_SIZE);
  			goto done;
  		}
  		m->m_ext.ext_free = sf_buf_free;


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message