From patchwork Mon Oct 24 03:20:24 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 618700 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id EF7FEC3A59D for ; Mon, 24 Oct 2022 03:24:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230081AbiJXDYX (ORCPT ); Sun, 23 Oct 2022 23:24:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59258 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230163AbiJXDWo (ORCPT ); Sun, 23 Oct 2022 23:22:44 -0400 Received: from smtp.infotech.no (smtp.infotech.no [82.134.31.41]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id CD4E15B041 for ; Sun, 23 Oct 2022 20:21:18 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id A2E052041C0; Mon, 24 Oct 2022 05:21:17 +0200 (CEST) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id j0AXh8KwthU2; Mon, 24 Oct 2022 05:21:17 +0200 (CEST) Received: from treten.bingwo.ca (unknown [10.16.20.11]) by smtp.infotech.no (Postfix) with ESMTPA id 7D68A2041AF; Mon, 24 Oct 2022 05:21:16 +0200 (CEST) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com, hare@suse.de, bvanassche@acm.org Subject: [PATCH v25 10/44] sg: change rwlock to spinlock Date: Sun, 23 Oct 2022 23:20:24 -0400 Message-Id: <20221024032058.14077-11-dgilbert@interlog.com> X-Mailer: git-send-email 2.37.3 In-Reply-To: <20221024032058.14077-1-dgilbert@interlog.com> References: <20221024032058.14077-1-dgilbert@interlog.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org A reviewer suggested that the extra overhead associated with a rw lock compared to a spinlock was not worth it for short, oft-used critcal sections. So the rwlock on the request list/array is changed to a spinlock. The head of that list is in the owning sf file descriptor object. Reviewed-by: Hannes Reinecke Signed-off-by: Douglas Gilbert --- drivers/scsi/sg.c | 58 +++++++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index 13d9c69494d1..0665e61d448c 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -144,7 +144,7 @@ struct sg_fd { /* holds the state of a file descriptor */ struct list_head sfd_entry; /* member sg_device::sfds list */ struct sg_device *parentdp; /* owning device */ wait_queue_head_t read_wait; /* queue read until command done */ - rwlock_t rq_list_lock; /* protect access to list in req_arr */ + spinlock_t rq_list_lock; /* protect access to list in req_arr */ struct mutex f_mutex; /* protect against changes in this fd */ int timeout; /* defaults to SG_DEFAULT_TIMEOUT */ int timeout_user; /* defaults to SG_DEFAULT_TIMEOUT_USER */ @@ -815,9 +815,9 @@ srp_done(struct sg_fd *sfp, struct sg_request *srp) unsigned long flags; int ret; - read_lock_irqsave(&sfp->rq_list_lock, flags); + spin_lock_irqsave(&sfp->rq_list_lock, flags); ret = srp->done; - read_unlock_irqrestore(&sfp->rq_list_lock, flags); + spin_unlock_irqrestore(&sfp->rq_list_lock, flags); return ret; } @@ -1032,15 +1032,15 @@ sg_ioctl_common(struct file *filp, struct sg_device *sdp, struct sg_fd *sfp, return result; result = wait_event_interruptible(sfp->read_wait, srp_done(sfp, srp)); - write_lock_irq(&sfp->rq_list_lock); + spin_lock_irq(&sfp->rq_list_lock); if (srp->done) { srp->done = 2; - write_unlock_irq(&sfp->rq_list_lock); + spin_unlock_irq(&sfp->rq_list_lock); result = sg_new_read(sfp, p, SZ_SG_IO_HDR, srp); return (result < 0) ? result : 0; } srp->orphan = 1; - write_unlock_irq(&sfp->rq_list_lock); + spin_unlock_irq(&sfp->rq_list_lock); return result; /* -ERESTARTSYS because signal hit process */ case SG_SET_TIMEOUT: result = get_user(val, ip); @@ -1092,15 +1092,15 @@ sg_ioctl_common(struct file *filp, struct sg_device *sdp, struct sg_fd *sfp, sfp->force_packid = val ? 1 : 0; return 0; case SG_GET_PACK_ID: - read_lock_irqsave(&sfp->rq_list_lock, iflags); + spin_lock_irqsave(&sfp->rq_list_lock, iflags); list_for_each_entry(srp, &sfp->rq_list, entry) { if ((1 == srp->done) && (!srp->sg_io_owned)) { - read_unlock_irqrestore(&sfp->rq_list_lock, + spin_unlock_irqrestore(&sfp->rq_list_lock, iflags); return put_user(srp->header.pack_id, ip); } } - read_unlock_irqrestore(&sfp->rq_list_lock, iflags); + spin_unlock_irqrestore(&sfp->rq_list_lock, iflags); return put_user(-1, ip); case SG_GET_NUM_WAITING: return put_user(atomic_read(&sfp->waiting), ip); @@ -1169,9 +1169,9 @@ sg_ioctl_common(struct file *filp, struct sg_device *sdp, struct sg_fd *sfp, GFP_KERNEL); if (!rinfo) return -ENOMEM; - read_lock_irqsave(&sfp->rq_list_lock, iflags); + spin_lock_irqsave(&sfp->rq_list_lock, iflags); sg_fill_request_table(sfp, rinfo); - read_unlock_irqrestore(&sfp->rq_list_lock, iflags); + spin_unlock_irqrestore(&sfp->rq_list_lock, iflags); #ifdef CONFIG_COMPAT if (in_compat_syscall()) result = put_compat_request_table(p, rinfo); @@ -1466,7 +1466,7 @@ sg_rq_end_io(struct request *rq, blk_status_t status) srp->rq = NULL; blk_mq_free_request(rq); - write_lock_irqsave(&sfp->rq_list_lock, iflags); + spin_lock_irqsave(&sfp->rq_list_lock, iflags); if (unlikely(srp->orphan)) { if (sfp->keep_orphan) srp->sg_io_owned = 0; @@ -1474,7 +1474,7 @@ sg_rq_end_io(struct request *rq, blk_status_t status) done = 0; } srp->done = done; - write_unlock_irqrestore(&sfp->rq_list_lock, iflags); + spin_unlock_irqrestore(&sfp->rq_list_lock, iflags); if (likely(done)) { /* Now wake up any sg_read() that is waiting for this @@ -2129,7 +2129,7 @@ sg_get_rq_mark(struct sg_fd *sfp, int pack_id, bool *busy) unsigned long iflags; *busy = false; - write_lock_irqsave(&sfp->rq_list_lock, iflags); + spin_lock_irqsave(&sfp->rq_list_lock, iflags); list_for_each_entry(resp, &sfp->rq_list, entry) { /* look for requests that are not SG_IO owned */ if ((!resp->sg_io_owned) && @@ -2140,14 +2140,14 @@ sg_get_rq_mark(struct sg_fd *sfp, int pack_id, bool *busy) break; case 1: /* request done; response ready to return */ resp->done = 2; /* guard against other readers */ - write_unlock_irqrestore(&sfp->rq_list_lock, iflags); + spin_unlock_irqrestore(&sfp->rq_list_lock, iflags); return resp; case 2: /* response already being returned */ break; } } } - write_unlock_irqrestore(&sfp->rq_list_lock, iflags); + spin_unlock_irqrestore(&sfp->rq_list_lock, iflags); return NULL; } @@ -2159,7 +2159,7 @@ sg_setup_req(struct sg_fd *sfp) unsigned long iflags; struct sg_request *rp = sfp->req_arr; - write_lock_irqsave(&sfp->rq_list_lock, iflags); + spin_lock_irqsave(&sfp->rq_list_lock, iflags); if (!list_empty(&sfp->rq_list)) { if (!sfp->cmd_q) goto out_unlock; @@ -2175,10 +2175,10 @@ sg_setup_req(struct sg_fd *sfp) rp->parentfp = sfp; rp->header.duration = jiffies_to_msecs(jiffies); list_add_tail(&rp->entry, &sfp->rq_list); - write_unlock_irqrestore(&sfp->rq_list_lock, iflags); + spin_unlock_irqrestore(&sfp->rq_list_lock, iflags); return rp; out_unlock: - write_unlock_irqrestore(&sfp->rq_list_lock, iflags); + spin_unlock_irqrestore(&sfp->rq_list_lock, iflags); return NULL; } @@ -2191,13 +2191,13 @@ sg_remove_request(struct sg_fd *sfp, struct sg_request *srp) if (!sfp || !srp || list_empty(&sfp->rq_list)) return res; - write_lock_irqsave(&sfp->rq_list_lock, iflags); + spin_lock_irqsave(&sfp->rq_list_lock, iflags); if (!list_empty(&srp->entry)) { list_del(&srp->entry); srp->parentfp = NULL; res = 1; } - write_unlock_irqrestore(&sfp->rq_list_lock, iflags); + spin_unlock_irqrestore(&sfp->rq_list_lock, iflags); /* * If the device is detaching, wakeup any readers in case we just @@ -2222,7 +2222,7 @@ sg_add_sfp(struct sg_device *sdp) return ERR_PTR(-ENOMEM); init_waitqueue_head(&sfp->read_wait); - rwlock_init(&sfp->rq_list_lock); + spin_lock_init(&sfp->rq_list_lock); INIT_LIST_HEAD(&sfp->rq_list); kref_init(&sfp->f_ref); mutex_init(&sfp->f_mutex); @@ -2267,14 +2267,14 @@ sg_remove_sfp_usercontext(struct work_struct *work) unsigned long iflags; /* Cleanup any responses which were never read(). */ - write_lock_irqsave(&sfp->rq_list_lock, iflags); + spin_lock_irqsave(&sfp->rq_list_lock, iflags); while (!list_empty(&sfp->rq_list)) { srp = list_first_entry(&sfp->rq_list, struct sg_request, entry); sg_finish_scsi_blk_rq(srp); list_del(&srp->entry); srp->parentfp = NULL; } - write_unlock_irqrestore(&sfp->rq_list_lock, iflags); + spin_unlock_irqrestore(&sfp->rq_list_lock, iflags); if (sfp->reserve.buflen > 0) { SG_LOG(6, sfp, "%s: buflen=%d, num_sgat=%d\n", __func__, @@ -2556,9 +2556,9 @@ sg_proc_seq_show_dev(struct seq_file *s, void *v) scsidp->host->host_no, scsidp->channel, scsidp->id, scsidp->lun, (int) scsidp->type, 1, - (int) scsidp->queue_depth, - (int) scsi_device_busy(scsidp), - (int) scsi_device_online(scsidp)); + (int)scsidp->queue_depth, + (int)scsi_device_busy(scsidp), + (int)scsi_device_online(scsidp)); } read_unlock_irqrestore(&sg_index_lock, iflags); return 0; @@ -2598,7 +2598,7 @@ sg_proc_debug_helper(struct seq_file *s, struct sg_device *sdp) k = 0; list_for_each_entry(fp, &sdp->sfds, sfd_entry) { k++; - read_lock(&fp->rq_list_lock); /* irqs already disabled */ + spin_lock(&fp->rq_list_lock); /* irqs already disabled */ seq_printf(s, " FD(%d): timeout=%dms buflen=%d (res)sgat=%d\n", k, jiffies_to_msecs(fp->timeout), fp->reserve.buflen, @@ -2646,7 +2646,7 @@ sg_proc_debug_helper(struct seq_file *s, struct sg_device *sdp) } if (list_empty(&fp->rq_list)) seq_puts(s, " No requests active\n"); - read_unlock(&fp->rq_list_lock); + spin_unlock(&fp->rq_list_lock); } }