I have updated the dmfe.c network driver for 2.4.0-test by adding proper
locking (I hope), and also made transmission much efficient.
I would appreciate any feedback from people using this driver, to confirm
that I did not break it.
It would also be great if someone could take a look at the lock handling,
to confirm that is is correct and sufficient.
/Tobias
--- dmfe.c.orig Wed Nov 15 19:53:48 2000
+++ dmfe.c Wed Nov 15 21:35:24 2000
@@ -57,6 +57,11 @@
Resource usage cleanups.
Report driver version to user.
+ Tobias Ringstr?m <[email protected]> :
+ Added proper locking.
+ Rewrote the transmit code to actually use the ring buffer,
+ and to generate a lot fewer interrupts.
+
TODO
Implement pci_driver::suspend() and pci_driver::resume()
@@ -108,6 +113,7 @@
#define TX_MAX_SEND_CNT 0x1 /* Maximum tx packet per time */
#define TX_DESC_CNT 0x10 /* Allocated Tx descriptors */
#define RX_DESC_CNT 0x10 /* Allocated Rx descriptors */
+#define TX_IRQ_THR 12
#define DESC_ALL_CNT TX_DESC_CNT+RX_DESC_CNT
#define TX_BUF_ALLOC 0x600
#define RX_ALLOC_SIZE 0x620
@@ -188,6 +194,8 @@
u32 cr7_data;
u32 cr15_data;
+ spinlock_t lock;
+
/* descriptor pointer */
unsigned char *buf_pool_ptr; /* Tx buffer pool memory */
unsigned char *buf_pool_start; /* Tx buffer pool align dword */
@@ -198,8 +206,7 @@
struct rx_desc *first_rx_desc;
struct rx_desc *rx_insert_ptr;
struct rx_desc *rx_ready_ptr; /* packet come pointer */
- u32 tx_packet_cnt; /* transmitted packet count */
- u32 tx_queue_cnt; /* wait to send packet count */
+ u32 tx_live_cnt; /* number of used/live tx slots */
u32 rx_avail_cnt; /* available rx descriptor count */
u32 interval_rx_cnt; /* rx packet count a callback time */
@@ -490,8 +497,6 @@
/* system variable init */
db->cr6_data = CR6_DEFAULT | dmfe_cr6_user_set;
- db->tx_packet_cnt = 0;
- db->tx_queue_cnt = 0;
db->rx_avail_cnt = 0;
db->link_failed = 0;
db->wait_reset = 0;
@@ -595,46 +600,42 @@
{
struct dmfe_board_info *db = dev->priv;
struct tx_desc *txptr;
+ static unsigned pkt_num = TX_IRQ_THR;
DMFE_DBUG(0, "dmfe_start_xmit", 0);
-
- netif_stop_queue(dev);
-
- /* Too large packet check */
- if (skb->len > MAX_PACKET_SIZE) {
- printk(KERN_ERR "%s: oversized frame (%d bytes) for transmit.\n", dev->name, (u16) skb->len);
- dev_kfree_skb(skb);
- return 0;
- }
- /* No Tx resource check, it never happen nromally */
- if (db->tx_packet_cnt >= TX_FREE_DESC_CNT) {
- return 1;
- }
+
+ spin_lock_irq(&db->lock);
/* transmit this packet */
txptr = db->tx_insert_ptr;
memcpy((char *) txptr->tx_buf_ptr, (char *) skb->data, skb->len);
- txptr->tdes1 = 0xe1000000 | skb->len;
+ if (--pkt_num == 0)
+ {
+ txptr->tdes1 = 0xe1000000 | skb->len;
+ pkt_num = TX_IRQ_THR;
+ }
+ else
+ txptr->tdes1 = 0x61000000 | skb->len;
+
+ /* Transmit Packet Process */
+ txptr->tdes0 = 0x80000000; /* set owner bit to DM910X */
+ outl(0x1, dev->base_addr + DCR1); /* Issue Tx polling comand */
+ dev->trans_start = jiffies; /* saved the time stamp */
/* Point to next transmit free descriptor */
- db->tx_insert_ptr = (struct tx_desc *) txptr->next_tx_desc;
+ txptr = (struct tx_desc *)txptr->next_tx_desc;
- /* Transmit Packet Process */
- if (db->tx_packet_cnt < TX_MAX_SEND_CNT) {
- txptr->tdes0 = 0x80000000; /* set owner bit to DM910X */
- db->tx_packet_cnt++; /* Ready to send count */
- outl(0x1, dev->base_addr + DCR1); /* Issue Tx polling comand */
- } else {
- db->tx_queue_cnt++; /* queue the tx packet */
- outl(0x1, dev->base_addr + DCR1); /* Issue Tx polling comand */
- }
+ if (txptr->tdes0 & 0x80000000)
+ netif_stop_queue(dev);
- /* Tx resource check */
- if (db->tx_packet_cnt < TX_FREE_DESC_CNT)
- netif_wake_queue(dev);
+ db->tx_insert_ptr = txptr;
+ db->tx_live_cnt++;
+
+ spin_unlock_irq(&tp->lock);
/* free this SKB */
dev_kfree_skb(skb);
+
return 0;
}
@@ -713,12 +714,14 @@
outl(0, ioaddr + DCR7); /* disable all interrupt */
return;
}
+
+ spin_lock(&db->lock);
+
/* Free the transmitted descriptor */
txptr = db->tx_remove_ptr;
- while (db->tx_packet_cnt) {
+ while (db->tx_live_cnt > 0 && (txptr->tdes0 & 0x80000000) == 0)
+ {
/* printk("tdes0=%x\n", txptr->tdes0); */
- if (txptr->tdes0 & 0x80000000)
- break;
db->stats.tx_packets++;
if ((txptr->tdes0 & TDES0_ERR_MASK) && (txptr->tdes0 != 0x7fffffff)) {
@@ -734,23 +737,16 @@
db->stats.tx_errors++;
}
txptr = (struct tx_desc *) txptr->next_tx_desc;
- db->tx_packet_cnt--;
+ db->tx_live_cnt--;
}
/* Update TX remove pointer to next */
db->tx_remove_ptr = (struct tx_desc *) txptr;
- /* Send the Tx packet in queue */
- if ((db->tx_packet_cnt < TX_MAX_SEND_CNT) && db->tx_queue_cnt) {
- txptr->tdes0 = 0x80000000; /* set owner bit to DM910X */
- db->tx_packet_cnt++; /* Ready to send count */
- outl(0x1, ioaddr + DCR1); /* Issue Tx polling command */
- dev->trans_start = jiffies; /* saved the time stamp */
- db->tx_queue_cnt--;
- }
- /* Resource available check */
- if (db->tx_packet_cnt < TX_FREE_DESC_CNT)
+ if ((db->tx_insert_ptr->tdes0 & 0x80000000) == 0)
netif_wake_queue(dev);
+ spin_unlock(&db->lock);
+
/* Received the coming packet */
if (db->rx_avail_cnt)
dmfe_rx_packet(dev, db);
@@ -930,12 +926,14 @@
db->interval_rx_cnt = 0;
- if (db->wait_reset | (db->tx_packet_cnt &&
- ((jiffies - dev->trans_start) > DMFE_TX_TIMEOUT)) | (db->rx_error_cnt > 3)) {
+ if (db->wait_reset ||
+ (db->tx_live_cnt > 0 &&
+ ((jiffies - dev->trans_start) > DMFE_TX_TIMEOUT)) ||
+ (db->rx_error_cnt > 3)) {
/*
printk("wait_reset %x, tx cnt %x, rx err %x, time %x\n", db->wait_reset, db->tx_packet_cnt, db->rx_error_cnt, jiffies-dev->trans_start);
*/
- DMFE_DBUG(0, "Warn!! Warn!! Tx/Rx moniotr step1", db->tx_packet_cnt);
+ DMFE_DBUG(0, "Warn!! Warn!! Tx/Rx monitor step1", 0);
dmfe_dynamic_reset(dev);
db->timer.expires = DMFE_TIMER_WUT;
add_timer(&db->timer);
@@ -1022,8 +1020,7 @@
dmfe_free_rxbuffer(db);
/* system variable init */
- db->tx_packet_cnt = 0;
- db->tx_queue_cnt = 0;
+ db->tx_live_cnt = 0;
db->rx_avail_cnt = 0;
db->link_failed = 0;
db->wait_reset = 0;
@@ -1182,22 +1179,32 @@
}
/* Send a setup frame for DM9102/DM9102A
- This setup frame initilize DM910X addres filter mode
+ This setup frame initilize DM910X address filter mode
*/
static void send_filter_frame(struct net_device *dev, int mc_cnt)
{
struct dmfe_board_info *db = dev->priv;
struct dev_mc_list *mcptr;
struct tx_desc *txptr;
+ unsigned long flags;
u16 *addrptr;
u32 *suptr;
int i;
DMFE_DBUG(0, "send_filetr_frame()", 0);
+ spin_lock_irqsave(&db->lock, flags);
+
txptr = db->tx_insert_ptr;
suptr = (u32 *) txptr->tx_buf_ptr;
+ if (txptr->tdes0 & 0x80000000) {
+ spin_unlock_irq(&tp->lock);
+ printk(KERN_WARNING "%s: Too busy to send filter frame\n",
+ dev->name);
+ return;
+ }
+
/* Node address */
addrptr = (u16 *) dev->dev_addr;
*suptr++ = addrptr[0];
@@ -1224,19 +1231,15 @@
}
/* prepare the setup frame */
db->tx_insert_ptr = (struct tx_desc *) txptr->next_tx_desc;
+ db->tx_live_cnt++;
txptr->tdes1 = 0x890000c0;
- /* Resource Check and Send the setup packet */
- if (!db->tx_packet_cnt) {
- /* Resource Empty */
- db->tx_packet_cnt++;
- txptr->tdes0 = 0x80000000;
- update_cr6(db->cr6_data | 0x2000, dev->base_addr);
- outl(0x1, dev->base_addr + DCR1); /* Issue Tx polling command */
- update_cr6(db->cr6_data, dev->base_addr);
- } else {
- /* Put into TX queue */
- db->tx_queue_cnt++;
- }
+ /* Send the setup packet */
+ txptr->tdes0 = 0x80000000;
+ update_cr6(db->cr6_data | 0x2000, dev->base_addr);
+ outl(0x1, dev->base_addr + DCR1); /* Issue Tx polling command */
+ update_cr6(db->cr6_data, dev->base_addr);
+
+ spin_unlock_irqrestore(&tp->lock, flags);
}
/*
Hello,
I'll double check the locking later today, but not sure about the
transmission changes.
Regards,
Frank
([email protected])
--On Wednesday, November 15, 2000 9:34 PM +0100 Tobias Ringstrom
<[email protected]> wrote:
> I have updated the dmfe.c network driver for 2.4.0-test by adding proper
> locking (I hope), and also made transmission much efficient.
>
> I would appreciate any feedback from people using this driver, to confirm
> that I did not break it.
>
> It would also be great if someone could take a look at the lock handling,
> to confirm that is is correct and sufficient.
>
> /Tobias
>
>
> --- dmfe.c.orig Wed Nov 15 19:53:48 2000
> +++ dmfe.c Wed Nov 15 21:35:24 2000
> @@ -57,6 +57,11 @@
> Resource usage cleanups.
> Report driver version to user.
>
> + Tobias Ringstr?m <[email protected]> :
> + Added proper locking.
> + Rewrote the transmit code to actually use the ring buffer,
> + and to generate a lot fewer interrupts.
> +
> TODO
>
> Implement pci_driver::suspend() and pci_driver::resume()
> @@ -108,6 +113,7 @@
> #define TX_MAX_SEND_CNT 0x1 /* Maximum tx packet per time */
> #define TX_DESC_CNT 0x10 /* Allocated Tx descriptors */
> #define RX_DESC_CNT 0x10 /* Allocated Rx descriptors */
> +#define TX_IRQ_THR 12
> #define DESC_ALL_CNT TX_DESC_CNT+RX_DESC_CNT
> #define TX_BUF_ALLOC 0x600
> #define RX_ALLOC_SIZE 0x620
> @@ -188,6 +194,8 @@
> u32 cr7_data;
> u32 cr15_data;
>
> + spinlock_t lock;
> +
> /* descriptor pointer */
> unsigned char *buf_pool_ptr; /* Tx buffer pool memory */
> unsigned char *buf_pool_start; /* Tx buffer pool align dword */
> @@ -198,8 +206,7 @@
> struct rx_desc *first_rx_desc;
> struct rx_desc *rx_insert_ptr;
> struct rx_desc *rx_ready_ptr; /* packet come pointer */
> - u32 tx_packet_cnt; /* transmitted packet count */
> - u32 tx_queue_cnt; /* wait to send packet count */
> + u32 tx_live_cnt; /* number of used/live tx slots */
> u32 rx_avail_cnt; /* available rx descriptor count */
> u32 interval_rx_cnt; /* rx packet count a callback time */
>
> @@ -490,8 +497,6 @@
>
> /* system variable init */
> db->cr6_data = CR6_DEFAULT | dmfe_cr6_user_set;
> - db->tx_packet_cnt = 0;
> - db->tx_queue_cnt = 0;
> db->rx_avail_cnt = 0;
> db->link_failed = 0;
> db->wait_reset = 0;
> @@ -595,46 +600,42 @@
> {
> struct dmfe_board_info *db = dev->priv;
> struct tx_desc *txptr;
> + static unsigned pkt_num = TX_IRQ_THR;
>
> DMFE_DBUG(0, "dmfe_start_xmit", 0);
> -
> - netif_stop_queue(dev);
> -
> - /* Too large packet check */
> - if (skb->len > MAX_PACKET_SIZE) {
> - printk(KERN_ERR "%s: oversized frame (%d bytes) for transmit.\n",
> dev->name, (u16) skb->len); - dev_kfree_skb(skb);
> - return 0;
> - }
> - /* No Tx resource check, it never happen nromally */
> - if (db->tx_packet_cnt >= TX_FREE_DESC_CNT) {
> - return 1;
> - }
> +
> + spin_lock_irq(&db->lock);
>
> /* transmit this packet */
> txptr = db->tx_insert_ptr;
> memcpy((char *) txptr->tx_buf_ptr, (char *) skb->data, skb->len);
> - txptr->tdes1 = 0xe1000000 | skb->len;
> + if (--pkt_num == 0)
> + {
> + txptr->tdes1 = 0xe1000000 | skb->len;
> + pkt_num = TX_IRQ_THR;
> + }
> + else
> + txptr->tdes1 = 0x61000000 | skb->len;
> +
> + /* Transmit Packet Process */
> + txptr->tdes0 = 0x80000000; /* set owner bit to DM910X */
> + outl(0x1, dev->base_addr + DCR1); /* Issue Tx polling comand */
> + dev->trans_start = jiffies; /* saved the time stamp */
>
> /* Point to next transmit free descriptor */
> - db->tx_insert_ptr = (struct tx_desc *) txptr->next_tx_desc;
> + txptr = (struct tx_desc *)txptr->next_tx_desc;
>
> - /* Transmit Packet Process */
> - if (db->tx_packet_cnt < TX_MAX_SEND_CNT) {
> - txptr->tdes0 = 0x80000000; /* set owner bit to DM910X */
> - db->tx_packet_cnt++; /* Ready to send count */
> - outl(0x1, dev->base_addr + DCR1); /* Issue Tx polling comand */
> - } else {
> - db->tx_queue_cnt++; /* queue the tx packet */
> - outl(0x1, dev->base_addr + DCR1); /* Issue Tx polling comand */
> - }
> + if (txptr->tdes0 & 0x80000000)
> + netif_stop_queue(dev);
>
> - /* Tx resource check */
> - if (db->tx_packet_cnt < TX_FREE_DESC_CNT)
> - netif_wake_queue(dev);
> + db->tx_insert_ptr = txptr;
> + db->tx_live_cnt++;
> +
> + spin_unlock_irq(&tp->lock);
>
> /* free this SKB */
> dev_kfree_skb(skb);
> +
> return 0;
> }
>
> @@ -713,12 +714,14 @@
> outl(0, ioaddr + DCR7); /* disable all interrupt */
> return;
> }
> +
> + spin_lock(&db->lock);
> +
> /* Free the transmitted descriptor */
> txptr = db->tx_remove_ptr;
> - while (db->tx_packet_cnt) {
> + while (db->tx_live_cnt > 0 && (txptr->tdes0 & 0x80000000) == 0)
> + {
> /* printk("tdes0=%x\n", txptr->tdes0); */
> - if (txptr->tdes0 & 0x80000000)
> - break;
> db->stats.tx_packets++;
>
> if ((txptr->tdes0 & TDES0_ERR_MASK) && (txptr->tdes0 != 0x7fffffff)) {
> @@ -734,23 +737,16 @@
> db->stats.tx_errors++;
> }
> txptr = (struct tx_desc *) txptr->next_tx_desc;
> - db->tx_packet_cnt--;
> + db->tx_live_cnt--;
> }
> /* Update TX remove pointer to next */
> db->tx_remove_ptr = (struct tx_desc *) txptr;
>
> - /* Send the Tx packet in queue */
> - if ((db->tx_packet_cnt < TX_MAX_SEND_CNT) && db->tx_queue_cnt) {
> - txptr->tdes0 = 0x80000000; /* set owner bit to DM910X */
> - db->tx_packet_cnt++; /* Ready to send count */
> - outl(0x1, ioaddr + DCR1); /* Issue Tx polling command */
> - dev->trans_start = jiffies; /* saved the time stamp */
> - db->tx_queue_cnt--;
> - }
> - /* Resource available check */
> - if (db->tx_packet_cnt < TX_FREE_DESC_CNT)
> + if ((db->tx_insert_ptr->tdes0 & 0x80000000) == 0)
> netif_wake_queue(dev);
>
> + spin_unlock(&db->lock);
> +
> /* Received the coming packet */
> if (db->rx_avail_cnt)
> dmfe_rx_packet(dev, db);
> @@ -930,12 +926,14 @@
>
> db->interval_rx_cnt = 0;
>
> - if (db->wait_reset | (db->tx_packet_cnt &&
> - ((jiffies - dev->trans_start) > DMFE_TX_TIMEOUT)) |
> (db->rx_error_cnt > 3)) { + if (db->wait_reset ||
> + (db->tx_live_cnt > 0 &&
> + ((jiffies - dev->trans_start) > DMFE_TX_TIMEOUT)) ||
> + (db->rx_error_cnt > 3)) {
> /*
> printk("wait_reset %x, tx cnt %x, rx err %x, time %x\n",
> db->wait_reset, db->tx_packet_cnt, db->rx_error_cnt,
> jiffies-dev->trans_start); */
> - DMFE_DBUG(0, "Warn!! Warn!! Tx/Rx moniotr step1", db->tx_packet_cnt);
> + DMFE_DBUG(0, "Warn!! Warn!! Tx/Rx monitor step1", 0);
> dmfe_dynamic_reset(dev);
> db->timer.expires = DMFE_TIMER_WUT;
> add_timer(&db->timer);
> @@ -1022,8 +1020,7 @@
> dmfe_free_rxbuffer(db);
>
> /* system variable init */
> - db->tx_packet_cnt = 0;
> - db->tx_queue_cnt = 0;
> + db->tx_live_cnt = 0;
> db->rx_avail_cnt = 0;
> db->link_failed = 0;
> db->wait_reset = 0;
> @@ -1182,22 +1179,32 @@
> }
>
> /* Send a setup frame for DM9102/DM9102A
> - This setup frame initilize DM910X addres filter mode
> + This setup frame initilize DM910X address filter mode
> */
> static void send_filter_frame(struct net_device *dev, int mc_cnt)
> {
> struct dmfe_board_info *db = dev->priv;
> struct dev_mc_list *mcptr;
> struct tx_desc *txptr;
> + unsigned long flags;
> u16 *addrptr;
> u32 *suptr;
> int i;
>
> DMFE_DBUG(0, "send_filetr_frame()", 0);
>
> + spin_lock_irqsave(&db->lock, flags);
> +
> txptr = db->tx_insert_ptr;
> suptr = (u32 *) txptr->tx_buf_ptr;
>
> + if (txptr->tdes0 & 0x80000000) {
> + spin_unlock_irq(&tp->lock);
> + printk(KERN_WARNING "%s: Too busy to send filter frame\n",
> + dev->name);
> + return;
> + }
> +
> /* Node address */
> addrptr = (u16 *) dev->dev_addr;
> *suptr++ = addrptr[0];
> @@ -1224,19 +1231,15 @@
> }
> /* prepare the setup frame */
> db->tx_insert_ptr = (struct tx_desc *) txptr->next_tx_desc;
> + db->tx_live_cnt++;
> txptr->tdes1 = 0x890000c0;
> - /* Resource Check and Send the setup packet */
> - if (!db->tx_packet_cnt) {
> - /* Resource Empty */
> - db->tx_packet_cnt++;
> - txptr->tdes0 = 0x80000000;
> - update_cr6(db->cr6_data | 0x2000, dev->base_addr);
> - outl(0x1, dev->base_addr + DCR1); /* Issue Tx polling command */
> - update_cr6(db->cr6_data, dev->base_addr);
> - } else {
> - /* Put into TX queue */
> - db->tx_queue_cnt++;
> - }
> + /* Send the setup packet */
> + txptr->tdes0 = 0x80000000;
> + update_cr6(db->cr6_data | 0x2000, dev->base_addr);
> + outl(0x1, dev->base_addr + DCR1); /* Issue Tx polling command */
> + update_cr6(db->cr6_data, dev->base_addr);
> +
> + spin_unlock_irqrestore(&tp->lock, flags);
> }
>
> /*
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> Please read the FAQ at http://www.tux.org/lkml/
Tobias Ringstrom wrote:
>
> I have updated the dmfe.c network driver for 2.4.0-test by adding proper
> locking (I hope), and also made transmission much efficient.
>
> I would appreciate any feedback from people using this driver, to confirm
> that I did not break it.
>
> It would also be great if someone could take a look at the lock handling,
> to confirm that is is correct and sufficient.
Would you mind creating a separate patch that -just- correcting the SMP
safety? That makes it much easier to review and apply, and then we can
consider the other changes...
Thanks,
Jeff
--
Jeff Garzik |
Building 1024 | The chief enemy of creativity is "good" sense
MandrakeSoft | -- Picasso
On Wed, 15 Nov 2000, Jeff Garzik wrote:
> Tobias Ringstrom wrote:
> >
> > I have updated the dmfe.c network driver for 2.4.0-test by adding proper
> > locking (I hope), and also made transmission much efficient.
> >
> > I would appreciate any feedback from people using this driver, to confirm
> > that I did not break it.
> >
> > It would also be great if someone could take a look at the lock handling,
> > to confirm that is is correct and sufficient.
>
> Would you mind creating a separate patch that -just- correcting the SMP
> safety? That makes it much easier to review and apply, and then we can
> consider the other changes...
Such a patch will appear shortly. I and Frank Davis are currently merging
our patches for dmfe.c.
[Actually, I just added reasonable locks while my main goal was to improve
performance. I did not realize that there was such a strong need for SMP
safety (since it has been broken in that regard for a long time, without
anyone fixing it).]
/Tobias
Tobias Ringstrom wrote:
> On Wed, 15 Nov 2000, Jeff Garzik wrote:
> > Tobias Ringstrom wrote:
> > > I have updated the dmfe.c network driver for 2.4.0-test by adding proper
> > > locking (I hope), and also made transmission much efficient.
> > Would you mind creating a separate patch that -just- correcting the SMP
> > safety? That makes it much easier to review and apply, and then we can
> > consider the other changes...
> Such a patch will appear shortly. I and Frank Davis are currently merging
> our patches for dmfe.c.
Thanks a bunch.
> [Actually, I just added reasonable locks while my main goal was to improve
> performance. I did not realize that there was such a strong need for SMP
> safety (since it has been broken in that regard for a long time, without
> anyone fixing it).]
The kernel driver APIs are designed so that SMP and UP cases are equally
high-performance, and portable beyond the x86 platform.
Pretty much all ISA and PCI drivers need to be portable and SMP safe...
if not so, it's a bug. That said, there is certainly more motivation to
make a popular PCI driver is SMP safe than an older ISA driver. And
portability is [IMHO] less of a priority than SMP safety, though it
depends on the hardware being supported.
Regards,
Jeff
--
Jeff Garzik |
Building 1024 | The chief enemy of creativity is "good" sense
MandrakeSoft | -- Picasso
On Thu, 16 Nov 2000, Jeff Garzik wrote:
> The kernel driver APIs are designed so that SMP and UP cases are equally
> high-performance, and portable beyond the x86 platform.
>
> Pretty much all ISA and PCI drivers need to be portable and SMP safe...
> if not so, it's a bug. That said, there is certainly more motivation to
> make a popular PCI driver is SMP safe than an older ISA driver. And
> portability is [IMHO] less of a priority than SMP safety, though it
> depends on the hardware being supported.
How about adding an ifdef CONFIG_SMP then print ugly warning to all known
SMP unsafe drivers? A message could be printed booth at compile and load
time.
/Tobias
I would rather fix those non-SMP compliant drivers to be SMP compliant,
then keeping them 'broken'. Adding the print statements would only be a
temporary solution.
Regards,
Frank
--On Friday, November 17, 2000 10:20 AM +0100 Tobias Ringstrom
> How about adding an ifdef CONFIG_SMP then print ugly warning to all known
> SMP unsafe drivers? A message could be printed booth at compile and load
> time.
>
> /Tobias
>
> --On Friday, November 17, 2000 10:20 AM +0100 Tobias Ringstrom
> > How about adding an ifdef CONFIG_SMP then print ugly warning to all known
> > SMP unsafe drivers? A message could be printed booth at compile and load
> > time.
Frank Davis wrote:
> I would rather fix those non-SMP compliant drivers to be SMP compliant,
> then keeping them 'broken'. Adding the print statements would only be a
> temporary solution.
Agreed. If people have SMP safety patches for net drivers, let me
know...
Jeff
--
Jeff Garzik |
Building 1024 | The chief enemy of creativity is "good" sense
MandrakeSoft | -- Picasso
On Fri, 17 Nov 2000, Frank Davis wrote:
>
> I would rather fix those non-SMP compliant drivers to be SMP compliant,
> then keeping them 'broken'. Adding the print statements would only be a
> temporary solution.
Of course. This list of priorites is very natural, I think:
1. Working SMP driver
2. Broken SMP driver with a warning.
3. Broken SMP driver without a warning. (Even if "everyone" knows it
is broken)
It takes less than a minute to add such a warning, but it can take days
or weeks to find someone to really fix the driver. That was my point.
/Tobias
Tobias Ringstrom wrote:
>
> On Fri, 17 Nov 2000, Frank Davis wrote:
> >
> > I would rather fix those non-SMP compliant drivers to be SMP compliant,
> > then keeping them 'broken'. Adding the print statements would only be a
> > temporary solution.
>
> Of course. This list of priorites is very natural, I think:
>
> 1. Working SMP driver
> 2. Broken SMP driver with a warning.
> 3. Broken SMP driver without a warning. (Even if "everyone" knows it
> is broken)
>
> It takes less than a minute to add such a warning, but it can take days
> or weeks to find someone to really fix the driver. That was my point.
Marking them with a #warning is fine with me.
--
Jeff Garzik |
Building 1024 | The chief enemy of creativity is "good" sense
MandrakeSoft | -- Picasso
[Jeff Garzik]
> Pretty much all ISA and PCI drivers need to be portable and SMP
> safe... if not so, it's a bug. That said, there is certainly more
> motivation to make a popular PCI driver is SMP safe than an older ISA
> driver.
Usually, but you never know...
"o SMP optimised 3c501"
-- 2.1.132ac2 changelog
(:
Peter