kern/123095 kern/131602 sendfile

[Andre Oppermann]

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