Re: [PATCH 1/2] pci: Block config access during BIST (resend)

From: Grant Grundler <grundler@parisc-linux.org>
To: Brian King <brking@us.ibm.com>
Cc: Grant Grundler <grundler@parisc-linux.org>,
	Paul Mackerras <paulus@samba.org>, Andrew Morton <akpm@osdl.org>,
	greg@kroah.com, matthew@wil.cx, benh@kernel.crashing.org,
	ak@muc.de, linux-kernel@vger.kernel.org,
	alan@lxorguk.ukuu.org.uk, linux-pci@atrey.karlin.mff.cuni.cz
Subject: Re: [PATCH 1/2] pci: Block config access during BIST (resend)
Date: Mon, 5 Sep 2005 22:48:09 -0600	[thread overview]
Message-ID: <20050906044809.GA19347@colo.lackof.org> (raw)
In-Reply-To: <431C8EF8.7020702@us.ibm.com>

On Mon, Sep 05, 2005 at 01:31:20PM -0500, Brian King wrote:
> That should work also. Here is an updated patch.
...

The code looks good...but it got me thinking.

...
> +void pci_block_user_cfg_access(struct pci_dev *dev)
> +{
> +	pci_save_state(dev);
> +	dev->block_ucfg_access = 1;
> +	mb();
> +	while (spin_is_locked(&pci_lock))
> +		cpu_relax();
> +}
> +EXPORT_SYMBOL_GPL(pci_block_user_cfg_access);
> +
> +/**
> + * pci_unblock_user_cfg_access - Unblock userspace PCI config reads/writes
> + * @dev:	pci device struct
> + *
> + * This function allows userspace PCI config accesses to resume.
> + **/
> +void pci_unblock_user_cfg_access(struct pci_dev *dev)
> +{
> +	dev->block_ucfg_access = 0;
> +}

Shouldn't pci_unblock_user_cfg_access() have a similar construct
as pci_block_user_cfg_access()?

I'm thinking we don't want to pull the rug out from under someone
who is accessing the saved state, right?
Or does something else guarantee that?

It wasn't obvious from this diff alone.

thanks,
grant