Date: Thu, 08 Jul 2010 13:34:28 +0200 From: Andre Oppermann <andre@freebsd.org> To: Kostik Belousov <kostikbel@gmail.com> Cc: lstewart@freebsd.org, bz@freebsd.org, Ming Fu <Ming.Fu@watchguard.com>, Andriy Gapon <avg@icyb.net.ua>, freebsd-net@freebsd.org Subject: Re: kern/123095 kern/131602 sendfile Message-ID: <4C35B7C4.40507@freebsd.org> In-Reply-To: <20100708094255.GA2408@deviant.kiev.zoral.com.ua> References: <7C3D15DD6E8F464998CA1470D8A322F302BB9F72@ES02CO.wgti.net> <20100707205041.GO13238@deviant.kiev.zoral.com.ua> <4C358843.5000001@freebsd.org> <4C358A01.8080206@icyb.net.ua> <20100708082943.GB2439@deviant.kiev.zoral.com.ua> <4C358EE5.8070003@icyb.net.ua> <20100708094255.GA2408@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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;
--
Andre
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4C35B7C4.40507>