Netdev List
 help / color / mirror / Atom feed
* Re: [UDP6]: Restore sk_filter optimisation
From: Herbert Xu @ 2007-10-31 14:42 UTC (permalink / raw)
  To: Mitsuru Chinen; +Cc: David S. Miller, netdev, YOSHIFUJI Hideaki
In-Reply-To: <20071031230545.9be711d4.mitch@linux.vnet.ibm.com>

On Wed, Oct 31, 2007 at 11:05:45PM +0900, Mitsuru Chinen wrote:
>
> > >  1. udp6InDatagrams is incremented instead of udpInErrors
> > >  2. In userland, recvfrom() replies an error with EAGAIN.
> > >     recvfrom() wasn't aware of such a packet before.
> > > 
> > > Are these changes intentional?
>
> As far as I tested, this doesn't happen with the old code even if
> a filter is attached. However, this happen with the new code
> without a filter and I don't see this rather when a filter is
> attached. So, I'm afraid it's new.

Sorry, I read the patch the wrong way around :)

1) is just an accounting issue.  It shouldn't be too difficult
to fix it up.  In fact, I think udpInErrors will still be
incremented once we detect the error.

2) shouldn't be an issue because we've already solved the
problem by making poll/select do the checksum verification
before indiciating that the socket is readable.

> > And, we're not sure how much the "optimization"'s benefit is.
> > It is even worse when we are hand

The checksum verification is costly because we have to bring
the payload into cache.  Since filters are very rare it's
worthwhile to postpone the checksum verification for the common
case.

Also as a general rule, we want to avoid divergent behaviour
between IPv4 and IPv6.  So for changes like this we should
really modify both stacks in future rather than have each
stack do its own thing.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 00/33] Swap over NFS -v14
From: Byron Stanoszek @ 2007-10-31 14:03 UTC (permalink / raw)
  To: Nick Piggin
  Cc: David Miller, a.p.zijlstra, torvalds, akpm, linux-kernel,
	linux-mm, netdev, trond.myklebust
In-Reply-To: <200710311504.24016.nickpiggin@yahoo.com.au>

On Wed, 31 Oct 2007, Nick Piggin wrote:

> On Wednesday 31 October 2007 15:37, David Miller wrote:
>> From: Nick Piggin <nickpiggin@yahoo.com.au>
>> Date: Wed, 31 Oct 2007 14:26:32 +1100
>>
>>> Is it really worth all the added complexity of making swap
>>> over NFS files work, given that you could use a network block
>>> device instead?
>>
>> Don't be misled.  Swapping over NFS is just a scarecrow for the
>> seemingly real impetus behind these changes which is network storage
>> stuff like iSCSI.
>
> Oh, I'm OK with the network reserves stuff (not the actual patch,
> which I'm not really qualified to review, but at least the idea
> of it...).
>
> And also I'm not as such against the idea of swap over network.
>
> However, specifically the change to make swapfiles work through
> the filesystem layer (ATM it goes straight to the block layer,
> modulo some initialisation stuff which uses block filesystem-
> specific calls).
>
> I mean, I assume that anybody trying to swap over network *today*
> has to be using a network block device anyway, so the idea of
> just being able to transparently improve that case seems better
> than adding new complexities for seemingly not much gain.

I have some embedded diskless devices that have 16 MB of RAM and >500MB of
swap. Its root fs and swap device are both done over NBD because NFS is too
expensive in 16MB of RAM. Any memory contention (i.e needing memory to swap
memory over the network), however infrequent, causes the system to freeze when
about 50 MB of VM is used up. I would love to see some work done in this area.

  -Byron

--
Byron Stanoszek                         Ph: (330) 644-3059
Systems Programmer                      Fax: (330) 644-8110
Commercial Timesharing Inc.             Email: byron@comtime.com

^ permalink raw reply

* Re: [PATCH 0/8] Cleanup/fix the sk_alloc() call
From: Arnaldo Carvalho de Melo @ 2007-10-31 14:14 UTC (permalink / raw)
  To: Pavel Emelyanov
  Cc: Arnaldo Carvalho de Melo, David Miller, Linux Netdev List, devel
In-Reply-To: <472891F4.3050304@openvz.org>

Em Wed, Oct 31, 2007 at 05:32:20PM +0300, Pavel Emelyanov escreveu:
> Arnaldo Carvalho de Melo wrote:
> > Em Wed, Oct 31, 2007 at 04:40:01PM +0300, Pavel Emelyanov escreveu:
> >> The sk_alloc() function suffers from two problems:
> >> 1 (major). The error path is not clean in it - if the security
> >>    call fails, the net namespace is not put, if the try_module_get
> >>    fails  additionally the security context is not released;
> >> 2 (minor). The zero_it argument is misleading, as it doesn't just 
> >>    zeroes it, but performs some extra setup. Besides this argument 
> >>    is used only in one place - in the sk_clone().
> >>
> >> So this set fixes these problems and performs some additional
> >> cleanup.
> >>
> >> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
> > 
> > for the series:
> > 
> > Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> Thanks a lot :)
> 
> > Haven't tested, but it looks straightforward and conceptually sound,
> > thanks for improving the sk_prot infrastructure! :-)
> 
> > Now we have just to make all the other protocols fill in the missing
> > sk->sk_prot-> methods (converting what is there now in socket->ops) so
> > that we can kill socket->ops and eliminate one level of indirection :-P
> 
> Do I get your idea right, that having the 'struct sock->ops' field is not
> that good and the long-term TODO is to remove it (or smth similar)? Can you, 
> please, pour some more light on this, because I'm not yet very common with 
> the networking code, but I'm trying to learn it better by fixing obvious 
> bugs and cleaning the code.

Start here:

const struct proto_ops inet_stream_ops = {
        .family            = PF_INET,
        .owner             = THIS_MODULE,
        .release           = inet_release,
        .bind              = inet_bind,
        .connect           = inet_stream_connect,
        .socketpair        = sock_no_socketpair,
        .accept            = inet_accept,
        .getname           = inet_getname,
        .poll              = tcp_poll,
        .ioctl             = inet_ioctl,
        .listen            = inet_listen,
        .shutdown          = inet_shutdown,
        .setsockopt        = sock_common_setsockopt,
        .getsockopt        = sock_common_getsockopt,
        .sendmsg           = tcp_sendmsg,
        .recvmsg           = sock_common_recvmsg,
        .mmap              = sock_no_mmap,
        .sendpage          = tcp_sendpage,
#ifdef CONFIG_COMPAT
        .compat_setsockopt = compat_sock_common_setsockopt,
        .compat_getsockopt = compat_sock_common_getsockopt,
#endif
};

Now look at all the "*_common_*" stuff, for instance:

int sock_common_recvmsg(struct kiocb *iocb, struct socket *sock,
                        struct msghdr *msg, size_t size, int flags)
{
        struct sock *sk = sock->sk;
        int addr_len = 0;
        int err;

        err = sk->sk_prot->recvmsg(iocb, sk, msg, size, flags & MSG_DONTWAIT,
                                   flags & ~MSG_DONTWAIT, &addr_len);
        if (err >= 0)
                msg->msg_namelen = addr_len;
        return err;
}

So if we made all protocols implement sk->sk_prot_recvmsg... got it?

And then look at the inet_* routines above, at least for LLC I was using
several unmodified.

Over the years the quality work is done on the mainstream protocols,
with the legacy ones lagging behind, so the more we share...

Anyway, look at my paper about it:

http://www.linuxsymposium.org/proceedings/reprints/Reprint-Melo-OLS2004.pdf

The DCCP paper also talks about this:

http://www.linuxinsight.com/files/ols2005/melo-reprint.pdf

- Arnaldo

^ permalink raw reply

* af_packet.c flush_dcache_page
From: Patrick McHardy @ 2007-10-31 14:08 UTC (permalink / raw)
  To: Linux Netdev List

I'm currently adding mmap support to af_netlink based on the
af_packet implementation and I'm wondering about this code in
tpacket_rcv():

         h->tp_status = status;
         smp_mb();

         {
                 struct page *p_start, *p_end;
                 u8 *h_end = (u8 *)h + macoff + snaplen - 1;

                 p_start = virt_to_page(h);
                 p_end = virt_to_page(h_end);
                 while (p_start <= p_end) {
                         flush_dcache_page(p_start);
                         p_start++;
                 }
         }

Shouldn't the flushing be done in reverse order to make sure
that the page containing tp_status is flushed last and userspace
doesn't start looking at following pages before all dcache entries
are flushed?

A related question: Documentation/cachetlb.txt mentions that
flushing also needs to be done for reading of shared+writable
mapped pages, so it seems like we also need to call flush_dcache_page 
before the tp_status check earlier in that function and packet_poll().

^ permalink raw reply

* Re: [UDP6]: Restore sk_filter optimisation
From: Mitsuru Chinen @ 2007-10-31 14:05 UTC (permalink / raw)
  To: Herbert Xu; +Cc: David S. Miller, netdev, YOSHIFUJI Hideaki
In-Reply-To: <20071029125328.GA5453@gondor.apana.org.au>

On Mon, 29 Oct 2007 20:53:28 +0800
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Mon, Oct 29, 2007 at 03:33:20PM +0900, Mitsuru Chinen wrote:
> > Hello Herbert,
> > 
> > Let me ask a question about this patch.
> > After this patch was applied, 2 of the protocol stack behaviors were
> > changed when it receives a UDP datagram with broken checksum:
> > 
> >  1. udp6InDatagrams is incremented instead of udpInErrors
> >  2. In userland, recvfrom() replies an error with EAGAIN.
> >     recvfrom() wasn't aware of such a packet before.
> > 
> > Are these changes intentional?
> 
> It wasn't my intention if that's what you mean :)
> 
> However, this would've happened with the old code anyway if
> someone had a filter attached so this isn't new.
>
> If it's a problem then we should just get it fixed.

As far as I tested, this doesn't happen with the old code even if
a filter is attached. However, this happen with the new code
without a filter and I don't see this rather when a filter is
attached. So, I'm afraid it's new.

By the way, could you answer the Yoshifuji-san's question?
I think the code where we should fix depends on this. 

On Mon, 29 Oct 2007 15:41:50 +0900 (JST)
YOSHIFUJI Hideaki / 吉藤英明 <yoshfuji@linux-ipv6.org> wrote:

> And, we're not sure how much the "optimization"'s benefit is.
> It is even worse when we are handling multicast packets.

Thank you,
----
Mitsuru Chinen <mitch@linux.vnet.ibm.com>

^ permalink raw reply

* Re: [PATCH]: Fix networking scatterlist regressions.
From: Jens Axboe @ 2007-10-31 13:46 UTC (permalink / raw)
  To: Herbert Xu; +Cc: David Miller, netdev, rusty
In-Reply-To: <20071031134518.GA27412@gondor.apana.org.au>

On Wed, Oct 31 2007, Herbert Xu wrote:
> On Tue, Oct 30, 2007 at 08:40:02PM -0700, David Miller wrote:
> > 
> > I just checked the following bug fix into net-2.6
> 
> Thanks for getting to the bottom of this Dave! I seem to have
> mistaken the = for a |= in sg_mark_end :)

I don't blame you, that function was definitely non-intuitive!

-- 
Jens Axboe


^ permalink raw reply

* Re: [PATCH]: Fix networking scatterlist regressions.
From: Herbert Xu @ 2007-10-31 13:45 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, rusty, jens.axboe
In-Reply-To: <20071030.204002.193703502.davem@davemloft.net>

On Tue, Oct 30, 2007 at 08:40:02PM -0700, David Miller wrote:
> 
> I just checked the following bug fix into net-2.6

Thanks for getting to the bottom of this Dave! I seem to have
mistaken the = for a |= in sg_mark_end :)

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 00/33] Swap over NFS -v14
From: Gregory Haskins @ 2007-10-31 13:44 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Jeff Garzik, Nick Piggin, Linus Torvalds, Andrew Morton,
	linux-kernel, linux-mm, netdev, trond.myklebust
In-Reply-To: <1193835413.27652.205.camel@twins>

Peter Zijlstra wrote:

> 
> But please, people who want this (I'm sure some of you are reading) do
> speak up. I'm just the motivated corporate drone implementing the
> feature :-)

FWIW, I could have used a "swap to network technology X" like system at
my last job.  We were building a large networking switch with blades,
and the IO cards didn't have anywhere near the resources that the
control modules had (no persistent storage, small ram, etc).  We were
already doing userspace coredumps over NFS to the control cards.  It
would have been nice to swap as well.


^ permalink raw reply

* Re: [PATCH 0/8] Cleanup/fix the sk_alloc() call
From: Pavel Emelyanov @ 2007-10-31 14:32 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: David Miller, Linux Netdev List, devel
In-Reply-To: <20071031131505.GD3962@ghostprotocols.net>

Arnaldo Carvalho de Melo wrote:
> Em Wed, Oct 31, 2007 at 04:40:01PM +0300, Pavel Emelyanov escreveu:
>> The sk_alloc() function suffers from two problems:
>> 1 (major). The error path is not clean in it - if the security
>>    call fails, the net namespace is not put, if the try_module_get
>>    fails  additionally the security context is not released;
>> 2 (minor). The zero_it argument is misleading, as it doesn't just 
>>    zeroes it, but performs some extra setup. Besides this argument 
>>    is used only in one place - in the sk_clone().
>>
>> So this set fixes these problems and performs some additional
>> cleanup.
>>
>> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
> 
> for the series:
> 
> Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Thanks a lot :)

> Haven't tested, but it looks straightforward and conceptually sound,
> thanks for improving the sk_prot infrastructure! :-)

> Now we have just to make all the other protocols fill in the missing
> sk->sk_prot-> methods (converting what is there now in socket->ops) so
> that we can kill socket->ops and eliminate one level of indirection :-P

Do I get your idea right, that having the 'struct sock->ops' field is not
that good and the long-term TODO is to remove it (or smth similar)? Can you, 
please, pour some more light on this, because I'm not yet very common with 
the networking code, but I'm trying to learn it better by fixing obvious 
bugs and cleaning the code.

> - Arnaldo

Thanks,
Pavel

^ permalink raw reply

* Re: [PATCH 00/33] Swap over NFS -v14
From: Arnaldo Carvalho de Melo @ 2007-10-31 13:18 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Jeff Garzik, Nick Piggin, Linus Torvalds, Andrew Morton,
	linux-kernel, linux-mm, netdev, trond.myklebust
In-Reply-To: <1193835413.27652.205.camel@twins>

Em Wed, Oct 31, 2007 at 01:56:53PM +0100, Peter Zijlstra escreveu:
> On Wed, 2007-10-31 at 08:16 -0400, Jeff Garzik wrote:
> > Thoughts:
> > 
> > 1) I absolutely agree that NFS is far more prominent and useful than any 
> > network block device, at the present time.
> > 
> > 
> > 2) Nonetheless, swap over NFS is a pretty rare case.  I view this work 
> > as interesting, but I really don't see a huge need, for swapping over 
> > NBD or swapping over NFS.  I tend to think swapping to a remote resource 
> > starts to approach "migration" rather than merely swapping.  Yes, we can 
> > do it...  but given the lack of burning need one must examine the price.
> 
> There is a large corporate demand for this, which is why I'm doing this.
> 
> The typical usage scenarios are:
>  - cluster/blades, where having local disks is a cost issue (maintenance
>    of failures, heat, etc)
>  - virtualisation, where dumping the storage on a networked storage unit
>    makes for trivial migration and what not..
> 
> But please, people who want this (I'm sure some of you are reading) do
> speak up. I'm just the motivated corporate drone implementing the
> feature :-)

Keep it up, Dave already mentioned iSCSI, there is AoE, there are RT
sockets, you name it, the networking bits we've talked about several
times, they look OK, so I'm sorry for not going over all of them in
detail, but you have my support neverthless.

- Arnaldo

^ permalink raw reply

* Re: [PATCH 0/8] Cleanup/fix the sk_alloc() call
From: Arnaldo Carvalho de Melo @ 2007-10-31 13:15 UTC (permalink / raw)
  To: Pavel Emelyanov; +Cc: David Miller, Linux Netdev List, devel
In-Reply-To: <472885B1.7090103@openvz.org>

Em Wed, Oct 31, 2007 at 04:40:01PM +0300, Pavel Emelyanov escreveu:
> The sk_alloc() function suffers from two problems:
> 1 (major). The error path is not clean in it - if the security
>    call fails, the net namespace is not put, if the try_module_get
>    fails  additionally the security context is not released;
> 2 (minor). The zero_it argument is misleading, as it doesn't just 
>    zeroes it, but performs some extra setup. Besides this argument 
>    is used only in one place - in the sk_clone().
> 
> So this set fixes these problems and performs some additional
> cleanup.
> 
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

for the series:

Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Haven't tested, but it looks straightforward and conceptually sound,
thanks for improving the sk_prot infrastructure! :-)

Now we have just to make all the other protocols fill in the missing
sk->sk_prot-> methods (converting what is there now in socket->ops) so
that we can kill socket->ops and eliminate one level of indirection :-P

- Arnaldo

^ permalink raw reply

* Re: [PATCH 03/33] mm: slub: add knowledge of reserve pages
From: Peter Zijlstra @ 2007-10-31 13:08 UTC (permalink / raw)
  To: Nick Piggin
  Cc: Linus Torvalds, Andrew Morton, linux-kernel, linux-mm, netdev,
	trond.myklebust
In-Reply-To: <1193835258.27652.199.camel@twins>

On Wed, 2007-10-31 at 13:54 +0100, Peter Zijlstra wrote:
> On Wed, 2007-10-31 at 22:25 +1100, Nick Piggin wrote:

> > What I'm saying is that the slab allocator slowpath should always
> > just check watermarks against the current task. Instead of this
> > ->reserve stuff.
> 
> So what you say is to allocate a slab every time we take the slow path,
> even when we already have one?

BTW, a task that does not have reserve permissions will already attempt
to allocate a new slab - this is done to probe the current watermarks.
If this succeeds the reserve status is lifted.



^ permalink raw reply

* Re: [PATCH 06/33] mm: allow PF_MEMALLOC from softirq context
From: Peter Zijlstra @ 2007-10-31 13:06 UTC (permalink / raw)
  To: Nick Piggin
  Cc: Linus Torvalds, Andrew Morton, linux-kernel, linux-mm, netdev,
	trond.myklebust
In-Reply-To: <200710312149.25296.nickpiggin@yahoo.com.au>

[-- Attachment #1: Type: text/plain, Size: 1195 bytes --]

On Wed, 2007-10-31 at 21:49 +1100, Nick Piggin wrote:
> On Wednesday 31 October 2007 21:42, Peter Zijlstra wrote:
> > On Wed, 2007-10-31 at 14:51 +1100, Nick Piggin wrote:
> > > On Wednesday 31 October 2007 03:04, Peter Zijlstra wrote:
> > > > Allow PF_MEMALLOC to be set in softirq context. When running softirqs
> > > > from a borrowed context save current->flags, ksoftirqd will have its
> > > > own task_struct.
> > >
> > > What's this for? Why would ksoftirqd pick up PF_MEMALLOC? (I guess
> > > that some networking thing must be picking it up in a subsequent patch,
> > > but I'm too lazy to look!)... Again, can you have more of a rationale in
> > > your patch headers, or ref the patch that uses it... thanks
> >
> > Right, I knew I was forgetting something in these changelogs.
> >
> > The network stack does quite a bit of packet processing from softirq
> > context. Once you start swapping over network, some of the packets want
> > to be processed under PF_MEMALLOC.
> 
> Hmm... what about processing from interrupt context?

From what I could tell that is not done, ISR just fills the skb and
sticks it on an RX queue to be further processed by the softirq.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH 8/8] Forget the zero_it argument of sk_alloc()
From: Pavel Emelyanov @ 2007-10-31 13:59 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List, devel
In-Reply-To: <472885B1.7090103@openvz.org>

Finally, the zero_it argument can be completely removed from
the callers and from the function prototype.

Besides, fix the checkpatch.pl warnings about using the
assignments inside if-s.

This patch is rather big, and it is a part of the previous one.
I splitted it wishing to make the patches more readable. Hope 
this particular split helped.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/drivers/net/pppoe.c b/drivers/net/pppoe.c
index 8936ed3..a005d8f 100644
--- a/drivers/net/pppoe.c
+++ b/drivers/net/pppoe.c
@@ -491,7 +491,7 @@ static int pppoe_create(struct net *net, struct socket *sock)
 	int error = -ENOMEM;
 	struct sock *sk;
 
-	sk = sk_alloc(net, PF_PPPOX, GFP_KERNEL, &pppoe_sk_proto, 1);
+	sk = sk_alloc(net, PF_PPPOX, GFP_KERNEL, &pppoe_sk_proto);
 	if (!sk)
 		goto out;
 
diff --git a/drivers/net/pppol2tp.c b/drivers/net/pppol2tp.c
index 921d4ef..f8904fd 100644
--- a/drivers/net/pppol2tp.c
+++ b/drivers/net/pppol2tp.c
@@ -1416,7 +1416,7 @@ static int pppol2tp_create(struct net *net, struct socket *sock)
 	int error = -ENOMEM;
 	struct sock *sk;
 
-	sk = sk_alloc(net, PF_PPPOX, GFP_KERNEL, &pppol2tp_sk_proto, 1);
+	sk = sk_alloc(net, PF_PPPOX, GFP_KERNEL, &pppol2tp_sk_proto);
 	if (!sk)
 		goto out;
 
diff --git a/include/net/sock.h b/include/net/sock.h
index ecad7b4..20de3fa 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -779,7 +779,7 @@ extern void FASTCALL(release_sock(struct sock *sk));
 
 extern struct sock		*sk_alloc(struct net *net, int family,
 					  gfp_t priority,
-					  struct proto *prot, int zero_it);
+					  struct proto *prot);
 extern void			sk_free(struct sock *sk);
 extern struct sock		*sk_clone(const struct sock *sk,
 					  const gfp_t priority);
diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
index 7c0b515..e0d37d6 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -1044,7 +1044,7 @@ static int atalk_create(struct net *net, struct socket *sock, int protocol)
 	if (sock->type != SOCK_RAW && sock->type != SOCK_DGRAM)
 		goto out;
 	rc = -ENOMEM;
-	sk = sk_alloc(net, PF_APPLETALK, GFP_KERNEL, &ddp_proto, 1);
+	sk = sk_alloc(net, PF_APPLETALK, GFP_KERNEL, &ddp_proto);
 	if (!sk)
 		goto out;
 	rc = 0;
diff --git a/net/atm/common.c b/net/atm/common.c
index e166d9e..eba09a0 100644
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -133,7 +133,7 @@ int vcc_create(struct net *net, struct socket *sock, int protocol, int family)
 	sock->sk = NULL;
 	if (sock->type == SOCK_STREAM)
 		return -EINVAL;
-	sk = sk_alloc(net, family, GFP_KERNEL, &vcc_proto, 1);
+	sk = sk_alloc(net, family, GFP_KERNEL, &vcc_proto);
 	if (!sk)
 		return -ENOMEM;
 	sock_init_data(sock, sk);
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 993e5c7..8378afd 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -836,7 +836,8 @@ static int ax25_create(struct net *net, struct socket *sock, int protocol)
 		return -ESOCKTNOSUPPORT;
 	}
 
-	if ((sk = sk_alloc(net, PF_AX25, GFP_ATOMIC, &ax25_proto, 1)) == NULL)
+	sk = sk_alloc(net, PF_AX25, GFP_ATOMIC, &ax25_proto);
+	if (sk == NULL)
 		return -ENOMEM;
 
 	ax25 = sk->sk_protinfo = ax25_create_cb();
@@ -861,7 +862,8 @@ struct sock *ax25_make_new(struct sock *osk, struct ax25_dev *ax25_dev)
 	struct sock *sk;
 	ax25_cb *ax25, *oax25;
 
-	if ((sk = sk_alloc(osk->sk_net, PF_AX25, GFP_ATOMIC, osk->sk_prot, 1)) == NULL)
+	sk = sk_alloc(osk->sk_net, PF_AX25, GFP_ATOMIC,	osk->sk_prot);
+	if (sk == NULL)
 		return NULL;
 
 	if ((ax25 = ax25_create_cb()) == NULL) {
diff --git a/net/bluetooth/bnep/sock.c b/net/bluetooth/bnep/sock.c
index f718965..9ebd3c6 100644
--- a/net/bluetooth/bnep/sock.c
+++ b/net/bluetooth/bnep/sock.c
@@ -213,7 +213,7 @@ static int bnep_sock_create(struct net *net, struct socket *sock, int protocol)
 	if (sock->type != SOCK_RAW)
 		return -ESOCKTNOSUPPORT;
 
-	sk = sk_alloc(net, PF_BLUETOOTH, GFP_ATOMIC, &bnep_proto, 1);
+	sk = sk_alloc(net, PF_BLUETOOTH, GFP_ATOMIC, &bnep_proto);
 	if (!sk)
 		return -ENOMEM;
 
diff --git a/net/bluetooth/cmtp/sock.c b/net/bluetooth/cmtp/sock.c
index cf700c2..783edab 100644
--- a/net/bluetooth/cmtp/sock.c
+++ b/net/bluetooth/cmtp/sock.c
@@ -204,7 +204,7 @@ static int cmtp_sock_create(struct net *net, struct socket *sock, int protocol)
 	if (sock->type != SOCK_RAW)
 		return -ESOCKTNOSUPPORT;
 
-	sk = sk_alloc(net, PF_BLUETOOTH, GFP_ATOMIC, &cmtp_proto, 1);
+	sk = sk_alloc(net, PF_BLUETOOTH, GFP_ATOMIC, &cmtp_proto);
 	if (!sk)
 		return -ENOMEM;
 
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 8825102..1499132 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -645,7 +645,7 @@ static int hci_sock_create(struct net *net, struct socket *sock, int protocol)
 
 	sock->ops = &hci_sock_ops;
 
-	sk = sk_alloc(net, PF_BLUETOOTH, GFP_ATOMIC, &hci_sk_proto, 1);
+	sk = sk_alloc(net, PF_BLUETOOTH, GFP_ATOMIC, &hci_sk_proto);
 	if (!sk)
 		return -ENOMEM;
 
diff --git a/net/bluetooth/hidp/sock.c b/net/bluetooth/hidp/sock.c
index 1de2b6f..3292b95 100644
--- a/net/bluetooth/hidp/sock.c
+++ b/net/bluetooth/hidp/sock.c
@@ -255,7 +255,7 @@ static int hidp_sock_create(struct net *net, struct socket *sock, int protocol)
 	if (sock->type != SOCK_RAW)
 		return -ESOCKTNOSUPPORT;
 
-	sk = sk_alloc(net, PF_BLUETOOTH, GFP_ATOMIC, &hidp_proto, 1);
+	sk = sk_alloc(net, PF_BLUETOOTH, GFP_ATOMIC, &hidp_proto);
 	if (!sk)
 		return -ENOMEM;
 
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 6fbbae7..477e052 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -607,7 +607,7 @@ static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, int p
 {
 	struct sock *sk;
 
-	sk = sk_alloc(net, PF_BLUETOOTH, prio, &l2cap_proto, 1);
+	sk = sk_alloc(net, PF_BLUETOOTH, prio, &l2cap_proto);
 	if (!sk)
 		return NULL;
 
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 266b697..c46d510 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -287,7 +287,7 @@ static struct sock *rfcomm_sock_alloc(struct net *net, struct socket *sock, int
 	struct rfcomm_dlc *d;
 	struct sock *sk;
 
-	sk = sk_alloc(net, PF_BLUETOOTH, prio, &rfcomm_proto, 1);
+	sk = sk_alloc(net, PF_BLUETOOTH, prio, &rfcomm_proto);
 	if (!sk)
 		return NULL;
 
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 82d0dfd..93ad1aa 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -421,7 +421,7 @@ static struct sock *sco_sock_alloc(struct net *net, struct socket *sock, int pro
 {
 	struct sock *sk;
 
-	sk = sk_alloc(net, PF_BLUETOOTH, prio, &sco_proto, 1);
+	sk = sk_alloc(net, PF_BLUETOOTH, prio, &sco_proto);
 	if (!sk)
 		return NULL;
 
diff --git a/net/core/sock.c b/net/core/sock.c
index 6046fc6..12ad206 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -927,7 +927,7 @@ static void sk_prot_free(struct proto *prot, struct sock *sk)
  *	@zero_it: if we should zero the newly allocated sock
  */
 struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
-		      struct proto *prot, int zero_it)
+		      struct proto *prot)
 {
 	struct sock *sk;
 
diff --git a/net/decnet/af_decnet.c b/net/decnet/af_decnet.c
index aabe98d..57d5749 100644
--- a/net/decnet/af_decnet.c
+++ b/net/decnet/af_decnet.c
@@ -474,7 +474,7 @@ static struct proto dn_proto = {
 static struct sock *dn_alloc_sock(struct net *net, struct socket *sock, gfp_t gfp)
 {
 	struct dn_scp *scp;
-	struct sock *sk = sk_alloc(net, PF_DECnet, gfp, &dn_proto, 1);
+	struct sock *sk = sk_alloc(net, PF_DECnet, gfp, &dn_proto);
 
 	if  (!sk)
 		goto out;
diff --git a/net/econet/af_econet.c b/net/econet/af_econet.c
index 9cae16b..f70df07 100644
--- a/net/econet/af_econet.c
+++ b/net/econet/af_econet.c
@@ -624,7 +624,7 @@ static int econet_create(struct net *net, struct socket *sock, int protocol)
 	sock->state = SS_UNCONNECTED;
 
 	err = -ENOBUFS;
-	sk = sk_alloc(net, PF_ECONET, GFP_KERNEL, &econet_proto, 1);
+	sk = sk_alloc(net, PF_ECONET, GFP_KERNEL, &econet_proto);
 	if (sk == NULL)
 		goto out;
 
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 621b128..d2f22e7 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -323,7 +323,7 @@ lookup_protocol:
 	BUG_TRAP(answer_prot->slab != NULL);
 
 	err = -ENOBUFS;
-	sk = sk_alloc(net, PF_INET, GFP_KERNEL, answer_prot, 1);
+	sk = sk_alloc(net, PF_INET, GFP_KERNEL, answer_prot);
 	if (sk == NULL)
 		goto out;
 
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 1b1caf3..ecbd388 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -162,7 +162,7 @@ lookup_protocol:
 	BUG_TRAP(answer_prot->slab != NULL);
 
 	err = -ENOBUFS;
-	sk = sk_alloc(net, PF_INET6, GFP_KERNEL, answer_prot, 1);
+	sk = sk_alloc(net, PF_INET6, GFP_KERNEL, answer_prot);
 	if (sk == NULL)
 		goto out;
 
diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
index 29b063d..a195a66 100644
--- a/net/ipx/af_ipx.c
+++ b/net/ipx/af_ipx.c
@@ -1381,7 +1381,7 @@ static int ipx_create(struct net *net, struct socket *sock, int protocol)
 		goto out;
 
 	rc = -ENOMEM;
-	sk = sk_alloc(net, PF_IPX, GFP_KERNEL, &ipx_proto, 1);
+	sk = sk_alloc(net, PF_IPX, GFP_KERNEL, &ipx_proto);
 	if (!sk)
 		goto out;
 #ifdef IPX_REFCNT_DEBUG
diff --git a/net/irda/af_irda.c b/net/irda/af_irda.c
index 0328ae2..48ce59a 100644
--- a/net/irda/af_irda.c
+++ b/net/irda/af_irda.c
@@ -1078,7 +1078,7 @@ static int irda_create(struct net *net, struct socket *sock, int protocol)
 	}
 
 	/* Allocate networking socket */
-	sk = sk_alloc(net, PF_IRDA, GFP_ATOMIC, &irda_proto, 1);
+	sk = sk_alloc(net, PF_IRDA, GFP_ATOMIC, &irda_proto);
 	if (sk == NULL)
 		return -ENOMEM;
 
diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index 43e01c8..aef6645 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -216,7 +216,7 @@ static struct sock *iucv_sock_alloc(struct socket *sock, int proto, gfp_t prio)
 {
 	struct sock *sk;
 
-	sk = sk_alloc(&init_net, PF_IUCV, prio, &iucv_proto, 1);
+	sk = sk_alloc(&init_net, PF_IUCV, prio, &iucv_proto);
 	if (!sk)
 		return NULL;
 
diff --git a/net/key/af_key.c b/net/key/af_key.c
index 266f112..10c89d4 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -152,7 +152,7 @@ static int pfkey_create(struct net *net, struct socket *sock, int protocol)
 		return -EPROTONOSUPPORT;
 
 	err = -ENOMEM;
-	sk = sk_alloc(net, PF_KEY, GFP_KERNEL, &key_proto, 1);
+	sk = sk_alloc(net, PF_KEY, GFP_KERNEL, &key_proto);
 	if (sk == NULL)
 		goto out;
 
diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
index 8ebc276..5c0b484 100644
--- a/net/llc/llc_conn.c
+++ b/net/llc/llc_conn.c
@@ -869,7 +869,7 @@ static void llc_sk_init(struct sock* sk)
  */
 struct sock *llc_sk_alloc(struct net *net, int family, gfp_t priority, struct proto *prot)
 {
-	struct sock *sk = sk_alloc(net, family, priority, prot, 1);
+	struct sock *sk = sk_alloc(net, family, priority, prot);
 
 	if (!sk)
 		goto out;
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 4f994c0..2601712 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -396,7 +396,7 @@ static int __netlink_create(struct net *net, struct socket *sock,
 
 	sock->ops = &netlink_ops;
 
-	sk = sk_alloc(net, PF_NETLINK, GFP_KERNEL, &netlink_proto, 1);
+	sk = sk_alloc(net, PF_NETLINK, GFP_KERNEL, &netlink_proto);
 	if (!sk)
 		return -ENOMEM;
 
diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
index 3a4d479..972250c 100644
--- a/net/netrom/af_netrom.c
+++ b/net/netrom/af_netrom.c
@@ -423,7 +423,8 @@ static int nr_create(struct net *net, struct socket *sock, int protocol)
 	if (sock->type != SOCK_SEQPACKET || protocol != 0)
 		return -ESOCKTNOSUPPORT;
 
-	if ((sk = sk_alloc(net, PF_NETROM, GFP_ATOMIC, &nr_proto, 1)) == NULL)
+	sk = sk_alloc(net, PF_NETROM, GFP_ATOMIC, &nr_proto);
+	if (sk  == NULL)
 		return -ENOMEM;
 
 	nr = nr_sk(sk);
@@ -465,7 +466,8 @@ static struct sock *nr_make_new(struct sock *osk)
 	if (osk->sk_type != SOCK_SEQPACKET)
 		return NULL;
 
-	if ((sk = sk_alloc(osk->sk_net, PF_NETROM, GFP_ATOMIC, osk->sk_prot, 1)) == NULL)
+	sk = sk_alloc(osk->sk_net, PF_NETROM, GFP_ATOMIC, osk->sk_prot);
+	if (sk == NULL)
 		return NULL;
 
 	nr = nr_sk(sk);
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index d093650..4cb2dfb 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -995,7 +995,7 @@ static int packet_create(struct net *net, struct socket *sock, int protocol)
 	sock->state = SS_UNCONNECTED;
 
 	err = -ENOBUFS;
-	sk = sk_alloc(net, PF_PACKET, GFP_KERNEL, &packet_proto, 1);
+	sk = sk_alloc(net, PF_PACKET, GFP_KERNEL, &packet_proto);
 	if (sk == NULL)
 		goto out;
 
diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
index 509defe..ed2d65c 100644
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c
@@ -513,7 +513,8 @@ static int rose_create(struct net *net, struct socket *sock, int protocol)
 	if (sock->type != SOCK_SEQPACKET || protocol != 0)
 		return -ESOCKTNOSUPPORT;
 
-	if ((sk = sk_alloc(net, PF_ROSE, GFP_ATOMIC, &rose_proto, 1)) == NULL)
+	sk = sk_alloc(net, PF_ROSE, GFP_ATOMIC, &rose_proto);
+	if (sk == NULL)
 		return -ENOMEM;
 
 	rose = rose_sk(sk);
@@ -551,7 +552,8 @@ static struct sock *rose_make_new(struct sock *osk)
 	if (osk->sk_type != SOCK_SEQPACKET)
 		return NULL;
 
-	if ((sk = sk_alloc(osk->sk_net, PF_ROSE, GFP_ATOMIC, &rose_proto, 1)) == NULL)
+	sk = sk_alloc(osk->sk_net, PF_ROSE, GFP_ATOMIC, &rose_proto);
+	if (sk == NULL)
 		return NULL;
 
 	rose = rose_sk(sk);
diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index c680017..d638945 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -627,7 +627,7 @@ static int rxrpc_create(struct net *net, struct socket *sock, int protocol)
 	sock->ops = &rxrpc_rpc_ops;
 	sock->state = SS_UNCONNECTED;
 
-	sk = sk_alloc(net, PF_RXRPC, GFP_KERNEL, &rxrpc_proto, 1);
+	sk = sk_alloc(net, PF_RXRPC, GFP_KERNEL, &rxrpc_proto);
 	if (!sk)
 		return -ENOMEM;
 
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index eb4deaf..7f31ff6 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -631,7 +631,7 @@ static struct sock *sctp_v6_create_accept_sk(struct sock *sk,
 	struct ipv6_pinfo *newnp, *np = inet6_sk(sk);
 	struct sctp6_sock *newsctp6sk;
 
-	newsk = sk_alloc(sk->sk_net, PF_INET6, GFP_KERNEL, sk->sk_prot, 1);
+	newsk = sk_alloc(sk->sk_net, PF_INET6, GFP_KERNEL, sk->sk_prot);
 	if (!newsk)
 		goto out;
 
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index f5cd96f..40c1a47 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -552,7 +552,8 @@ static struct sock *sctp_v4_create_accept_sk(struct sock *sk,
 {
 	struct inet_sock *inet = inet_sk(sk);
 	struct inet_sock *newinet;
-	struct sock *newsk = sk_alloc(sk->sk_net, PF_INET, GFP_KERNEL, sk->sk_prot, 1);
+	struct sock *newsk = sk_alloc(sk->sk_net, PF_INET, GFP_KERNEL,
+			sk->sk_prot);
 
 	if (!newsk)
 		goto out;
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index e36b4b5..6b79226 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -201,7 +201,7 @@ static int tipc_create(struct net *net, struct socket *sock, int protocol)
 		return -EPROTOTYPE;
 	}
 
-	sk = sk_alloc(net, AF_TIPC, GFP_KERNEL, &tipc_proto, 1);
+	sk = sk_alloc(net, AF_TIPC, GFP_KERNEL, &tipc_proto);
 	if (!sk) {
 		tipc_deleteport(ref);
 		return -ENOMEM;
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 9163ec5..515e7a6 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -602,7 +602,7 @@ static struct sock * unix_create1(struct net *net, struct socket *sock)
 	if (atomic_read(&unix_nr_socks) >= 2*get_max_files())
 		goto out;
 
-	sk = sk_alloc(net, PF_UNIX, GFP_KERNEL, &unix_proto, 1);
+	sk = sk_alloc(net, PF_UNIX, GFP_KERNEL, &unix_proto);
 	if (!sk)
 		goto out;
 
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index fc416f9..92cfe8e 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -472,7 +472,7 @@ static struct proto x25_proto = {
 static struct sock *x25_alloc_socket(struct net *net)
 {
 	struct x25_sock *x25;
-	struct sock *sk = sk_alloc(net, AF_X25, GFP_ATOMIC, &x25_proto, 1);
+	struct sock *sk = sk_alloc(net, AF_X25, GFP_ATOMIC, &x25_proto);
 
 	if (!sk)
 		goto out;

^ permalink raw reply related

* Re: [PATCH 00/33] Swap over NFS -v14
From: Peter Zijlstra @ 2007-10-31 12:56 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Nick Piggin, Linus Torvalds, Andrew Morton, linux-kernel,
	linux-mm, netdev, trond.myklebust
In-Reply-To: <47287220.8050804@garzik.org>

[-- Attachment #1: Type: text/plain, Size: 2911 bytes --]

On Wed, 2007-10-31 at 08:16 -0400, Jeff Garzik wrote:
> Thoughts:
> 
> 1) I absolutely agree that NFS is far more prominent and useful than any 
> network block device, at the present time.
> 
> 
> 2) Nonetheless, swap over NFS is a pretty rare case.  I view this work 
> as interesting, but I really don't see a huge need, for swapping over 
> NBD or swapping over NFS.  I tend to think swapping to a remote resource 
> starts to approach "migration" rather than merely swapping.  Yes, we can 
> do it...  but given the lack of burning need one must examine the price.

There is a large corporate demand for this, which is why I'm doing this.

The typical usage scenarios are:
 - cluster/blades, where having local disks is a cost issue (maintenance
   of failures, heat, etc)
 - virtualisation, where dumping the storage on a networked storage unit
   makes for trivial migration and what not..

But please, people who want this (I'm sure some of you are reading) do
speak up. I'm just the motivated corporate drone implementing the
feature :-)

> 3) You note
> > Swap over network has the problem that the network subsystem does not use fixed
> > sized allocations, but heavily relies on kmalloc(). This makes mempools
> > unusable.
> 
> True, but IMO there are mitigating factors that should be researched and 
> taken into account:
> 
> a) To give you some net driver background/history, most mainstream net 
> drivers were coded to allocate RX skbs of size 1538, under the theory 
> that they would all be allocating out of the same underlying slab cache. 
>   It would not be difficult to update a great many of the [non-jumbo] 
> cases to create a fixed size allocation pattern.

One issue that comes to mind is how to ensure we'd still overflow the
IP-reassembly buffers. Currently those are managed on the number of
bytes present, not the number of fragments.

One of the goals of my approach was to not rewrite the network subsystem
to accomodate this feature (and I hope I succeeded).

> b) Spare-time experiments and anecdotal evidence points to RX and TX skb 
> recycling as a potentially valuable area of research.  If you are able 
> to do something like that, then memory suddenly becomes a lot more 
> bounded and predictable.
> 
> 
> So my gut feeling is that taking a hard look at how net drivers function 
> in the field should give you a lot of good ideas that approach the 
> shared goal of making network memory allocations more predictable and 
> bounded.

Note that being bounded only comes from dropping most packets before
trying them to a socket. That is the crucial part of the RX path, to
receive all packets from the NIC (regardless their size) but to not pass
them on to the network stack - unless they belong to a 'special' socket
that promises undelayed processing.

Thanks for these ideas, I'll look into them.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH 03/33] mm: slub: add knowledge of reserve pages
From: Peter Zijlstra @ 2007-10-31 12:54 UTC (permalink / raw)
  To: Nick Piggin
  Cc: Linus Torvalds, Andrew Morton, linux-kernel, linux-mm, netdev,
	trond.myklebust
In-Reply-To: <200710312225.07249.nickpiggin@yahoo.com.au>

[-- Attachment #1: Type: text/plain, Size: 1028 bytes --]

On Wed, 2007-10-31 at 22:25 +1100, Nick Piggin wrote:
> On Wednesday 31 October 2007 23:17, Peter Zijlstra wrote:
> > On Wed, 2007-10-31 at 21:46 +1100, Nick Piggin wrote:
> 
> > > And I'd prevent these ones from doing so.
> > >
> > > Without keeping track of "reserve" pages, which doesn't feel
> > > too clean.
> >
> > The problem with that is that once a slab was allocated with the right
> > allocation context, anybody can get objects from these slabs.
> 
> [snip]
> 
> I understand that.
> 
> 
> > So we either reserve a page per object, which for 32 byte objects is a
> > large waste, or we stop anybody who doesn't have the right permissions
> > from obtaining objects. I took the latter approach.
> 
> What I'm saying is that the slab allocator slowpath should always
> just check watermarks against the current task. Instead of this
> ->reserve stuff.

So what you say is to allocate a slab every time we take the slow path,
even when we already have one?

That sounds rather sub-optimal.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH 7/8] Remove bogus zero_it argument from sk_alloc
From: Pavel Emelyanov @ 2007-10-31 13:56 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List, devel
In-Reply-To: <472885B1.7090103@openvz.org>

At this point nobody calls the sk_alloc(() with zero_it == 0,
so remove unneeded checks from it.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/net/core/sock.c b/net/core/sock.c
index c032f48..77575c3 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -931,21 +931,16 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
 {
 	struct sock *sk;
 
-	if (zero_it)
-		priority |= __GFP_ZERO;
-
-	sk = sk_prot_alloc(prot, priority, family);
+	sk = sk_prot_alloc(prot, priority | __GFP_ZERO, family);
 	if (sk) {
-		if (zero_it) {
-			sk->sk_family = family;
-			/*
-			 * See comment in struct sock definition to understand
-			 * why we need sk_prot_creator -acme
-			 */
-			sk->sk_prot = sk->sk_prot_creator = prot;
-			sock_lock_init(sk);
-			sk->sk_net = get_net(net);
-		}
+		sk->sk_family = family;
+		/*
+		 * See comment in struct sock definition to understand
+		 * why we need sk_prot_creator -acme
+		 */
+		sk->sk_prot = sk->sk_prot_creator = prot;
+		sock_lock_init(sk);
+		sk->sk_net = get_net(net);
 	}
 
 	return sk;
-- 
1.5.3.4


^ permalink raw reply related

* [PATCH 6/8] Make the sk_clone() lighter
From: Pavel Emelyanov @ 2007-10-31 13:54 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List, devel
In-Reply-To: <472885B1.7090103@openvz.org>

The sk_prot_alloc() already performs all the stuff needed by the
sk_clone(). Besides, the sk_prot_alloc() requires almost twice
less arguments than the sk_alloc() does, so call the sk_prot_alloc()
saving the stack a bit.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/net/core/sock.c b/net/core/sock.c
index e7537e4..c032f48 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -976,8 +976,9 @@ void sk_free(struct sock *sk)
 
 struct sock *sk_clone(const struct sock *sk, const gfp_t priority)
 {
-	struct sock *newsk = sk_alloc(sk->sk_net, sk->sk_family, priority, sk->sk_prot, 0);
-
+	struct sock *newsk;
+
+	newsk = sk_prot_alloc(sk->sk_prot, priority, sk->sk_family);
 	if (newsk != NULL) {
 		struct sk_filter *filter;
 
-- 
1.5.3.4


^ permalink raw reply related

* [PATCH 5/8] Move some core sock setup into sk_prot_alloc
From: Pavel Emelyanov @ 2007-10-31 13:51 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List, devel
In-Reply-To: <472885B1.7090103@openvz.org>

The security_sk_alloc() and the module_get is a part of the
object allocations - move it in the proper place.

Note, that since we do not reset the newly allocated sock
in the sk_alloc() (memset() is removed with the previous
patch) we can safely do this.

Also fix the error path in sk_prot_alloc() - release the security
context if needed.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/net/core/sock.c b/net/core/sock.c
index 21fc79b..e7537e4 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -870,7 +870,8 @@ static void sock_copy(struct sock *nsk, const struct sock *osk)
 #endif
 }
 
-static struct sock *sk_prot_alloc(struct proto *prot, gfp_t priority)
+static struct sock *sk_prot_alloc(struct proto *prot, gfp_t priority,
+		int family)
 {
 	struct sock *sk;
 	struct kmem_cache *slab;
@@ -881,18 +882,40 @@ static struct sock *sk_prot_alloc(struct proto *prot, gfp_t priority)
 	else
 		sk = kmalloc(prot->obj_size, priority);
 
+	if (sk != NULL) {
+		if (security_sk_alloc(sk, family, priority))
+			goto out_free;
+
+		if (!try_module_get(prot->owner))
+			goto out_free_sec;
+	}
+
 	return sk;
+
+out_free_sec:
+	security_sk_free(sk);
+out_free:
+	if (slab != NULL)
+		kmem_cache_free(slab, sk);
+	else
+		kfree(sk);
+	return NULL;
 }
 
 static void sk_prot_free(struct proto *prot, struct sock *sk)
 {
 	struct kmem_cache *slab;
-
+	struct module *owner;
+
+	owner = prot->owner;
 	slab = prot->slab;
+
+	security_sk_free(sk);
 	if (slab != NULL)
 		kmem_cache_free(slab, sk);
 	else
 		kfree(sk);
+	module_put(owner);
 }
 
 /**
@@ -911,7 +934,7 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
 	if (zero_it)
 		priority |= __GFP_ZERO;
 
-	sk = sk_prot_alloc(prot, priority);
+	sk = sk_prot_alloc(prot, priority, family);
 	if (sk) {
 		if (zero_it) {
 			sk->sk_family = family;
@@ -923,24 +946,14 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
 			sock_lock_init(sk);
 			sk->sk_net = get_net(net);
 		}
-
-		if (security_sk_alloc(sk, family, priority))
-			goto out_free;
-
-		if (!try_module_get(prot->owner))
-			goto out_free;
 	}
-	return sk;
 
-out_free:
-	sk_prot_free(prot, sk);
-	return NULL;
+	return sk;
 }
 
 void sk_free(struct sock *sk)
 {
 	struct sk_filter *filter;
-	struct module *owner = sk->sk_prot_creator->owner;
 
 	if (sk->sk_destruct)
 		sk->sk_destruct(sk);
@@ -957,10 +970,8 @@ void sk_free(struct sock *sk)
 		printk(KERN_DEBUG "%s: optmem leakage (%d bytes) detected.\n",
 		       __FUNCTION__, atomic_read(&sk->sk_omem_alloc));
 
-	security_sk_free(sk);
 	put_net(sk->sk_net);
 	sk_prot_free(sk->sk_prot_creator, sk);
-	module_put(owner);
 }
 
 struct sock *sk_clone(const struct sock *sk, const gfp_t priority)
-- 
1.5.3.4


^ permalink raw reply related

* [PATCH 4/8] Auto-zero the allocated sock object
From: Pavel Emelyanov @ 2007-10-31 13:48 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List, devel
In-Reply-To: <472885B1.7090103@openvz.org>

We have a __GFP_ZERO flag that allocates a zeroed chunk of memory.
Use it in the sk_alloc() and avoid a hand-made memset().

This is a temporary patch that will help us in the nearest future :)

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/net/core/sock.c b/net/core/sock.c
index 7c2e3db..21fc79b 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -908,10 +908,12 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
 {
 	struct sock *sk;
 
+	if (zero_it)
+		priority |= __GFP_ZERO;
+
 	sk = sk_prot_alloc(prot, priority);
 	if (sk) {
 		if (zero_it) {
-			memset(sk, 0, prot->obj_size);
 			sk->sk_family = family;
 			/*
 			 * See comment in struct sock definition to understand
-- 
1.5.3.4


^ permalink raw reply related

* [PATCH 3/8] Cleanup the allocation/freeing of the sock object
From: Pavel Emelyanov @ 2007-10-31 13:47 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List, devel
In-Reply-To: <472885B1.7090103@openvz.org>

The sock object is allocated either from the generic cache with
the kmalloc, or from the proc->slab cache.

Move this logic into an isolated set of helpers and make the
sk_alloc/sk_free look a bit nicer.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/net/core/sock.c b/net/core/sock.c
index 9c2dbfa..7c2e3db 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -870,6 +870,31 @@ static void sock_copy(struct sock *nsk, const struct sock *osk)
 #endif
 }
 
+static struct sock *sk_prot_alloc(struct proto *prot, gfp_t priority)
+{
+	struct sock *sk;
+	struct kmem_cache *slab;
+
+	slab = prot->slab;
+	if (slab != NULL)
+		sk = kmem_cache_alloc(slab, priority);
+	else
+		sk = kmalloc(prot->obj_size, priority);
+
+	return sk;
+}
+
+static void sk_prot_free(struct proto *prot, struct sock *sk)
+{
+	struct kmem_cache *slab;
+
+	slab = prot->slab;
+	if (slab != NULL)
+		kmem_cache_free(slab, sk);
+	else
+		kfree(sk);
+}
+
 /**
  *	sk_alloc - All socket objects are allocated here
  *	@net: the applicable net namespace
@@ -881,14 +906,9 @@ static void sock_copy(struct sock *nsk, const struct sock *osk)
 struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
 		      struct proto *prot, int zero_it)
 {
-	struct sock *sk = NULL;
-	struct kmem_cache *slab = prot->slab;
-
-	if (slab != NULL)
-		sk = kmem_cache_alloc(slab, priority);
-	else
-		sk = kmalloc(prot->obj_size, priority);
+	struct sock *sk;
 
+	sk = sk_prot_alloc(prot, priority);
 	if (sk) {
 		if (zero_it) {
 			memset(sk, 0, prot->obj_size);
@@ -911,10 +931,7 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
 	return sk;
 
 out_free:
-	if (slab != NULL)
-		kmem_cache_free(slab, sk);
-	else
-		kfree(sk);
+	sk_prot_free(prot, sk);
 	return NULL;
 }
 
@@ -940,10 +957,7 @@ void sk_free(struct sock *sk)
 
 	security_sk_free(sk);
 	put_net(sk->sk_net);
-	if (sk->sk_prot_creator->slab != NULL)
-		kmem_cache_free(sk->sk_prot_creator->slab, sk);
-	else
-		kfree(sk);
+	sk_prot_free(sk->sk_prot_creator, sk);
 	module_put(owner);
 }
 
-- 
1.5.3.4


^ permalink raw reply related

* Re: [PATCH 03/33] mm: slub: add knowledge of reserve pages
From: Nick Piggin @ 2007-10-31 11:25 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Linus Torvalds, Andrew Morton, linux-kernel, linux-mm, netdev,
	trond.myklebust
In-Reply-To: <1193833072.27652.167.camel@twins>

On Wednesday 31 October 2007 23:17, Peter Zijlstra wrote:
> On Wed, 2007-10-31 at 21:46 +1100, Nick Piggin wrote:

> > And I'd prevent these ones from doing so.
> >
> > Without keeping track of "reserve" pages, which doesn't feel
> > too clean.
>
> The problem with that is that once a slab was allocated with the right
> allocation context, anybody can get objects from these slabs.

[snip]

I understand that.


> So we either reserve a page per object, which for 32 byte objects is a
> large waste, or we stop anybody who doesn't have the right permissions
> from obtaining objects. I took the latter approach.

What I'm saying is that the slab allocator slowpath should always
just check watermarks against the current task. Instead of this
->reserve stuff.

^ permalink raw reply

* [PATCH 2/8] Move the get_net() from sock_copy()
From: Pavel Emelyanov @ 2007-10-31 13:44 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List, devel
In-Reply-To: <472885B1.7090103@openvz.org>

The sock_copy() is supposed to just clone the socket. In a perfect
world it has to be just memcpy, but we have to handle the security
mark correctly. All the extra setup must be performed in sk_clone() 
call, so move the get_net() into more proper place.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/net/core/sock.c b/net/core/sock.c
index fdacf9c..9c2dbfa 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -864,7 +864,6 @@ static void sock_copy(struct sock *nsk, const struct sock *osk)
 #endif
 
 	memcpy(nsk, osk, osk->sk_prot->obj_size);
-	get_net(nsk->sk_net);
 #ifdef CONFIG_SECURITY_NETWORK
 	nsk->sk_security = sptr;
 	security_sk_clone(osk, nsk);
@@ -958,6 +957,7 @@ struct sock *sk_clone(const struct sock *sk, const gfp_t priority)
 		sock_copy(newsk, sk);
 
 		/* SANITY */
+		get_net(newsk->sk_net);
 		sk_node_init(&newsk->sk_node);
 		sock_lock_init(newsk);
 		bh_lock_sock(newsk);
-- 
1.5.3.4


^ permalink raw reply related

* [PATCH 1/8] Move the sock_copy() from the header
From: Pavel Emelyanov @ 2007-10-31 13:42 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List, devel
In-Reply-To: <472885B1.7090103@openvz.org>

The sock_copy() call is not used outside the sock.c file,
so just move it into a sock.c

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/include/net/sock.h b/include/net/sock.h
index 43fc3fa..ecad7b4 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -993,20 +993,6 @@ static inline void sock_graft(struct sock *sk, struct socket *parent)
 	write_unlock_bh(&sk->sk_callback_lock);
 }
 
-static inline void sock_copy(struct sock *nsk, const struct sock *osk)
-{
-#ifdef CONFIG_SECURITY_NETWORK
-	void *sptr = nsk->sk_security;
-#endif
-
-	memcpy(nsk, osk, osk->sk_prot->obj_size);
-	get_net(nsk->sk_net);
-#ifdef CONFIG_SECURITY_NETWORK
-	nsk->sk_security = sptr;
-	security_sk_clone(osk, nsk);
-#endif
-}
-
 extern int sock_i_uid(struct sock *sk);
 extern unsigned long sock_i_ino(struct sock *sk);
 
diff --git a/net/core/sock.c b/net/core/sock.c
index bba9949..fdacf9c 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -857,6 +857,20 @@ static inline void sock_lock_init(struct sock *sk)
 			af_family_keys + sk->sk_family);
 }
 
+static void sock_copy(struct sock *nsk, const struct sock *osk)
+{
+#ifdef CONFIG_SECURITY_NETWORK
+	void *sptr = nsk->sk_security;
+#endif
+
+	memcpy(nsk, osk, osk->sk_prot->obj_size);
+	get_net(nsk->sk_net);
+#ifdef CONFIG_SECURITY_NETWORK
+	nsk->sk_security = sptr;
+	security_sk_clone(osk, nsk);
+#endif
+}
+
 /**
  *	sk_alloc - All socket objects are allocated here
  *	@net: the applicable net namespace
-- 
1.5.3.4


^ permalink raw reply related

* [PATCH 0/8] Cleanup/fix the sk_alloc() call
From: Pavel Emelyanov @ 2007-10-31 13:40 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List, devel

The sk_alloc() function suffers from two problems:
1 (major). The error path is not clean in it - if the security
   call fails, the net namespace is not put, if the try_module_get
   fails  additionally the security context is not released;
2 (minor). The zero_it argument is misleading, as it doesn't just 
   zeroes it, but performs some extra setup. Besides this argument 
   is used only in one place - in the sk_clone().

So this set fixes these problems and performs some additional
cleanup.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox