Prev: [PATCH] ntp: Make time_esterror and time_maxerror static
Next: Confirm Your Email
From: Stephen Hemminger on 1 Feb 2010 13:30
This fixes the fact that re->flags is always zero without causing
other confusion.

--- a/drivers/net/sky2.c 2010-02-01 10:07:42.676296236 -0800
+++ b/drivers/net/sky2.c 2010-02-01 10:18:12.575044064 -0800
@@ -1025,11 +1025,8 @@ static void sky2_prefetch_init(struct sk
static inline struct sky2_tx_le *get_tx_le(struct sky2_port *sky2, u16 *slot)
{
struct sky2_tx_le *le = sky2->tx_le + *slot;
- struct tx_ring_info *re = sky2->tx_ring + *slot;

*slot = RING_NEXT(*slot, sky2->tx_ring_size);
- re->flags = 0;
- re->skb = NULL;
le->ctrl = 0;
return le;
}
@@ -1622,8 +1619,7 @@ static unsigned tx_le_req(const struct s
return count;
}

-static void sky2_tx_unmap(struct pci_dev *pdev,
- const struct tx_ring_info *re)
+static void sky2_tx_unmap(struct pci_dev *pdev, struct tx_ring_info *re)
{
if (re->flags & TX_MAP_SINGLE)
pci_unmap_single(pdev, pci_unmap_addr(re, mapaddr),
@@ -1633,6 +1629,7 @@ static void sky2_tx_unmap(struct pci_dev
pci_unmap_page(pdev, pci_unmap_addr(re, mapaddr),
pci_unmap_len(re, maplen),
PCI_DMA_TODEVICE);
+ re->flags = 0;
}

/*
@@ -1839,6 +1836,7 @@ static void sky2_tx_complete(struct sky2
dev->stats.tx_packets++;
dev->stats.tx_bytes += skb->len;

+ re->skb = NULL;
dev_kfree_skb_any(skb);

sky2->tx_next = RING_NEXT(idx, sky2->tx_ring_size);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Michael Breuer on 1 Feb 2010 13:50
On 2/1/2010 1:20 PM, Stephen Hemminger wrote:
> This fixes the fact that re->flags is always zero without causing
> other confusion.
>
> --- a/drivers/net/sky2.c 2010-02-01 10:07:42.676296236 -0800
> +++ b/drivers/net/sky2.c 2010-02-01 10:18:12.575044064 -0800
> @@ -1025,11 +1025,8 @@ static void sky2_prefetch_init(struct sk
> static inline struct sky2_tx_le *get_tx_le(struct sky2_port *sky2, u16 *slot)
> {
> struct sky2_tx_le *le = sky2->tx_le + *slot;
> - struct tx_ring_info *re = sky2->tx_ring + *slot;
>
> *slot = RING_NEXT(*slot, sky2->tx_ring_size);
> - re->flags = 0;
> - re->skb = NULL;
> le->ctrl = 0;
> return le;
> }
> @@ -1622,8 +1619,7 @@ static unsigned tx_le_req(const struct s
> return count;
> }
>
> -static void sky2_tx_unmap(struct pci_dev *pdev,
> - const struct tx_ring_info *re)
> +static void sky2_tx_unmap(struct pci_dev *pdev, struct tx_ring_info *re)
> {
> if (re->flags& TX_MAP_SINGLE)
> pci_unmap_single(pdev, pci_unmap_addr(re, mapaddr),
> @@ -1633,6 +1629,7 @@ static void sky2_tx_unmap(struct pci_dev
> pci_unmap_page(pdev, pci_unmap_addr(re, mapaddr),
> pci_unmap_len(re, maplen),
> PCI_DMA_TODEVICE);
> + re->flags = 0;
> }
>
> /*
> @@ -1839,6 +1836,7 @@ static void sky2_tx_complete(struct sky2
> dev->stats.tx_packets++;
> dev->stats.tx_bytes += skb->len;
>
> + re->skb = NULL;
> dev_kfree_skb_any(skb);
>
> sky2->tx_next = RING_NEXT(idx, sky2->tx_ring_size);
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo(a)vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Running with this patch now - dma debug num_free_entries stable - so
far, so good.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jarek Poplawski on 1 Feb 2010 15:20
On Mon, Feb 01, 2010 at 10:20:18AM -0800, Stephen Hemminger wrote:
> This fixes the fact that re->flags is always zero without causing
> other confusion.

Actually, there is a slight confusion: after tx_init() slots #0 are
skipped and waiting for tx_complete. Of course, no big deal, but no
problem with fixing it too, so there is the main difference between
these two patches. (Moving re->flags and re->skb initializations is
an optimization, but I tried to change only the buggy parts.)

Jarek P.

>
> --- a/drivers/net/sky2.c 2010-02-01 10:07:42.676296236 -0800
> +++ b/drivers/net/sky2.c 2010-02-01 10:18:12.575044064 -0800
> @@ -1025,11 +1025,8 @@ static void sky2_prefetch_init(struct sk
> static inline struct sky2_tx_le *get_tx_le(struct sky2_port *sky2, u16 *slot)
> {
> struct sky2_tx_le *le = sky2->tx_le + *slot;
> - struct tx_ring_info *re = sky2->tx_ring + *slot;
>
> *slot = RING_NEXT(*slot, sky2->tx_ring_size);
> - re->flags = 0;
> - re->skb = NULL;
> le->ctrl = 0;
> return le;
> }
> @@ -1622,8 +1619,7 @@ static unsigned tx_le_req(const struct s
> return count;
> }
>
> -static void sky2_tx_unmap(struct pci_dev *pdev,
> - const struct tx_ring_info *re)
> +static void sky2_tx_unmap(struct pci_dev *pdev, struct tx_ring_info *re)
> {
> if (re->flags & TX_MAP_SINGLE)
> pci_unmap_single(pdev, pci_unmap_addr(re, mapaddr),
> @@ -1633,6 +1629,7 @@ static void sky2_tx_unmap(struct pci_dev
> pci_unmap_page(pdev, pci_unmap_addr(re, mapaddr),
> pci_unmap_len(re, maplen),
> PCI_DMA_TODEVICE);
> + re->flags = 0;
> }
>
> /*
> @@ -1839,6 +1836,7 @@ static void sky2_tx_complete(struct sky2
> dev->stats.tx_packets++;
> dev->stats.tx_bytes += skb->len;
>
> + re->skb = NULL;
> dev_kfree_skb_any(skb);
>
> sky2->tx_next = RING_NEXT(idx, sky2->tx_ring_size);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jarek Poplawski on 1 Feb 2010 15:50
On Mon, Feb 01, 2010 at 09:13:23PM +0100, Jarek Poplawski wrote:
> On Mon, Feb 01, 2010 at 10:20:18AM -0800, Stephen Hemminger wrote:
> > This fixes the fact that re->flags is always zero without causing
> > other confusion.
>
> Actually, there is a slight confusion: after tx_init() slots #0 are
> skipped and waiting for tx_complete. Of course, no big deal, but no
> problem with fixing it too, so there is the main difference between

Hmm... On the other hand it could be fixed simpler by moving
sky2->tx_cons. I'll send v3.

Thanks,
Jarek P.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Stephen Hemminger on 1 Feb 2010 17:40
On Mon, 1 Feb 2010 22:27:41 +0100
Jarek Poplawski <jarkao2(a)gmail.com> wrote:

> @@ -1038,13 +1035,14 @@ static void tx_init(struct sky2_port *sky2)
> {
> struct sky2_tx_le *le;
>
> - sky2->tx_prod = sky2->tx_cons = 0;
> + sky2->tx_prod = 0;
> sky2->tx_tcpsum = 0;
> sky2->tx_last_mss = 0;
>
> le = get_tx_le(sky2, &sky2->tx_prod);
> le->addr = 0;
> le->opcode = OP_ADDR64 | HW_OWNER;
> + sky2->tx_cons = sky2->tx_prod;
> sky2->tx_last_upper = 0;
> }

Your change causes the initial element to be skipped. I want
it to goto the hardware. It makes sure the upper bits of the
first request are set (0).

I don't see what was wrong with my fix.

--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  |  Next  |  Last
Pages: 1 2 3 4 5 6 7
Prev: [PATCH] ntp: Make time_esterror and time_maxerror static
Next: Confirm Your Email