All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cao jin <caoj.fnst@cn.fujitsu.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH] igb: drop field "tail" of struct igb_ring
Date: Tue, 8 Nov 2016 14:07:58 +0800	[thread overview]
Message-ID: <58216BBE.5000108@cn.fujitsu.com> (raw)
In-Reply-To: <CAKgT0UcZiJJbb+-vCkhL=A2X9CB+ekp8-CnTZznEB+NBMYALFg@mail.gmail.com>



On 11/08/2016 12:12 PM, Alexander Duyck wrote:
>
>
> On Monday, November 7, 2016, Cao jin <caoj.fnst@cn.fujitsu.com
> <mailto:caoj.fnst@cn.fujitsu.com>> wrote:
>
>

>
> We removed head because it isn't really accessed very often, it is only
> really used for when the ring is configured.  Tail is accessed every
> time we add a descriptor to a ring.  The pointer chasing from ring to
> netdev to adapter to hw is expensive.  That is one of the rasons why
> we cache the pointer to the tail register.

I see. I can submit the patch as you suggested.

>
>             Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
>             ---
>                drivers/net/ethernet/intel/igb/igb.h      |  1 -
>                drivers/net/ethernet/intel/igb/igb_main.c | 16
>             +++++++++-------

>
>     hw->hw_addr could be alterred to NULL(in igb_rd32), this is why
>     writel oops the kernel, you give a fine solution.
>
>     But from the oops message, we can find, register reading returns all
>     F's, I also have a question want to consult: when igb device is
>     reset, would reading register(no matter config space or non-PCIe
>     configuration registers) during reset returns all F's? (I guess this
>     is the core of my issue)
>
>
> An all F's value means the read failed.  The device is likely off of the
> bus and the hw_addr may not have been repopulated after the reset.
>
> You might want to check the mailing list as I thought someone had
> submitted a patch recently for one of the drivers to repopulate hw_addr
> after a reset.
>

I guess you are saying this one:
   http://patchwork.ozlabs.org/patch/689592/

Seems they have a similar issue with me.


-- 
Yours Sincerely,

Cao jin



WARNING: multiple messages have this Message-ID (diff)
From: Cao jin <caoj.fnst@cn.fujitsu.com>
To: Alexander Duyck <alexander.duyck@gmail.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Netdev <netdev@vger.kernel.org>,
	"Izumi, Taku/泉 拓" <izumi.taku@jp.fujitsu.com>,
	intel-wired-lan <intel-wired-lan@lists.osuosl.org>
Subject: Re: [Intel-wired-lan] [PATCH] igb: drop field "tail" of struct igb_ring
Date: Tue, 8 Nov 2016 14:07:58 +0800	[thread overview]
Message-ID: <58216BBE.5000108@cn.fujitsu.com> (raw)
In-Reply-To: <CAKgT0UcZiJJbb+-vCkhL=A2X9CB+ekp8-CnTZznEB+NBMYALFg@mail.gmail.com>



On 11/08/2016 12:12 PM, Alexander Duyck wrote:
>
>
> On Monday, November 7, 2016, Cao jin <caoj.fnst@cn.fujitsu.com
> <mailto:caoj.fnst@cn.fujitsu.com>> wrote:
>
>

>
> We removed head because it isn't really accessed very often, it is only
> really used for when the ring is configured.  Tail is accessed every
> time we add a descriptor to a ring.  The pointer chasing from ring to
> netdev to adapter to hw is expensive.  That is one of the rasons why
> we cache the pointer to the tail register.

I see. I can submit the patch as you suggested.

>
>             Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
>             ---
>                drivers/net/ethernet/intel/igb/igb.h      |  1 -
>                drivers/net/ethernet/intel/igb/igb_main.c | 16
>             +++++++++-------

>
>     hw->hw_addr could be alterred to NULL(in igb_rd32), this is why
>     writel oops the kernel, you give a fine solution.
>
>     But from the oops message, we can find, register reading returns all
>     F's, I also have a question want to consult: when igb device is
>     reset, would reading register(no matter config space or non-PCIe
>     configuration registers) during reset returns all F's? (I guess this
>     is the core of my issue)
>
>
> An all F's value means the read failed.  The device is likely off of the
> bus and the hw_addr may not have been repopulated after the reset.
>
> You might want to check the mailing list as I thought someone had
> submitted a patch recently for one of the drivers to repopulate hw_addr
> after a reset.
>

I guess you are saying this one:
   http://patchwork.ozlabs.org/patch/689592/

Seems they have a similar issue with me.


-- 
Yours Sincerely,

Cao jin

  reply	other threads:[~2016-11-08  6:07 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-07 12:44 [Intel-wired-lan] [PATCH] igb: drop field "tail" of struct igb_ring Cao jin
2016-11-07 12:44 ` Cao jin
2016-11-07 18:49 ` [Intel-wired-lan] " Alexander Duyck
2016-11-07 18:49   ` Alexander Duyck
2016-11-08  2:50   ` Cao jin
2016-11-08  2:50     ` Cao jin
2016-11-08  4:12     ` Alexander Duyck
2016-11-08  6:07       ` Cao jin [this message]
2016-11-08  6:07         ` Cao jin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=58216BBE.5000108@cn.fujitsu.com \
    --to=caoj.fnst@cn.fujitsu.com \
    --cc=intel-wired-lan@osuosl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.