Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 12 Dec 2018 07:40:53 -0700
From:   Keith Busch <keith.busch@intel.com>
To:     Yanjiang Jin <yanjiang.jin@hxt-semitech.com>
Cc:     bhelgaas@google.com, yu.zheng@hxt-semitech.com, ruscur@russell.cc,
        sbobroff@linux.ibm.com, oohall@gmail.com, jinyanjiang@gmail.com,
        linuxppc-dev@lists.ozlabs.org, linux-pci@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] PCI/AER: only insert one element into kfifo
Message-ID: <20181212144052.GA10780@localhost.localdomain>
References: <1544603550-14208-1-git-send-email-yanjiang.jin@hxt-semitech.com>
 <1544603550-14208-2-git-send-email-yanjiang.jin@hxt-semitech.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1544603550-14208-2-git-send-email-yanjiang.jin@hxt-semitech.com>
User-Agent: Mutt/1.9.1 (2017-09-22)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Dec 12, 2018 at 04:32:30PM +0800, Yanjiang Jin wrote:
> 'commit ecae65e133f2 ("PCI/AER: Use kfifo_in_spinlocked() to
> insert locked elements")' replace kfifo_put() with kfifo_in_spinlocked().
> 
> But as "kfifo_in(fifo, buf, n)" describes:
> " * @n: number of elements to be added".
> 
> We want to insert only one element into kfifo, not "sizeof(entry) = 16".
> Without this patch, we would get 15 uninitialized elements.
> 
> Signed-off-by: Yanjiang Jin <yanjiang.jin@hxt-semitech.com>

My bad. I had trouble testing the GHES path for this.  Thanks for the fix.

Reviewed-by: Keith Busch <keith.busch@intel.com>

> ---
>  drivers/pci/pcie/aer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index a90a919..fed29de 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1064,7 +1064,7 @@ void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn,
>                 .regs           = aer_regs,
>         };
> 
> -       if (kfifo_in_spinlocked(&aer_recover_ring, &entry, sizeof(entry),
> +       if (kfifo_in_spinlocked(&aer_recover_ring, &entry, 1,
>                                  &aer_recover_ring_lock))
>                 schedule_work(&aer_recover_work);
>         else
> --
> 1.8.3.1