kern/123095 kern/131602 sendfile

[Kostik Belousov]

On Thu, Jul 08, 2010 at 01:34:28PM +0200, Andre Oppermann wrote:
> On 08.07.2010 11:42, Kostik Belousov wrote:
> >On Thu, Jul 08, 2010 at 11:40:05AM +0300, Andriy Gapon wrote:
> >>on 08/07/2010 11:29 Kostik Belousov said the following:
> >>>Right, the patch maps the page in sf buffer read-only (on i386 only).
> >>>But note the parallel posting with m_cat() change. It is still not 
> >>>enough,
> >>>and I am not set up for the real network testing ATM.
> >>
> >>Could you also try to experiment with mb_dupcl?
> >>Namely transfer M_RDONLY from source mbuf.
> >
> >Right, it is it.
> >
> >Below is my current patch including debugging facilities that seems to 
> >work.
> >Real changes that needed are in m_cat and mb_dupcl.
> >
> ...
> >diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c
> >index f41eb03..1701ef2 100644
> >--- a/sys/kern/uipc_mbuf.c
> >+++ b/sys/kern/uipc_mbuf.c
> >@@ -301,7 +301,7 @@ mb_dupcl(struct mbuf *n, struct mbuf *m)
> >  	n->m_ext.ext_size = m->m_ext.ext_size;
> >  	n->m_ext.ref_cnt = m->m_ext.ref_cnt;
> >  	n->m_ext.ext_type = m->m_ext.ext_type;
> >-	n->m_flags |= M_EXT;
> >+	n->m_flags |= M_EXT | (M_RDONLY & m->m_flags);
> >  }
> 
> Having the M_EXT flag always implies readonly and M_WRITABLE gets this
> right.  Not inheriting all the flags from the source seems questionable.
> So IMHO this should be done here:
> 
>  n->m_flags |= (M_EXT | m->m_flags)
> 
> >  /*
> >@@ -911,7 +911,8 @@ m_cat(struct mbuf *m, struct mbuf *n)
> >  		m = m->m_next;
> >  	while (n) {
> >  		if (m->m_flags&  M_EXT ||
> >-		    m->m_data + m->m_len + n->m_len>=&m->m_dat[MLEN]) {
> >+		    m->m_data + m->m_len + n->m_len>=&m->m_dat[MLEN] ||
> >+		    !M_WRITABLE(m)) {
> 
> Here you can fully replace the (m->m_flags & M_EXT) test with
> M_WRITABLE().  The M_EXT test is included in it.
> 
> >  			/* just join the two chains */
> >  			m->m_next = n;
> >  			return;

The patch is below. Works as well for me, thank you for the feedback.

diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c
index f41eb03..58567a4 100644
--- a/sys/kern/uipc_mbuf.c
+++ b/sys/kern/uipc_mbuf.c
@@ -301,7 +301,7 @@ mb_dupcl(struct mbuf *n, struct mbuf *m)
 	n->m_ext.ext_size = m->m_ext.ext_size;
 	n->m_ext.ref_cnt = m->m_ext.ref_cnt;
 	n->m_ext.ext_type = m->m_ext.ext_type;
-	n->m_flags |= M_EXT;
+	n->m_flags |= M_EXT | m->m_flags;
 }
 
 /*
@@ -910,7 +910,7 @@ m_cat(struct mbuf *m, struct mbuf *n)
 	while (m->m_next)
 		m = m->m_next;
 	while (n) {
-		if (m->m_flags & M_EXT ||
+		if (!M_WRITABLE(m) ||
 		    m->m_data + m->m_len + n->m_len >= &m->m_dat[MLEN]) {
 			/* just join the two chains */
 			m->m_next = n;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-net/attachments/20100708/6a336286/attachment.pgp