* [PATCH] PCI/VGA: Replace manual locks with lock guards
@ 2025-08-28 12:25 Erick Karanja
2025-08-28 12:58 ` Julia Lawall
0 siblings, 1 reply; 2+ messages in thread
From: Erick Karanja @ 2025-08-28 12:25 UTC (permalink / raw)
To: bhelgaas; +Cc: julia.lawall, linux-pci, linux-kernel, Erick Karanja
Switch from explicit lock/unlock pairs to scoped lock guards.
This simplifies error handling and improves code readability.
Generated-by: Coccinelle SmPL
Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
---
drivers/pci/vgaarb.c | 87 ++++++++++++++++----------------------------
1 file changed, 32 insertions(+), 55 deletions(-)
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index 78748e8d2dba..6a00ee00e362 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -501,8 +501,6 @@ EXPORT_SYMBOL(vga_get);
static int vga_tryget(struct pci_dev *pdev, unsigned int rsrc)
{
struct vga_device *vgadev;
- unsigned long flags;
- int rc = 0;
vga_check_first_use();
@@ -511,17 +509,13 @@ static int vga_tryget(struct pci_dev *pdev, unsigned int rsrc)
pdev = vga_default_device();
if (pdev == NULL)
return 0;
- spin_lock_irqsave(&vga_lock, flags);
+ guard(spinlock_irqsave)(&vga_lock);
vgadev = vgadev_find(pdev);
- if (vgadev == NULL) {
- rc = -ENODEV;
- goto bail;
- }
+ if (vgadev == NULL)
+ return -ENODEV;
if (__vga_tryget(vgadev, rsrc))
- rc = -EBUSY;
-bail:
- spin_unlock_irqrestore(&vga_lock, flags);
- return rc;
+ return -EBUSY;
+ return 0;
}
/**
@@ -537,20 +531,17 @@ static int vga_tryget(struct pci_dev *pdev, unsigned int rsrc)
void vga_put(struct pci_dev *pdev, unsigned int rsrc)
{
struct vga_device *vgadev;
- unsigned long flags;
/* The caller should check for this, but let's be sure */
if (pdev == NULL)
pdev = vga_default_device();
if (pdev == NULL)
return;
- spin_lock_irqsave(&vga_lock, flags);
+ guard(spinlock_irqsave)(&vga_lock);
vgadev = vgadev_find(pdev);
if (vgadev == NULL)
- goto bail;
+ return;
__vga_put(vgadev, rsrc);
-bail:
- spin_unlock_irqrestore(&vga_lock, flags);
}
EXPORT_SYMBOL(vga_put);
@@ -912,29 +903,20 @@ static void __vga_set_legacy_decoding(struct pci_dev *pdev,
bool userspace)
{
struct vga_device *vgadev;
- unsigned long flags;
decodes &= VGA_RSRC_LEGACY_MASK;
- spin_lock_irqsave(&vga_lock, flags);
+ guard(spinlock_irqsave)(&vga_lock);
vgadev = vgadev_find(pdev);
if (vgadev == NULL)
- goto bail;
+ return;
/* Don't let userspace futz with kernel driver decodes */
if (userspace && vgadev->set_decode)
- goto bail;
+ return;
/* Update the device decodes + counter */
vga_update_device_decodes(vgadev, decodes);
-
- /*
- * XXX If somebody is going from "doesn't decode" to "decodes"
- * state here, additional care must be taken as we may have pending
- * ownership of non-legacy region.
- */
-bail:
- spin_unlock_irqrestore(&vga_lock, flags);
}
/**
@@ -981,14 +963,13 @@ EXPORT_SYMBOL(vga_set_legacy_decoding);
int vga_client_register(struct pci_dev *pdev,
unsigned int (*set_decode)(struct pci_dev *pdev, bool decode))
{
- unsigned long flags;
struct vga_device *vgadev;
- spin_lock_irqsave(&vga_lock, flags);
- vgadev = vgadev_find(pdev);
- if (vgadev)
- vgadev->set_decode = set_decode;
- spin_unlock_irqrestore(&vga_lock, flags);
+ scoped_guard (spinlock_irqsave, &vga_lock) {
+ vgadev = vgadev_find(pdev);
+ if (vgadev)
+ vgadev->set_decode = set_decode;
+ }
if (!vgadev)
return -ENODEV;
return 0;
@@ -1411,7 +1392,6 @@ static __poll_t vga_arb_fpoll(struct file *file, poll_table *wait)
static int vga_arb_open(struct inode *inode, struct file *file)
{
struct vga_arb_private *priv;
- unsigned long flags;
pr_debug("%s\n", __func__);
@@ -1421,9 +1401,8 @@ static int vga_arb_open(struct inode *inode, struct file *file)
spin_lock_init(&priv->lock);
file->private_data = priv;
- spin_lock_irqsave(&vga_user_lock, flags);
- list_add(&priv->list, &vga_user_list);
- spin_unlock_irqrestore(&vga_user_lock, flags);
+ scoped_guard (spinlock_irqsave, &vga_user_lock)
+ list_add(&priv->list, &vga_user_list);
/* Set the client's lists of locks */
priv->target = vga_default_device(); /* Maybe this is still null! */
@@ -1438,25 +1417,25 @@ static int vga_arb_release(struct inode *inode, struct file *file)
{
struct vga_arb_private *priv = file->private_data;
struct vga_arb_user_card *uc;
- unsigned long flags;
int i;
pr_debug("%s\n", __func__);
- spin_lock_irqsave(&vga_user_lock, flags);
- list_del(&priv->list);
- for (i = 0; i < MAX_USER_CARDS; i++) {
- uc = &priv->cards[i];
- if (uc->pdev == NULL)
- continue;
- vgaarb_dbg(&uc->pdev->dev, "uc->io_cnt == %d, uc->mem_cnt == %d\n",
- uc->io_cnt, uc->mem_cnt);
- while (uc->io_cnt--)
- vga_put(uc->pdev, VGA_RSRC_LEGACY_IO);
- while (uc->mem_cnt--)
- vga_put(uc->pdev, VGA_RSRC_LEGACY_MEM);
+ scoped_guard (spinlock_irqsave, &vga_user_lock) {
+ list_del(&priv->list);
+ for (i = 0; i < MAX_USER_CARDS; i++) {
+ uc = &priv->cards[i];
+ if (uc->pdev == NULL)
+ continue;
+ vgaarb_dbg(&uc->pdev->dev,
+ "uc->io_cnt == %d, uc->mem_cnt == %d\n",
+ uc->io_cnt, uc->mem_cnt);
+ while (uc->io_cnt--)
+ vga_put(uc->pdev, VGA_RSRC_LEGACY_IO);
+ while (uc->mem_cnt--)
+ vga_put(uc->pdev, VGA_RSRC_LEGACY_MEM);
+ }
}
- spin_unlock_irqrestore(&vga_user_lock, flags);
kfree(priv);
@@ -1470,7 +1449,6 @@ static int vga_arb_release(struct inode *inode, struct file *file)
static void vga_arbiter_notify_clients(void)
{
struct vga_device *vgadev;
- unsigned long flags;
unsigned int new_decodes;
bool new_state;
@@ -1479,7 +1457,7 @@ static void vga_arbiter_notify_clients(void)
new_state = (vga_count > 1) ? false : true;
- spin_lock_irqsave(&vga_lock, flags);
+ guard(spinlock_irqsave)(&vga_lock);
list_for_each_entry(vgadev, &vga_list, list) {
if (vgadev->set_decode) {
new_decodes = vgadev->set_decode(vgadev->pdev,
@@ -1487,7 +1465,6 @@ static void vga_arbiter_notify_clients(void)
vga_update_device_decodes(vgadev, new_decodes);
}
}
- spin_unlock_irqrestore(&vga_lock, flags);
}
static int pci_notify(struct notifier_block *nb, unsigned long action,
--
2.43.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] PCI/VGA: Replace manual locks with lock guards
2025-08-28 12:25 [PATCH] PCI/VGA: Replace manual locks with lock guards Erick Karanja
@ 2025-08-28 12:58 ` Julia Lawall
0 siblings, 0 replies; 2+ messages in thread
From: Julia Lawall @ 2025-08-28 12:58 UTC (permalink / raw)
To: Erick Karanja; +Cc: bhelgaas, linux-pci, linux-kernel
On Thu, 28 Aug 2025, Erick Karanja wrote:
> Switch from explicit lock/unlock pairs to scoped lock guards.
> This simplifies error handling and improves code readability.
>
> Generated-by: Coccinelle SmPL
>
> Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
> ---
> drivers/pci/vgaarb.c | 87 ++++++++++++++++----------------------------
> 1 file changed, 32 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> index 78748e8d2dba..6a00ee00e362 100644
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -501,8 +501,6 @@ EXPORT_SYMBOL(vga_get);
> static int vga_tryget(struct pci_dev *pdev, unsigned int rsrc)
> {
> struct vga_device *vgadev;
> - unsigned long flags;
> - int rc = 0;
>
> vga_check_first_use();
>
> @@ -511,17 +509,13 @@ static int vga_tryget(struct pci_dev *pdev, unsigned int rsrc)
> pdev = vga_default_device();
> if (pdev == NULL)
> return 0;
> - spin_lock_irqsave(&vga_lock, flags);
> + guard(spinlock_irqsave)(&vga_lock);
> vgadev = vgadev_find(pdev);
> - if (vgadev == NULL) {
> - rc = -ENODEV;
> - goto bail;
> - }
> + if (vgadev == NULL)
> + return -ENODEV;
> if (__vga_tryget(vgadev, rsrc))
> - rc = -EBUSY;
> -bail:
> - spin_unlock_irqrestore(&vga_lock, flags);
> - return rc;
> + return -EBUSY;
> + return 0;
> }
>
> /**
> @@ -537,20 +531,17 @@ static int vga_tryget(struct pci_dev *pdev, unsigned int rsrc)
> void vga_put(struct pci_dev *pdev, unsigned int rsrc)
> {
> struct vga_device *vgadev;
> - unsigned long flags;
>
> /* The caller should check for this, but let's be sure */
> if (pdev == NULL)
> pdev = vga_default_device();
> if (pdev == NULL)
> return;
> - spin_lock_irqsave(&vga_lock, flags);
> + guard(spinlock_irqsave)(&vga_lock);
> vgadev = vgadev_find(pdev);
> if (vgadev == NULL)
> - goto bail;
> + return;
> __vga_put(vgadev, rsrc);
> -bail:
> - spin_unlock_irqrestore(&vga_lock, flags);
> }
> EXPORT_SYMBOL(vga_put);
>
> @@ -912,29 +903,20 @@ static void __vga_set_legacy_decoding(struct pci_dev *pdev,
> bool userspace)
> {
> struct vga_device *vgadev;
> - unsigned long flags;
>
> decodes &= VGA_RSRC_LEGACY_MASK;
>
> - spin_lock_irqsave(&vga_lock, flags);
> + guard(spinlock_irqsave)(&vga_lock);
> vgadev = vgadev_find(pdev);
> if (vgadev == NULL)
> - goto bail;
> + return;
>
> /* Don't let userspace futz with kernel driver decodes */
> if (userspace && vgadev->set_decode)
> - goto bail;
> + return;
>
> /* Update the device decodes + counter */
> vga_update_device_decodes(vgadev, decodes);
> -
> - /*
> - * XXX If somebody is going from "doesn't decode" to "decodes"
> - * state here, additional care must be taken as we may have pending
> - * ownership of non-legacy region.
> - */
Maybe the comment should be kept somehow?
julia
> -bail:
> - spin_unlock_irqrestore(&vga_lock, flags);
> }
>
> /**
> @@ -981,14 +963,13 @@ EXPORT_SYMBOL(vga_set_legacy_decoding);
> int vga_client_register(struct pci_dev *pdev,
> unsigned int (*set_decode)(struct pci_dev *pdev, bool decode))
> {
> - unsigned long flags;
> struct vga_device *vgadev;
>
> - spin_lock_irqsave(&vga_lock, flags);
> - vgadev = vgadev_find(pdev);
> - if (vgadev)
> - vgadev->set_decode = set_decode;
> - spin_unlock_irqrestore(&vga_lock, flags);
> + scoped_guard (spinlock_irqsave, &vga_lock) {
> + vgadev = vgadev_find(pdev);
> + if (vgadev)
> + vgadev->set_decode = set_decode;
> + }
> if (!vgadev)
> return -ENODEV;
> return 0;
> @@ -1411,7 +1392,6 @@ static __poll_t vga_arb_fpoll(struct file *file, poll_table *wait)
> static int vga_arb_open(struct inode *inode, struct file *file)
> {
> struct vga_arb_private *priv;
> - unsigned long flags;
>
> pr_debug("%s\n", __func__);
>
> @@ -1421,9 +1401,8 @@ static int vga_arb_open(struct inode *inode, struct file *file)
> spin_lock_init(&priv->lock);
> file->private_data = priv;
>
> - spin_lock_irqsave(&vga_user_lock, flags);
> - list_add(&priv->list, &vga_user_list);
> - spin_unlock_irqrestore(&vga_user_lock, flags);
> + scoped_guard (spinlock_irqsave, &vga_user_lock)
> + list_add(&priv->list, &vga_user_list);
>
> /* Set the client's lists of locks */
> priv->target = vga_default_device(); /* Maybe this is still null! */
> @@ -1438,25 +1417,25 @@ static int vga_arb_release(struct inode *inode, struct file *file)
> {
> struct vga_arb_private *priv = file->private_data;
> struct vga_arb_user_card *uc;
> - unsigned long flags;
> int i;
>
> pr_debug("%s\n", __func__);
>
> - spin_lock_irqsave(&vga_user_lock, flags);
> - list_del(&priv->list);
> - for (i = 0; i < MAX_USER_CARDS; i++) {
> - uc = &priv->cards[i];
> - if (uc->pdev == NULL)
> - continue;
> - vgaarb_dbg(&uc->pdev->dev, "uc->io_cnt == %d, uc->mem_cnt == %d\n",
> - uc->io_cnt, uc->mem_cnt);
> - while (uc->io_cnt--)
> - vga_put(uc->pdev, VGA_RSRC_LEGACY_IO);
> - while (uc->mem_cnt--)
> - vga_put(uc->pdev, VGA_RSRC_LEGACY_MEM);
> + scoped_guard (spinlock_irqsave, &vga_user_lock) {
> + list_del(&priv->list);
> + for (i = 0; i < MAX_USER_CARDS; i++) {
> + uc = &priv->cards[i];
> + if (uc->pdev == NULL)
> + continue;
> + vgaarb_dbg(&uc->pdev->dev,
> + "uc->io_cnt == %d, uc->mem_cnt == %d\n",
> + uc->io_cnt, uc->mem_cnt);
> + while (uc->io_cnt--)
> + vga_put(uc->pdev, VGA_RSRC_LEGACY_IO);
> + while (uc->mem_cnt--)
> + vga_put(uc->pdev, VGA_RSRC_LEGACY_MEM);
> + }
> }
> - spin_unlock_irqrestore(&vga_user_lock, flags);
>
> kfree(priv);
>
> @@ -1470,7 +1449,6 @@ static int vga_arb_release(struct inode *inode, struct file *file)
> static void vga_arbiter_notify_clients(void)
> {
> struct vga_device *vgadev;
> - unsigned long flags;
> unsigned int new_decodes;
> bool new_state;
>
> @@ -1479,7 +1457,7 @@ static void vga_arbiter_notify_clients(void)
>
> new_state = (vga_count > 1) ? false : true;
>
> - spin_lock_irqsave(&vga_lock, flags);
> + guard(spinlock_irqsave)(&vga_lock);
> list_for_each_entry(vgadev, &vga_list, list) {
> if (vgadev->set_decode) {
> new_decodes = vgadev->set_decode(vgadev->pdev,
> @@ -1487,7 +1465,6 @@ static void vga_arbiter_notify_clients(void)
> vga_update_device_decodes(vgadev, new_decodes);
> }
> }
> - spin_unlock_irqrestore(&vga_lock, flags);
> }
>
> static int pci_notify(struct notifier_block *nb, unsigned long action,
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-08-28 12:59 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-28 12:25 [PATCH] PCI/VGA: Replace manual locks with lock guards Erick Karanja
2025-08-28 12:58 ` Julia Lawall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).