On 08.07.2010 13:47, Kostik Belousov wrote:
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.

You may want to run the change to m_dupcl() by rwatson just to be sure.
It should be the right thing to do but better have another mbuf expert
look at it.

Does this fix the sendfile corruption problem for all cases or just the
panic for the loopback case?

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;

--
Andre
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"

Reply via email to