linux-kernel - Re: drivers/scsi/fnic/fdls_disc.c:263 fdls_schedule_oxid_free_retry

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fd4d8516-5eaa-43b0-9100-2984312930f8@stanley.mountain>
Date: Fri, 14 Feb 2025 18:02:35 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: oe-kbuild@...ts.linux.dev, Karan Tilak Kumar <kartilak@...co.com>
Cc: lkp@...el.com, oe-kbuild-all@...ts.linux.dev,
	linux-kernel@...r.kernel.org,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	Sesidhar Baddela <sebaddel@...co.com>,
	Gian Carlo Boffa <gcboffa@...co.com>,
	Arulprabhu Ponnusamy <arulponn@...co.com>,
	Arun Easi <aeasi@...co.com>
Subject: Re: drivers/scsi/fnic/fdls_disc.c:263
 fdls_schedule_oxid_free_retry_work() warn: inconsistent indenting

On Fri, Feb 14, 2025 at 05:42:25PM +0300, Dan Carpenter wrote:
> 
> I guess we can't call schedule_delayed_work() without holding the
> spin_lock?  It's a strange thing.

Actually, I see now that we're just dropping the lock to avoid a sleeping
in atomic warning with the GFP_ATOMIC allocation...  I bet you could make
the code under the lock much smaller.  Just the test_and_clear_bit() stuff
basically.

regards,
dan carpenter

diff --git a/drivers/scsi/fnic/fdls_disc.c b/drivers/scsi/fnic/fdls_disc.c
index 11211c469583..ed141236a4aa 100644
--- a/drivers/scsi/fnic/fdls_disc.c
+++ b/drivers/scsi/fnic/fdls_disc.c
@@ -308,31 +308,22 @@ void fdls_schedule_oxid_free_retry_work(struct work_struct *work)
 	struct fnic *fnic = iport->fnic;
 	struct reclaim_entry_s *reclaim_entry;
 	unsigned long delay_j = msecs_to_jiffies(OXID_RECLAIM_TOV(iport));
+	unsigned long flags;
 	int idx;
 
-	spin_lock_irqsave(&fnic->fnic_lock, fnic->lock_flags);
-
 	for_each_set_bit(idx, oxid_pool->pending_schedule_free, FNIC_OXID_POOL_SZ) {
 
 		FNIC_FCS_DBG(KERN_INFO, fnic->host, fnic->fnic_num,
 			"Schedule oxid free. oxid idx: %d\n", idx);
 
-		spin_unlock_irqrestore(&fnic->fnic_lock, fnic->lock_flags);
-	reclaim_entry = (struct reclaim_entry_s *)
-	kzalloc(sizeof(struct reclaim_entry_s), GFP_KERNEL);
-		spin_lock_irqsave(&fnic->fnic_lock, fnic->lock_flags);
-
+		reclaim_entry = kzalloc(sizeof(*reclaim_entry), GFP_KERNEL);
 		if (!reclaim_entry) {
-			FNIC_FCS_DBG(KERN_WARNING, fnic->host, fnic->fnic_num,
-				"Failed to allocate memory for reclaim struct for oxid idx: 0x%x\n",
-				idx);
-
 			schedule_delayed_work(&oxid_pool->schedule_oxid_free_retry,
 				msecs_to_jiffies(SCHEDULE_OXID_FREE_RETRY_TIME));
-			spin_unlock_irqrestore(&fnic->fnic_lock, fnic->lock_flags);
 			return;
 		}
 
+		spin_lock_irqsave(&fnic->fnic_lock, flags);
 		if (test_and_clear_bit(idx, oxid_pool->pending_schedule_free)) {
 			reclaim_entry->oxid_idx = idx;
 			reclaim_entry->expires = round_jiffies(jiffies + delay_j);
@@ -342,9 +333,8 @@ void fdls_schedule_oxid_free_retry_work(struct work_struct *work)
 			/* unlikely scenario, free the allocated memory and continue */
 			kfree(reclaim_entry);
 		}
-}
-
-	spin_unlock_irqrestore(&fnic->fnic_lock, fnic->lock_flags);
+		spin_unlock_irqrestore(&fnic->fnic_lock, flags);
+	}
 }
 
 static bool fdls_is_oxid_fabric_req(uint16_t oxid)