Re: [PATCH 2/5] lguest guest feedback tidyups
On Fri, May 11, 2007 at 11:21:30AM +1000, Rusty Russell wrote: > 1) send-dma and bind-dma hypercall wrappers for drivers to use, > 2) formalization of the convention that devices can use the irq >corresponding to their index on the lguest_bus. > 3) ___force to shut up sparse: guests *can* use ioremap as virtual mem. No, they can't. Even if in your case the underlying address spaces happen to be the same anything returned by ioremap must use the proper accessors. That's the whole point of having this separation, otherwise you wouldn't need to use ioremap at all. So instead of sprinkling cast around add lguest_read*/lguest_write* accessors that do the __force cast once and make sure the ioremap return value is always accessed using those. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 0/5] lguest feedback tidyups
On Fri, May 11, 2007 at 11:17:26AM +1000, Rusty Russell wrote: > - But the cost was high: lots of __force casts 8( That sounds like something is very fishy. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 2/2] lguest: Convert to hrtimer framework
On Wed, 2007-05-09 at 09:37 -0400, James Morris wrote: > Convert lguest to the hrtimer framework, enabling dynamic ticks and high > resolution timers. Thanks very much for this James! Applied after the Great Documentation patch, so I'm adding some extra verbiage. Cheers, Rusty. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
[PATCH 5/5] lguest console driver feedback tidyups
1) Use new lguest_send_dma & lguest_bind_dma functions.
2) sparse: lguest_cons can be static.
Signed-off-by: Rusty Russell <[EMAIL PROTECTED]>
---
drivers/char/hvc_lguest.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
===
--- a/drivers/char/hvc_lguest.c
+++ b/drivers/char/hvc_lguest.c
@@ -36,7 +36,7 @@ static int put_chars(u32 vtermno, const
dma.len[1] = 0;
dma.addr[0] = __pa(buf);
- hcall(LHCALL_SEND_DMA, LGUEST_CONSOLE_DMA_KEY, __pa(&dma), 0);
+ lguest_send_dma(LGUEST_CONSOLE_DMA_KEY, &dma);
return count;
}
@@ -59,7 +59,7 @@ static int get_chars(u32 vtermno, char *
return count;
}
-struct hv_ops lguest_cons = {
+static struct hv_ops lguest_cons = {
.get_chars = get_chars,
.put_chars = put_chars,
};
@@ -75,14 +75,17 @@ console_initcall(cons_init);
static int lguestcons_probe(struct lguest_device *lgdev)
{
- lgdev->private = hvc_alloc(0, lgdev->index+1, &lguest_cons, 256);
+ int err;
+
+ lgdev->private = hvc_alloc(0, lgdev_irq(lgdev), &lguest_cons, 256);
if (IS_ERR(lgdev->private))
return PTR_ERR(lgdev->private);
- if (!hcall(LHCALL_BIND_DMA, LGUEST_CONSOLE_DMA_KEY, __pa(&cons_input),
- (1<<8) + lgdev->index+1))
+ err = lguest_bind_dma(LGUEST_CONSOLE_DMA_KEY, &cons_input, 1,
+ lgdev_irq(lgdev));
+ if (err)
printk("lguest console: failed to bind buffer.\n");
- return 0;
+ return err;
}
static struct lguest_driver lguestcons_drv = {
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
[PATCH 2/5] lguest guest feedback tidyups
1) send-dma and bind-dma hypercall wrappers for drivers to use,
2) formalization of the convention that devices can use the irq
corresponding to their index on the lguest_bus.
3) ___force to shut up sparse: guests *can* use ioremap as virtual mem.
4) lguest.c should include "lguest_bus.h" for lguest_devices declaration.
Signed-off-by: Rusty Russell <[EMAIL PROTECTED]>
---
drivers/lguest/lguest.c | 20
drivers/lguest/lguest_bus.c |2 +-
include/linux/lguest_bus.h | 13 -
3 files changed, 33 insertions(+), 2 deletions(-)
===
--- a/include/linux/lguest_bus.h
+++ b/include/linux/lguest_bus.h
@@ -7,7 +7,6 @@
struct lguest_device {
/* Unique busid, and index into lguest_page->devices[] */
- /* By convention, each device can use irq index+1 if it wants to. */
unsigned int index;
struct device dev;
@@ -15,6 +14,18 @@ struct lguest_device {
/* Driver can hang data off here. */
void *private;
};
+
+/* By convention, each device can use irq index+1 if it wants to. */
+static inline int lgdev_irq(const struct lguest_device *dev)
+{
+ return dev->index + 1;
+}
+
+/* dma args must not be vmalloced! */
+void lguest_send_dma(unsigned long key, struct lguest_dma *dma);
+int lguest_bind_dma(unsigned long key, struct lguest_dma *dmas,
+ unsigned int num, u8 irq);
+void lguest_unbind_dma(unsigned long key, struct lguest_dma *dmas);
struct lguest_driver {
const char *name;
===
--- a/drivers/lguest/lguest.c
+++ b/drivers/lguest/lguest.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -99,6 +100,25 @@ void async_hcall(unsigned long call,
next_call = 0;
}
local_irq_restore(flags);
+}
+
+void lguest_send_dma(unsigned long key, struct lguest_dma *dma)
+{
+ dma->used_len = 0;
+ hcall(LHCALL_SEND_DMA, key, __pa(dma), 0);
+}
+
+int lguest_bind_dma(unsigned long key, struct lguest_dma *dmas,
+ unsigned int num, u8 irq)
+{
+ if (!hcall(LHCALL_BIND_DMA, key, __pa(dmas), (num << 8) | irq))
+ return -ENOMEM;
+ return 0;
+}
+
+void lguest_unbind_dma(unsigned long key, struct lguest_dma *dmas)
+{
+ hcall(LHCALL_BIND_DMA, key, __pa(dmas), 0);
}
static unsigned long save_fl(void)
===
--- a/drivers/lguest/lguest_bus.c
+++ b/drivers/lguest/lguest_bus.c
@@ -136,7 +136,7 @@ static int __init lguest_bus_init(void)
return 0;
/* Devices are in page above top of "normal" mem. */
- lguest_devices = ioremap(max_pfn << PAGE_SHIFT, PAGE_SIZE);
+ lguest_devices = (__force void*)ioremap(max_pfn
[PATCH 4/5] lguest block driver feedback tidyups
1) Use new dma wrapper functions, and handle bind failure (may happen
in future)
2) Use new lgdev_irq() "get me a good interrupt number" function.
3) __force the ioremap: guests can use it as normal memory.
Signed-off-by: Rusty Russell <[EMAIL PROTECTED]>
---
drivers/block/lguest_blk.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
===
--- a/drivers/block/lguest_blk.c
+++ b/drivers/block/lguest_blk.c
@@ -123,7 +123,7 @@ static void do_write(struct blockdev *bd
pr_debug("lgb: WRITE sector %li\n", (long)req->sector);
setup_req(bd, 1, req, &send);
- hcall(LHCALL_SEND_DMA, bd->phys_addr, __pa(&send), 0);
+ lguest_send_dma(bd->phys_addr, &send);
}
static void do_read(struct blockdev *bd, struct request *req)
@@ -134,7 +134,7 @@ static void do_read(struct blockdev *bd,
setup_req(bd, 0, req, &bd->dma);
empty_dma(&ping);
- hcall(LHCALL_SEND_DMA, bd->phys_addr, __pa(&ping), 0);
+ lguest_send_dma(bd->phys_addr, &ping);
}
static void do_lgb_request(request_queue_t *q)
@@ -183,13 +183,13 @@ static int lguestblk_probe(struct lguest
return -ENOMEM;
spin_lock_init(&bd->lock);
- bd->irq = lgdev->index+1;
+ bd->irq = lgdev_irq(lgdev);
bd->req = NULL;
bd->dma.used_len = 0;
bd->dma.len[0] = 0;
bd->phys_addr = (lguest_devices[lgdev->index].pfn << PAGE_SHIFT);
- bd->lb_page = (void *)ioremap(bd->phys_addr, PAGE_SIZE);
+ bd->lb_page = (__force void *)ioremap(bd->phys_addr, PAGE_SIZE);
if (!bd->lb_page) {
err = -ENOMEM;
goto out_free_bd;
@@ -225,7 +225,9 @@ static int lguestblk_probe(struct lguest
if (err)
goto out_cleanup_queue;
- hcall(LHCALL_BIND_DMA, bd->phys_addr, __pa(&bd->dma), (1<<8)+bd->irq);
+ err = lguest_bind_dma(bd->phys_addr, &bd->dma, 1, bd->irq);
+ if (err)
+ goto out_free_irq;
bd->disk->major = bd->major;
bd->disk->first_minor = 0;
@@ -241,6 +243,8 @@ static int lguestblk_probe(struct lguest
lgdev->private = bd;
return 0;
+out_free_irq:
+ free_irq(bd->irq, bd);
out_cleanup_queue:
blk_cleanup_queue(bd->disk->queue);
out_put_disk:
@@ -248,7 +252,7 @@ out_unregister_blkdev:
out_unregister_blkdev:
unregister_blkdev(bd->major, "lguestblk");
out_unmap:
- iounmap(bd->lb_page);
+ iounmap((__force void *__iomem)bd->lb_page);
out_free_bd:
kfree(bd);
return err;
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
[PATCH 3/5] lguest network driver feedback tidyups
Feedback from Jeff Garzik:
1) Use netdev_priv instead of dev->priv.
2) Check for ioremap failure
3) iounmap on failure.
4) Wrap SEND_DMA and BIND_DMA calls
5) Don't set NETIF_F_SG unless we set NETIF_F_NO_CSUM
6) Use SET_NETDEV_DEV()
7) Don't set dev->irq, mem_start & mem_end (deprecated)
Sparse warnings:
8) __force the ioremap: guests can use it as normal memory.
Signed-off-by: Rusty Russell <[EMAIL PROTECTED]>
---
drivers/net/lguest_net.c | 53 ++
1 file changed, 31 insertions(+), 22 deletions(-)
===
--- a/drivers/net/lguest_net.c
+++ b/drivers/net/lguest_net.c
@@ -35,6 +35,9 @@ struct lguestnet_info
unsigned long peer_phys;
unsigned long mapsize;
+ /* The lguest_device I come from */
+ struct lguest_device *lgdev;
+
/* My peerid. */
unsigned int me;
@@ -84,7 +87,7 @@ static void skb_to_dma(const struct sk_b
static void lguestnet_set_multicast(struct net_device *dev)
{
- struct lguestnet_info *info = dev->priv;
+ struct lguestnet_info *info = netdev_priv(dev);
if ((dev->flags & (IFF_PROMISC|IFF_ALLMULTI)) || dev->mc_count)
info->peer[info->me].mac[0] |= PROMISC_BIT;
@@ -110,13 +113,13 @@ static void transfer_packet(struct net_d
struct sk_buff *skb,
unsigned int peernum)
{
- struct lguestnet_info *info = dev->priv;
+ struct lguestnet_info *info = netdev_priv(dev);
struct lguest_dma dma;
skb_to_dma(skb, skb_headlen(skb), &dma);
pr_debug("xfer length %04x (%u)\n", htons(skb->len), skb->len);
- hcall(LHCALL_SEND_DMA, peer_key(info,peernum), __pa(&dma), 0);
+ lguest_send_dma(peer_key(info, peernum), &dma);
if (dma.used_len != skb->len) {
dev->stats.tx_carrier_errors++;
pr_debug("Bad xfer to peer %i: %i of %i (dma %p/%i)\n",
@@ -137,7 +140,7 @@ static int lguestnet_start_xmit(struct s
{
unsigned int i;
int broadcast;
- struct lguestnet_info *info = dev->priv;
+ struct lguestnet_info *info = netdev_priv(dev);
const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
pr_debug("%s: xmit %02x:%02x:%02x:%02x:%02x:%02x\n",
@@ -162,7 +165,7 @@ static int lguestnet_start_xmit(struct s
/* Find a new skb to put in this slot in shared mem. */
static int fill_slot(struct net_device *dev, unsigned int slot)
{
- struct lguestnet_info *info = dev->priv;
+ struct lguestnet_info *info = netdev_priv(dev);
/* Try to create and register a new one. */
info->skb[slot] = netdev_alloc_skb(dev, ETH_HLEN + ETH_DATA_LEN);
if (!info->skb[slot]) {
@@ -180,7 +183,7 @@ static irqreturn_t lguestnet_rcv(int irq
static irqreturn_t lguestnet_rcv(int irq, void *dev_id)
{
struct net_device *dev = dev_id;
- struct lguestnet_info *info = dev->priv;
+ struct lguestnet_info *info = netdev_priv(dev);
unsigned int i, done = 0;
for (i = 0; i < ARRAY_SIZE(info->dma); i++) {
@@ -220,7 +223,7 @@ static int lguestnet_open(struct net_dev
static int lguestnet_open(struct net_device *dev)
{
int i;
- struct lguestnet_info *info = dev->priv;
+ struct lguestnet_info *info = netdev_priv(dev);
/* Set up our MAC address */
memcpy(info->peer[info->me].mac, dev->dev_addr, ETH_ALEN);
@@ -232,8 +235,8 @@ static int lguestnet_open(struct net_dev
if (fill_slot(dev, i) != 0)
goto cleanup;
}
- if (!hcall(LHCALL_BIND_DMA, peer_key(info, info->me), __pa(info->dma),
- (NUM_SKBS << 8) | dev->irq))
+ if (lguest_bind_dma(peer_key(info,info->me), info->dma,
+ NUM_SKBS, lgdev_irq(info->lgdev)) != 0)
goto cleanup;
return 0;
@@ -246,13 +249,13 @@ static int lguestnet_close(struct net_de
static int lguestnet_close(struct net_device *dev)
{
unsigned int i;
- struct lguestnet_info *info = dev->priv;
+ struct lguestnet_info *info = netdev_priv(dev);
/* Clear all trace: others might deliver packets, we'll ignore it. */
memset(&info->peer[info->me], 0, sizeof(info->peer[info->me]));
/* Deregister sg lists. */
- hcall(LHCALL_BIND_DMA, peer_key(info, info->me), __pa(info->dma), 0);
+ lguest_unbind_dma(peer_key(info, info->me), info->dma);
for (i = 0; i < ARRAY_SIZE(info->dma); i++)
dev_kfree_skb(info->skb[i]);
return 0;
@@ -290,30 +293,34 @@ static int lguestnet_probe(struct lguest
/* Turning on/off promisc will call dev->set_multicast_list.
* We don't actually support multicast yet */
dev->set_multicast_list = lguestnet_set_multicast;
- dev->mem_start = ((unsigned long)desc->pfn << PAGE_SHIFT);
- dev->mem_end = dev->me
[PATCH 1/5] lguest host feedback tidyups
1) Sam Ravnborg says lg-objs is deprecated, use lg-y.
2) Sparse: page_tables.c unnecessary initialization
3) Lots of __force to shut sparse up: guest "physical" addresses are
userspace virtual.
4) Change prototype of run_lguest and do cast in caller instead (when we add
__iomem to cast, it runs over another line).
Signed-off-by: Rusty Russell <[EMAIL PROTECTED]>
---
drivers/lguest/Makefile |2 +-
drivers/lguest/core.c | 16
drivers/lguest/hypercalls.c |3 ++-
drivers/lguest/interrupts_and_traps.c |4 ++--
drivers/lguest/lg.h |2 +-
drivers/lguest/lguest_user.c |2 +-
drivers/lguest/page_tables.c |2 +-
7 files changed, 16 insertions(+), 15 deletions(-)
===
--- a/drivers/lguest/Makefile
+++ b/drivers/lguest/Makefile
@@ -3,5 +3,5 @@ obj-$(CONFIG_LGUEST_GUEST) += lguest.o l
# Host requires the other files, which can be a module.
obj-$(CONFIG_LGUEST) += lg.o
-lg-objs := core.o hypercalls.o page_tables.o interrupts_and_traps.o \
+lg-y := core.o hypercalls.o page_tables.o interrupts_and_traps.o \
segments.o io.o lguest_user.o switcher.o
===
--- a/drivers/lguest/core.c
+++ b/drivers/lguest/core.c
@@ -218,7 +218,7 @@ u32 lgread_u32(struct lguest *lg, u32 ad
/* Don't let them access lguest binary */
if (!lguest_address_ok(lg, addr, sizeof(val))
- || get_user(val, (u32 __user *)addr) != 0)
+ || get_user(val, (__force u32 __user *)addr) != 0)
kill_guest(lg, "bad read address %u", addr);
return val;
}
@@ -226,14 +226,14 @@ void lgwrite_u32(struct lguest *lg, u32
void lgwrite_u32(struct lguest *lg, u32 addr, u32 val)
{
if (!lguest_address_ok(lg, addr, sizeof(val))
- || put_user(val, (u32 __user *)addr) != 0)
+ || put_user(val, (__force u32 __user *)addr) != 0)
kill_guest(lg, "bad write address %u", addr);
}
void lgread(struct lguest *lg, void *b, u32 addr, unsigned bytes)
{
if (!lguest_address_ok(lg, addr, bytes)
- || copy_from_user(b, (void __user *)addr, bytes) != 0) {
+ || copy_from_user(b, (__force void __user *)addr, bytes) != 0) {
/* copy_from_user should do this, but as we rely on it... */
memset(b, 0, bytes);
kill_guest(lg, "bad read address %u len %u", addr, bytes);
@@ -243,7 +243,7 @@ void lgwrite(struct lguest *lg, u32 addr
void lgwrite(struct lguest *lg, u32 addr, const void *b, unsigned bytes)
{
if (!lguest_address_ok(lg, addr, bytes)
- || copy_to_user((void __user *)addr, b, bytes) != 0)
+ || copy_to_user((__force void __user *)addr, b, bytes) != 0)
kill_guest(lg, "bad write address %u len %u", addr, bytes);
}
@@ -294,7 +294,7 @@ static void run_guest_once(struct lguest
: "memory", "%edx", "%ecx", "%edi", "%esi");
}
-int run_guest(struct lguest *lg, char *__user user)
+int run_guest(struct lguest *lg, unsigned long __user *user)
{
while (!lg->dead) {
unsigned int cr2 = 0; /* Damn gcc */
@@ -302,8 +302,8 @@ int run_guest(struct lguest *lg, char *_
/* Hypercalls first: we might have been out to userspace */
do_hypercalls(lg);
if (lg->dma_is_pending) {
- if (put_user(lg->pending_dma, (unsigned long *)user) ||
- put_user(lg->pending_key, (unsigned long *)user+1))
+ if (put_user(lg->pending_dma, user) ||
+ put_user(lg->pending_key, user+1))
return -EFAULT;
return sizeof(unsigned long)*2;
}
@@ -420,7 +420,7 @@ static int __init init(void)
lock_cpu_hotplug();
if (cpu_has_pge) { /* We have a broader idea of "global". */
cpu_had_pge = 1;
- on_each_cpu(adjust_pge, 0, 0, 1);
+ on_each_cpu(adjust_pge, (void *)0, 0, 1);
clear_bit(X86_FEATURE_PGE, boot_cpu_data.x86_capability);
}
unlock_cpu_hotplug();
===
--- a/drivers/lguest/hypercalls.c
+++ b/drivers/lguest/hypercalls.c
@@ -83,7 +83,8 @@ static void do_hcall(struct lguest *lg,
guest_set_pmd(lg, regs->edx, regs->ebx);
break;
case LHCALL_LOAD_TLS:
- guest_load_tls(lg, (struct desc_struct __user*)regs->edx);
+ guest_load_tls(lg,
+ (__force struct desc_struct __user*)regs->edx);
break;
case LHCALL_TS:
lg->ts = regs->edx;
===
--- a/drivers/lguest/interrupts_and
[PATCH 0/5] lguest feedback tidyups
Hi all, Gratefully-received recent feedback from CC'd was applied to excellent effect (and the advice from Matt Mackall about my personal appearance is best unrequited). The patch is split in 5 parts to correspond with the 9 parts Andrew sent out before, but here's the summary: 1) Sparse (thanks Christoph Hellwig): - lguest_const can be static now - lguest.c should include "lguest_bus.h" for lguest_devices declaration. - page_tables.c unnecessary initialization - But the cost was high: lots of __force casts 8( 2) Jeff Garzik - Use netdev_priv instead of dev->priv. - Check for ioremap failure - iounmap on failure. - Wrap SEND_DMA and BIND_DMA calls - Don't set NETIF_F_SG unless we set NETIF_F_NO_CSUM - Use SET_NETDEV_DEV() - Don't set dev->irq, mem_start & mem_end (deprecated) 3) Sam Ravnborg - lg-objs is deprecated, use lg-y. Cheers, Rusty. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [1/2] [NET] link_watch: Move link watch list into net_device
From: Jeremy Fitzhardinge <[EMAIL PROTECTED]> Date: Thu, 10 May 2007 15:45:42 -0700 > David Miller wrote: > > I'm not so certain now that we know it's the jiffies wrap point :-) > > > > The fixes in question are attached below and they were posted and > > discussed on netdev: > > > > Yep, this patch gets rid of my spinning thread. I can't find this patch > or any discussion on marc.info; is there a better netdev list archive? I don't see it there either... let me check my mail archive... Indeed, they were "posted" to netdev but were blocked by the vger regexp filters on the keyword "urgent" so that postings never made it to the list. I removed that filter regexp so that never happens again, sorry. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [1/2] [NET] link_watch: Move link watch list into net_device
* Jeremy Fitzhardinge ([EMAIL PROTECTED]) wrote: > Yep, this patch gets rid of my spinning thread. I can't find this patch > or any discussion on marc.info; is there a better netdev list archive? See the "linkwatch bustage in git-net" thread on netdev http://thread.gmane.org/gmane.linux.network/61800/focus=61812 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [1/2] [NET] link_watch: Move link watch list into net_device
David Miller wrote: > I'm not so certain now that we know it's the jiffies wrap point :-) > > The fixes in question are attached below and they were posted and > discussed on netdev: > Yep, this patch gets rid of my spinning thread. I can't find this patch or any discussion on marc.info; is there a better netdev list archive? J ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [1/2] [NET] link_watch: Move link watch list into net_device
From: Jeremy Fitzhardinge <[EMAIL PROTECTED]>
Date: Thu, 10 May 2007 15:22:17 -0700
> Andrew Morton wrote:
> > Five minutes after boot is when jiffies wraps. Are you sure it's
> > a list-screwup rather than a jiffy-wrap screwup?
> >
>
>
> Hm, its suggestive, isn't it? Apparently they've already fixed this in
> the sekret networking clubhouse, so I'll need to track it down.
I'm not so certain now that we know it's the jiffies wrap point :-)
The fixes in question are attached below and they were posted and
discussed on netdev:
commit fe47cdba83b3041e4ac1aa1418431020a4afe1e0
Author: Herbert Xu <[EMAIL PROTECTED]>
Date: Tue May 8 23:22:43 2007 -0700
[NET] link_watch: Eliminate potential delay on wrap-around
When the jiffies wrap around or when the system boots up for the first
time, down events can be delayed indefinitely since we no longer
update linkwatch_nextevent when only urgent events are processed.
This patch fixes this by setting linkwatch_nextevent when a
wrap-around occurs.
Signed-off-by: Herbert Xu <[EMAIL PROTECTED]>
Signed-off-by: David S. Miller <[EMAIL PROTECTED]>
diff --git a/net/core/link_watch.c b/net/core/link_watch.c
index b5f4579..4674ae5 100644
--- a/net/core/link_watch.c
+++ b/net/core/link_watch.c
@@ -101,8 +101,10 @@ static void linkwatch_schedule_work(unsigned long delay)
return;
/* If we wrap around we'll delay it by at most HZ. */
- if (delay > HZ)
+ if (delay > HZ) {
+ linkwatch_nextevent = jiffies;
delay = 0;
+ }
schedule_delayed_work(&linkwatch_work, delay);
}
commit 4cba637dbb9a13706494a1c85174c8e736914010
Author: Herbert Xu <[EMAIL PROTECTED]>
Date: Wed May 9 00:17:30 2007 -0700
[NET] link_watch: Always schedule urgent events
Urgent events may be delayed if we already have a non-urgent event
queued for that device. This patch changes this by making sure that
an urgent event is always looked at immediately.
I've replaced the LW_RUNNING flag by LW_URGENT since whether work
is scheduled is already kept track by the work queue system.
The only complication is that we have to provide some exclusion for
the setting linkwatch_nextevent which is available in the actual
work function.
Signed-off-by: Herbert Xu <[EMAIL PROTECTED]>
Signed-off-by: David S. Miller <[EMAIL PROTECTED]>
diff --git a/net/core/link_watch.c b/net/core/link_watch.c
index 4674ae5..a5e372b 100644
--- a/net/core/link_watch.c
+++ b/net/core/link_watch.c
@@ -26,7 +26,7 @@
enum lw_bits {
- LW_RUNNING = 0,
+ LW_URGENT = 0,
};
static unsigned long linkwatch_flags;
@@ -95,18 +95,41 @@ static void linkwatch_add_event(struct net_device *dev)
}
-static void linkwatch_schedule_work(unsigned long delay)
+static void linkwatch_schedule_work(int urgent)
{
- if (test_and_set_bit(LW_RUNNING, &linkwatch_flags))
+ unsigned long delay = linkwatch_nextevent - jiffies;
+
+ if (test_bit(LW_URGENT, &linkwatch_flags))
return;
- /* If we wrap around we'll delay it by at most HZ. */
- if (delay > HZ) {
- linkwatch_nextevent = jiffies;
+ /* Minimise down-time: drop delay for up event. */
+ if (urgent) {
+ if (test_and_set_bit(LW_URGENT, &linkwatch_flags))
+ return;
delay = 0;
}
- schedule_delayed_work(&linkwatch_work, delay);
+ /* If we wrap around we'll delay it by at most HZ. */
+ if (delay > HZ)
+ delay = 0;
+
+ /*
+* This is true if we've scheduled it immeditately or if we don't
+* need an immediate execution and it's already pending.
+*/
+ if (schedule_delayed_work(&linkwatch_work, delay) == !delay)
+ return;
+
+ /* Don't bother if there is nothing urgent. */
+ if (!test_bit(LW_URGENT, &linkwatch_flags))
+ return;
+
+ /* It's already running which is good enough. */
+ if (!cancel_delayed_work(&linkwatch_work))
+ return;
+
+ /* Otherwise we reschedule it again for immediate exection. */
+ schedule_delayed_work(&linkwatch_work, 0);
}
@@ -123,7 +146,11 @@ static void __linkwatch_run_queue(int urgent_only)
*/
if (!urgent_only)
linkwatch_nextevent = jiffies + HZ;
- clear_bit(LW_RUNNING, &linkwatch_flags);
+ /* Limit wrap-around effect on delay. */
+ else if (time_after(linkwatch_nextevent, jiffies + HZ))
+ linkwatch_nextevent = jiffies;
+
+ clear_bit(LW_URGENT, &linkwatch_flags);
spin_lock_irq(&lweventlist_lock);
next = lweventlist;
@@ -166,7 +193,7 @@ static void __linkwatch_run_queue(int urgent_only)
}
if (lweventlist)
- linkwatch_schedule_work(linkwatch_nextev
Re: [1/2] [NET] link_watch: Move link watch list into net_device
Andrew Morton wrote: > Five minutes after boot is when jiffies wraps. Are you sure it's > a list-screwup rather than a jiffy-wrap screwup? > Hm, its suggestive, isn't it? Apparently they've already fixed this in the sekret networking clubhouse, so I'll need to track it down. J ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [1/2] [NET] link_watch: Move link watch list into net_device
On Thu, 10 May 2007 15:00:05 -0700 Jeremy Fitzhardinge <[EMAIL PROTECTED]> wrote: > Herbert Xu wrote: > > [NET] link_watch: Move link watch list into net_device > > > > These days the link watch mechanism is an integral part of the > > network subsystem as it manages the carrier status. So it now > > makes sense to allocate some memory for it in net_device rather > > than allocating it on demand. > > I think there's a problem with one of these two patches. I've been > noticing that one of my events/X threads has been going into a spin for > about 5 mins after boot. I added some debugging to > kernel/workqueue.c:run_workqueue, since its that loop which seems to be > spinning due to list corruption. > > When I look to see if that loop has iterated for more than 100 times in > one go (which seems unlikely), I get this: > > BUG: cpu 3, count=101 list screwup on c04babe4, func c03217e8 > func=linkwatch_event+0x0/0x2a > [] show_trace_log_lvl+0x1a/0x30 > [] show_trace+0x12/0x14 > [] dump_stack+0x16/0x18 > [] run_workqueue+0x97/0x18c > [] worker_thread+0xe5/0xf5 > [] kthread+0x3b/0x62 > [] kernel_thread_helper+0x7/0x10 > === > > > I wonder if the problem is that the linkwatch_work is being rescheduled > when its already been scheduled, or something like that? Five minutes after boot is when jiffies wraps. Are you sure it's a list-screwup rather than a jiffy-wrap screwup? ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [1/2] [NET] link_watch: Move link watch list into net_device
David Miller wrote: > From: Jeremy Fitzhardinge <[EMAIL PROTECTED]> > Date: Thu, 10 May 2007 15:00:05 -0700 > > >> Herbert Xu wrote: >> >>> [NET] link_watch: Move link watch list into net_device >>> >>> These days the link watch mechanism is an integral part of the >>> network subsystem as it manages the carrier status. So it now >>> makes sense to allocate some memory for it in net_device rather >>> than allocating it on demand. >>> >> I think there's a problem with one of these two patches. >> > > Yes, there are :-) > > Did you catch the follow-on bug fixes? > Nope. Guess I got dropped from the cc: list. Was this on lkml or netdev? J ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [1/2] [NET] link_watch: Move link watch list into net_device
From: Jeremy Fitzhardinge <[EMAIL PROTECTED]> Date: Thu, 10 May 2007 15:00:05 -0700 > Herbert Xu wrote: > > [NET] link_watch: Move link watch list into net_device > > > > These days the link watch mechanism is an integral part of the > > network subsystem as it manages the carrier status. So it now > > makes sense to allocate some memory for it in net_device rather > > than allocating it on demand. > > I think there's a problem with one of these two patches. Yes, there are :-) Did you catch the follow-on bug fixes? ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [1/2] [NET] link_watch: Move link watch list into net_device
Herbert Xu wrote: > [NET] link_watch: Move link watch list into net_device > > These days the link watch mechanism is an integral part of the > network subsystem as it manages the carrier status. So it now > makes sense to allocate some memory for it in net_device rather > than allocating it on demand. I think there's a problem with one of these two patches. I've been noticing that one of my events/X threads has been going into a spin for about 5 mins after boot. I added some debugging to kernel/workqueue.c:run_workqueue, since its that loop which seems to be spinning due to list corruption. When I look to see if that loop has iterated for more than 100 times in one go (which seems unlikely), I get this: BUG: cpu 3, count=101 list screwup on c04babe4, func c03217e8 func=linkwatch_event+0x0/0x2a [] show_trace_log_lvl+0x1a/0x30 [] show_trace+0x12/0x14 [] dump_stack+0x16/0x18 [] run_workqueue+0x97/0x18c [] worker_thread+0xe5/0xf5 [] kthread+0x3b/0x62 [] kernel_thread_helper+0x7/0x10 === I wonder if the problem is that the linkwatch_work is being rescheduled when its already been scheduled, or something like that? J ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [patch 8/9] lguest: the block driver
On 05/09/2007 12:22 PM, Pekka Enberg wrote:
+static void end_entire_request(struct request *req, int uptodate)
+{
+ if (end_that_request_first(req, uptodate, req->hard_nr_sectors))
+ BUG();
+ add_disk_randomness(req->rq_disk);
+ blkdev_dequeue_request(req);
+ end_that_request_last(req, uptodate);
+}
Again, I would prefer this went straight into block/ll_rw_blk.c. Rene
and I am using something similar in the new Mitsumi driver although
one of us has a bug already, we're using req->nr_sectors for this...
The req->hard_nr_sectors is specifically marked block layer internal and
since nr_sectors is what we're requesting and reading (we can't deal with
partial transfers anyway since the drive won't tell us where it failed as
far as I've been able to see and we do want to make read requests for as
many sectors as possible at a time for speed) I believe that in our case the
nr_sectors is fine.
Rene.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [patch 7/9] lguest: the net driver
On Thu, 2007-05-10 at 01:33 -0400, Jeff Garzik wrote:
> Rusty Russell wrote:
> > I realize your continual battle with this, but adding a layer of
> > indirection doesn't seem like it will add clarity. The issues with
> > __pa() are reasonably known (don't hand it a vmalloc address, for
> > example). Any wrapper I create would be another hurdle to jump 8(
>
> You don't want this low level stuff in drivers.
[snip answers]
Hi Jeff,
This email was illuminating, thanks. I've put NAPI, set_mac_address
and module unload in the documentation patch for future enthusiasts.
The rest is implemented here.
Here's the extract from my current "feedback" patch:
...
2) Jeff Garzik
- Use netdev_priv instead of dev->priv.
- Check for ioremap failure
- iounmap on failure.
- Wrap SEND_DMA and BIND_DMA calls
- Don't set NETIF_F_SG unless we set NETIF_F_NO_CSUM
- Use SET_NETDEV_DEV()
- Don't set dev->irq, mem_start & mem_end (deprecated)
diff -r 35c8b37f8d3c drivers/lguest/lguest.c
--- a/drivers/lguest/lguest.c Wed May 09 23:23:56 2007 +1000
+++ b/drivers/lguest/lguest.c Thu May 10 20:06:25 2007 +1000
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -99,6 +100,25 @@ void async_hcall(unsigned long call,
next_call = 0;
}
local_irq_restore(flags);
+}
+
+void lguest_send_dma(unsigned long key, struct lguest_dma *dma)
+{
+ dma->used_len = 0;
+ hcall(LHCALL_SEND_DMA, key, __pa(dma), 0);
+}
+
+int lguest_bind_dma(unsigned long key, struct lguest_dma *dmas,
+ unsigned int num, u8 irq)
+{
+ if (!hcall(LHCALL_BIND_DMA, key, __pa(dmas), (num << 8) | irq))
+ return -ENOMEM;
+ return 0;
+}
+
+void lguest_unbind_dma(unsigned long key, struct lguest_dma *dmas)
+{
+ hcall(LHCALL_BIND_DMA, key, __pa(dmas), 0);
}
static unsigned long save_fl(void)
diff -r 35c8b37f8d3c drivers/net/lguest_net.c
--- a/drivers/net/lguest_net.c Wed May 09 23:23:56 2007 +1000
+++ b/drivers/net/lguest_net.c Thu May 10 19:30:21 2007 +1000
@@ -35,6 +35,9 @@ struct lguestnet_info
unsigned long peer_phys;
unsigned long mapsize;
+ /* The lguest_device I come from */
+ struct lguest_device *lgdev;
+
/* My peerid. */
unsigned int me;
@@ -84,7 +87,7 @@ static void skb_to_dma(const struct sk_b
static void lguestnet_set_multicast(struct net_device *dev)
{
- struct lguestnet_info *info = dev->priv;
+ struct lguestnet_info *info = netdev_priv(dev);
if ((dev->flags & (IFF_PROMISC|IFF_ALLMULTI)) || dev->mc_count)
info->peer[info->me].mac[0] |= PROMISC_BIT;
@@ -110,13 +113,13 @@ static void transfer_packet(struct net_d
struct sk_buff *skb,
unsigned int peernum)
{
- struct lguestnet_info *info = dev->priv;
+ struct lguestnet_info *info = netdev_priv(dev);
struct lguest_dma dma;
skb_to_dma(skb, skb_headlen(skb), &dma);
pr_debug("xfer length %04x (%u)\n", htons(skb->len), skb->len);
- hcall(LHCALL_SEND_DMA, peer_key(info,peernum), __pa(&dma), 0);
+ lguest_send_dma(peer_key(info, peernum), &dma);
if (dma.used_len != skb->len) {
dev->stats.tx_carrier_errors++;
pr_debug("Bad xfer to peer %i: %i of %i (dma %p/%i)\n",
@@ -137,7 +140,7 @@ static int lguestnet_start_xmit(struct s
{
unsigned int i;
int broadcast;
- struct lguestnet_info *info = dev->priv;
+ struct lguestnet_info *info = netdev_priv(dev);
const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
pr_debug("%s: xmit %02x:%02x:%02x:%02x:%02x:%02x\n",
@@ -162,7 +165,7 @@ static int lguestnet_start_xmit(struct s
/* Find a new skb to put in this slot in shared mem. */
static int fill_slot(struct net_device *dev, unsigned int slot)
{
- struct lguestnet_info *info = dev->priv;
+ struct lguestnet_info *info = netdev_priv(dev);
/* Try to create and register a new one. */
info->skb[slot] = netdev_alloc_skb(dev, ETH_HLEN + ETH_DATA_LEN);
if (!info->skb[slot]) {
@@ -180,7 +183,7 @@ static irqreturn_t lguestnet_rcv(int irq
static irqreturn_t lguestnet_rcv(int irq, void *dev_id)
{
struct net_device *dev = dev_id;
- struct lguestnet_info *info = dev->priv;
+ struct lguestnet_info *info = netdev_priv(dev);
unsigned int i, done = 0;
for (i = 0; i < ARRAY_SIZE(info->dma); i++) {
@@ -220,7 +223,7 @@ static int lguestnet_open(struct net_dev
static int lguestnet_open(struct net_device *dev)
{
int i;
- struct lguestnet_info *info = dev->priv;
+ struct lguestnet_info *info = netdev_priv(dev);
/* Set up our MAC address */
memcpy(info->peer[info->me].mac, dev->dev_addr, ETH_ALEN);
@@ -232,8 +235,8 @@ static int lguestnet_open(struct net_dev
