Re: [uml-devel] [PATCH] UML - Fix irqstack crash
On lunedì 24 settembre 2007, Jeff Dike wrote: > On Thu, Sep 20, 2007 at 05:57:49PM +0200, Paolo Giarrusso wrote: > > Yes, indeed - or sign extension on 64bit machines would set to 1 the > > whole high-word. > > > > But using long for that mask makes no difference; either int or long > > long (or better, either u32 or u64) should be used, given that the > > used signal range is the same on 32 and 64bit machines, it should > > be u32 for normal signals or u64 if RT-signals are also allowed. > > We don't use RT signals for anything, so we could use u32. I wasn't sure of that. > I don't > see it making much difference though. Agreed - the only difference is for cleanliness and easy verification. -- "Doh!" (cit.), I've made another mistake! Paolo Giarrusso, aka Blaisorblade signature.asc Description: This is a digitally signed message part. - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/___ User-mode-linux-devel mailing list User-mode-linux-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
Re: [uml-devel] [PATCH] UML - time build fix
On mercoledì 26 settembre 2007, Thomas Gleixner wrote: > Jeff, > > On Tue, 2007-09-25 at 17:56 -0400, Jeff Dike wrote: > > On Tue, Sep 25, 2007 at 09:54:15PM +0200, Thomas Gleixner wrote: > > > On Tue, 2007-09-25 at 13:37 -0400, Jeff Dike wrote: > > > > Put back an implementation of timeval_to_ns in > > > > arch/um/os-Linux/time.c. tglx pointed out in his review of tickless > > > > support that there was a perfectly good implementation of it in > > > > linux/time.h. The problem is that this is userspace code which can't > > > > pull in kernel headers and there doesn't seem to be a libc version. > > > > > > Oops. Did not notice. > > > > It's a UML peculiarity... > > > > > Can't we move it into some header file which is accessible from > > > everywhere ? There is a way to do this without code duplication, but it is creating a non-inline function which calls the inline and calling the non-inline from userspace. It's done for a variety of other functions. There is a tradeoff of speed vs code duplication - and if this function is not supposed to change and to need to be kept in sync, it could be copied. I conceptually hate this solution, but it can make some sense. -- "Doh!" (cit.), I've made another mistake! Paolo Giarrusso, aka Blaisorblade signature.asc Description: This is a digitally signed message part. - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/___ User-mode-linux-devel mailing list User-mode-linux-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
[uml-devel] Is commit da3e30e78ed9652322dccb49fa149e7b4e985f74 a candidate for -stable?
Without this commit, you can't compile UML on a system with 2.6.22 kernel headers (the reserved2 and reserved3 fields have been renamed). With it, you can use 2.6.22 or older as you like. (not forwarding to -stable because it seems importunate of me to do so: is this being too cautious?) -- `Some people don't think performance issues are "real bugs", and I think such people shouldn't be allowed to program.' --- Linus Torvalds - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ User-mode-linux-devel mailing list User-mode-linux-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
Re: [uml-devel] Is commit da3e30e78ed9652322dccb49fa149e7b4e985f74 a candidate for -stable?
On Wed, Sep 26, 2007 at 09:59:47PM +0100, Nix wrote: > Without this commit, you can't compile UML on a system with 2.6.22 > kernel headers (the reserved2 and reserved3 fields have been > renamed). With it, you can use 2.6.22 or older as you like. I take nominations... The ones I decide on myself are those where I have more than one report of an already-fixed crash or compilation problem. > (not forwarding to -stable because it seems importunate of me to do so: > is this being too cautious?) This is a good forum. Jeff -- Work email - jdike at linux dot intel dot com - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ User-mode-linux-devel mailing list User-mode-linux-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
[uml-devel] [PATCH 2/3] UML - Network driver MTU cleanups
A bunch of MTU-related cleanups in the network code. First, there is the addition of the notion of a maximally-sized packet, which is the MTU plus headers. This is used to size the skb that will receive a packet. This allows ether_adjust_skb to go away, as it was used to resize the skb after it was allocated. Since the skb passed into the low-level read routine is no longer resized, and possibly reallocated, there, they (and the write routines) don't need to get an sk_buff **. They just need the sk_buff * now. The callers of ether_adjust_skb still need to do the skb_put, so that's now inlined. The MAX_PACKET definitions in most of the drivers are gone. The set_mtu methods were all the same and did nothing, so they can be removed. The ethertap driver had a typo which doubled the size of the packet rather than adding two bytes to it. It also wasn't defining its setup_size, causing a zero-byte kmalloc and crash when the invalid pointer returned from kmalloc was dereferenced. Signed-off-by: Jeff Dike <[EMAIL PROTECTED]> --- arch/um/drivers/daemon_kern.c| 15 +++ arch/um/drivers/daemon_user.c| 11 +--- arch/um/drivers/mcast_kern.c | 13 +++-- arch/um/drivers/mcast_user.c | 11 +--- arch/um/drivers/net_kern.c | 42 +-- arch/um/drivers/pcap_kern.c | 13 ++--- arch/um/drivers/pcap_user.c |9 ++ arch/um/drivers/slip_kern.c | 10 ++- arch/um/drivers/slip_user.c |9 +- arch/um/drivers/slirp_kern.c | 14 -- arch/um/drivers/slirp_user.c |9 +- arch/um/drivers/vde_kern.c | 19 +- arch/um/drivers/vde_user.c | 11 +--- arch/um/include/net_kern.h | 13 - arch/um/include/net_user.h |4 +- arch/um/os-Linux/drivers/ethertap_kern.c | 30 +++--- arch/um/os-Linux/drivers/ethertap_user.c |9 +- arch/um/os-Linux/drivers/tuntap_kern.c | 15 +++ arch/um/os-Linux/drivers/tuntap_user.c | 11 +--- 19 files changed, 80 insertions(+), 188 deletions(-) Index: linux-2.6.20/arch/um/drivers/daemon_user.c === --- linux-2.6.20.orig/arch/um/drivers/daemon_user.c 2007-09-26 17:14:22.0 -0400 +++ linux-2.6.20/arch/um/drivers/daemon_user.c 2007-09-26 17:19:31.0 -0400 @@ -19,8 +19,6 @@ #include "um_malloc.h" #include "user.h" -#define MAX_PACKET (ETH_MAX_PACKET + ETH_HEADER_OTHER) - enum request_type { REQ_NEW_CONTROL }; #define SWITCH_MAGIC 0xfeedface @@ -184,18 +182,13 @@ int daemon_user_write(int fd, void *buf, return net_sendto(fd, buf, len, data_addr, sizeof(*data_addr)); } -static int daemon_set_mtu(int mtu, void *data) -{ - return mtu; -} - const struct net_user_info daemon_user_info = { .init = daemon_user_init, .open = daemon_open, .close = NULL, .remove = daemon_remove, - .set_mtu= daemon_set_mtu, .add_address= NULL, .delete_address = NULL, - .max_packet = MAX_PACKET - ETH_HEADER_OTHER + .mtu= ETH_MAX_PACKET, + .max_packet = ETH_MAX_PACKET + ETH_HEADER_OTHER, }; Index: linux-2.6.20/arch/um/drivers/mcast_user.c === --- linux-2.6.20.orig/arch/um/drivers/mcast_user.c 2007-09-26 17:14:22.0 -0400 +++ linux-2.6.20/arch/um/drivers/mcast_user.c 2007-09-26 17:19:31.0 -0400 @@ -20,8 +20,6 @@ #include "um_malloc.h" #include "user.h" -#define MAX_PACKET (ETH_MAX_PACKET + ETH_HEADER_OTHER) - static struct sockaddr_in *new_addr(char *addr, unsigned short port) { struct sockaddr_in *sin; @@ -154,18 +152,13 @@ int mcast_user_write(int fd, void *buf, return net_sendto(fd, buf, len, data_addr, sizeof(*data_addr)); } -static int mcast_set_mtu(int mtu, void *data) -{ - return mtu; -} - const struct net_user_info mcast_user_info = { .init = mcast_user_init, .open = mcast_open, .close = mcast_close, .remove = mcast_remove, - .set_mtu= mcast_set_mtu, .add_address= NULL, .delete_address = NULL, - .max_packet = MAX_PACKET - ETH_HEADER_OTHER + .mtu= ETH_MAX_PACKET, + .max_packet = ETH_MAX_PACKET + ETH_HEADER_OTHER, }; Index: linux-2.6.20/arch/um/drivers/pcap_user.c === --- linux-2.6.20.orig/arch/um/drivers/pcap_user.c 2007-09-26 17:14:22.0 -0400 +++ linux-2.6.20/arch/um/drivers/pcap_user.c2007-09-26 17:19:31.0 -0400 @@ -13,8 +13,6 @@ #include "um_malloc.h" #include "user.h" -#de
[uml-devel] [PATCH 3/3] UML - Correctly handle skb allocation failures
Handle memory allocation failures when reading packets. We have to read something from the host, even if we can't allocate any memory. If we don't, the host side of the device may fill up and stop delivering interrupts because no new packets can be queued. A single sk_buff is allocated whenever an MTU is seen which is larger than any seen earlier. This is used to read packets if there is a memory allocation failure. The large MTU check is done from eth_configure, which is called when a interface is added to the system, and from uml_net_change_mtu, which is called when an existing interface has its MTU changed. Signed-off-by: Jeff Dike <[EMAIL PROTECTED]> --- arch/um/drivers/net_kern.c | 60 + 1 file changed, 60 insertions(+) Index: linux-2.6.20/arch/um/drivers/net_kern.c === --- linux-2.6.20.orig/arch/um/drivers/net_kern.c2007-09-26 16:48:21.0 -0400 +++ linux-2.6.20/arch/um/drivers/net_kern.c 2007-09-26 16:56:16.0 -0400 @@ -34,6 +34,48 @@ static inline void set_ether_mac(struct static DEFINE_SPINLOCK(opened_lock); static LIST_HEAD(opened); +/* + * The throwaway skb is used when we can't allocate an skb. The + * packet is read into throwaway in order to get the data off the + * connection to the host. + * It is reallocated whenever an MTU is seen which is larger than + * anything seen before. update_throwaway_skb is called from + * eth_configure for new interfaces and from uml_net_change_mtu for + * MTU changes on existing interfaces. + */ +static DEFINE_SPINLOCK(throwaway_lock); +static struct sk_buff *throwaway; +static int throwaway_max; + +static int update_throwaway_skb(int max) +{ + struct sk_buff *new; + int err = 0; + + spin_lock(&throwaway_lock); + + if (max <= throwaway_max) + goto out; + + err = -ENOMEM; + new = dev_alloc_skb(max); + if (new == NULL) + goto out; + + skb_put(new, max); + + kfree_skb(throwaway); + throwaway = new; + throwaway_max = max; + err = 0; +out: + spin_unlock(&throwaway_lock); + + return err; +} + +int npackets = 0; + static int uml_net_rx(struct net_device *dev) { struct uml_net_private *lp = dev->priv; @@ -42,7 +84,14 @@ static int uml_net_rx(struct net_device /* If we can't allocate memory, try again next round. */ skb = dev_alloc_skb(lp->max_packet); + if ((++npackets % 100) == 0){ + kfree_skb(skb); + skb = NULL; + } + if (skb == NULL) { + throwaway->dev = dev; + (*lp->read)(lp->fd, throwaway, lp); lp->stats.rx_dropped++; return 0; } @@ -240,6 +289,13 @@ static int uml_net_set_mac(struct net_de static int uml_net_change_mtu(struct net_device *dev, int new_mtu) { + struct uml_net_private *lp = dev->priv; + int err; + + err = update_throwaway_skb(lp->max_packet); + if (err) + return err; + dev->mtu = new_mtu; return 0; @@ -447,6 +503,10 @@ static void eth_configure(int n, void *i dev->watchdog_timeo = (HZ >> 1); dev->irq = UM_ETH_IRQ; + err = update_throwaway_skb(lp->max_packet); + if (err) + goto out_undo_user_init; + rtnl_lock(); err = register_netdevice(dev); rtnl_unlock(); - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ User-mode-linux-devel mailing list User-mode-linux-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
Re: [uml-devel] [PATCH 3/3] UML - Correctly handle skb allocation failures
On Wed, 26 Sep 2007 17:46:13 -0400 Jeff Dike <[EMAIL PROTECTED]> wrote: > Handle memory allocation failures when reading packets. > > We have to read something from the host, even if we can't allocate any > memory. If we don't, the host side of the device may fill up and stop > delivering interrupts because no new packets can be queued. > > A single sk_buff is allocated whenever an MTU is seen which is larger > than any seen earlier. This is used to read packets if there is a > memory allocation failure. > > The large MTU check is done from eth_configure, which is called when a > interface is added to the system, and from uml_net_change_mtu, which > is called when an existing interface has its MTU changed. > > Signed-off-by: Jeff Dike <[EMAIL PROTECTED]> > --- > arch/um/drivers/net_kern.c | 60 > + > 1 file changed, 60 insertions(+) > > Index: linux-2.6.20/arch/um/drivers/net_kern.c > === > --- linux-2.6.20.orig/arch/um/drivers/net_kern.c 2007-09-26 > 16:48:21.0 -0400 > +++ linux-2.6.20/arch/um/drivers/net_kern.c 2007-09-26 16:56:16.0 > -0400 > @@ -34,6 +34,48 @@ static inline void set_ether_mac(struct > static DEFINE_SPINLOCK(opened_lock); > static LIST_HEAD(opened); > > +/* > + * The throwaway skb is used when we can't allocate an skb. The > + * packet is read into throwaway in order to get the data off the > + * connection to the host. > + * It is reallocated whenever an MTU is seen which is larger than > + * anything seen before. update_throwaway_skb is called from > + * eth_configure for new interfaces and from uml_net_change_mtu for > + * MTU changes on existing interfaces. > + */ > +static DEFINE_SPINLOCK(throwaway_lock); > +static struct sk_buff *throwaway; > +static int throwaway_max; > + > +static int update_throwaway_skb(int max) > +{ > + struct sk_buff *new; > + int err = 0; > + > + spin_lock(&throwaway_lock); > + > + if (max <= throwaway_max) > + goto out; > + > + err = -ENOMEM; > + new = dev_alloc_skb(max); > + if (new == NULL) > + goto out; > + > + skb_put(new, max); > + > + kfree_skb(throwaway); > + throwaway = new; > + throwaway_max = max; > + err = 0; > +out: > + spin_unlock(&throwaway_lock); > + > + return err; > +} > + > +int npackets = 0; Unneeded initialisation? Maybe not a good name for a global symbol ;) I worry that the memory at "throwaway" can get thrown away... > static int uml_net_rx(struct net_device *dev) > { > struct uml_net_private *lp = dev->priv; > @@ -42,7 +84,14 @@ static int uml_net_rx(struct net_device > > /* If we can't allocate memory, try again next round. */ > skb = dev_alloc_skb(lp->max_packet); > + if ((++npackets % 100) == 0){ > + kfree_skb(skb); > + skb = NULL; > + } > + > if (skb == NULL) { > + throwaway->dev = dev; > + (*lp->read)(lp->fd, throwaway, lp); ... while other code is still using it. Are you sure we don't need throwaway_lock here? > lp->stats.rx_dropped++; > return 0; > } > @@ -240,6 +289,13 @@ static int uml_net_set_mac(struct net_de > > static int uml_net_change_mtu(struct net_device *dev, int new_mtu) > { > + struct uml_net_private *lp = dev->priv; > + int err; > + > + err = update_throwaway_skb(lp->max_packet); > + if (err) > + return err; > + > dev->mtu = new_mtu; > > return 0; > @@ -447,6 +503,10 @@ static void eth_configure(int n, void *i > dev->watchdog_timeo = (HZ >> 1); > dev->irq = UM_ETH_IRQ; > > + err = update_throwaway_skb(lp->max_packet); > + if (err) > + goto out_undo_user_init; > + > rtnl_lock(); > err = register_netdevice(dev); > rtnl_unlock(); - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ User-mode-linux-devel mailing list User-mode-linux-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel