All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Hari Vyas <hari.vyas@broadcom.com>
Cc: bhelgaas@google.com, benh@kernel.crashing.org,
	linux-pci@vger.kernel.org, ray.jui@broadcom.com
Subject: Re: [PATCH v2 1/3] PCI: Data corruption happening due to race condition
Date: Sun, 1 Jul 2018 16:06:34 +0200	[thread overview]
Message-ID: <20180701140634.GA22322@wunner.de> (raw)
In-Reply-To: <1530268061-17324-2-git-send-email-hari.vyas@broadcom.com>

On Fri, Jun 29, 2018 at 03:57:39PM +0530, Hari Vyas wrote:
> Fix moves device addition is_added bit to separate private flag
> variable and use different atomic functions to set, clear and
> retrieve device addition state. As is_added shares different
> memory location so race condition is avoided.

As 0-day has already discovered, you need to squash all 3 patches
together to avoid breaking the build.


> +static inline int pci_dev_set_added(struct pci_dev *dev, void *unused)
> +{
> +	set_bit(PCI_DEV_ADDED, &dev->priv_flags);
> +	return 0;
> +}
> +
> +static inline int pci_dev_clear_added(struct pci_dev *dev, void *unused)
> +{
> +	clear_bit(PCI_DEV_ADDED, &dev->priv_flags);
> +	return 0;
> +}

You don't need the "unused" parameter here and you can return void.
pci_dev_set_disconnected() has the parameter because the function is
passed in to pci_walk_bus() in a few places, but you're not doing that
AFAICS.

What you *could* do however is collapse pci_dev_set_added() and
pci_dev_clear_added() into a single function, pass in a bool "added",
then use assign_bit() to set or clear it.  It would save 6 LoC.

Thanks,

Lukas

  parent reply	other threads:[~2018-07-01 14:06 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-29 10:27 [PATCH v2 0/3] PCI: Data corruption happening due to race condition Hari Vyas
2018-06-29 10:27 ` [PATCH v2 1/3] " Hari Vyas
2018-06-29 14:38   ` kbuild test robot
2018-06-29 18:22   ` kbuild test robot
2018-07-01 14:06   ` Lukas Wunner [this message]
2018-07-02 14:20     ` Hari Vyas
2018-06-29 10:27 ` [PATCH v2 2/3] PCI: use new pci function to get device addition state Hari Vyas
2018-06-29 10:27 ` [PATCH v2 3/3] PCI: Hotplug: " Hari Vyas

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=20180701140634.GA22322@wunner.de \
    --to=lukas@wunner.de \
    --cc=benh@kernel.crashing.org \
    --cc=bhelgaas@google.com \
    --cc=hari.vyas@broadcom.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=ray.jui@broadcom.com \
    /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.