Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752324AbbGMPFS (ORCPT ); Mon, 13 Jul 2015 11:05:18 -0400 Received: from p01c11o143.mxlogic.net ([208.65.144.66]:57600 "EHLO p01c11o143.mxlogic.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751958AbbGMPFO (ORCPT ); Mon, 13 Jul 2015 11:05:14 -0400 X-MXL-Hash: 55a3d3a91ad71548-e33cc0bd64cb5a6991b44bacefe364acdbeecea4 Message-ID: <55A3D3A5.6090005@stratus.com> Date: Mon, 13 Jul 2015 11:05:09 -0400 From: Joe Lawrence User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Calvin Owens , Nagalakshmi Nandigama , Praveen Krishnamoorthy , Sreekanth Reddy , Abhijit Mahajan CC: , , , , Christoph Hellwig , Bart Van Assche Subject: Re: [PATCH 1/2] mpt2sas: Refcount sas_device objects and fix unsafe list usage References: <1433821856-2815280-1-git-send-email-calvinowens@fb.com> <1436675096-324527-1-git-send-email-calvinowens@fb.com> <1436675096-324527-2-git-send-email-calvinowens@fb.com> In-Reply-To: <1436675096-324527-2-git-send-email-calvinowens@fb.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [134.111.199.152] X-AnalysisOut: [v=2.1 cv=bOkCIZOZ c=1 sm=1 tr=0 a=VNTQBUOG7PJoPU2GWtE7Ww==] X-AnalysisOut: [:117 a=VNTQBUOG7PJoPU2GWtE7Ww==:17 a=uelBKuKpAAAA:8 a=YlVT] X-AnalysisOut: [AMxIAAAA:8 a=hTJJpgObMeYA:10 a=CdzKgOd8jloA:10 a=N659UExz7] X-AnalysisOut: [-8A:10 a=zOBTXjUuO1YA:10 a=JfrnYn6hAAAA:8 a=InJrZTXqAAAA:8] X-AnalysisOut: [ a=FOH2dFAWAAAA:8 a=QtQ9sy81bdeGt4pFvpMA:9 a=VPz4NYaq0n2Qt] X-AnalysisOut: [7HJ:21 a=UOqvaB3J_swY5lT0:21 a=pILNOxqGKmIA:10] X-Spam: [F=0.5000000000; CM=0.500; MH=0.500(2015071307); S=0.200(2014051901)] X-MAIL-FROM: X-SOURCE-IP: [134.111.1.17] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2283 Lines: 61 On 07/12/2015 12:24 AM, Calvin Owens wrote: > These objects can be referenced concurrently throughout the driver, we > need a way to make sure threads can't delete them out from under each > other. This patch adds the refcount, and refactors the code to use it. > > Additionally, we cannot iterate over the sas_device_list without > holding the lock, or we risk corrupting random memory if items are > added or deleted as we iterate. This patch refactors _scsih_probe_sas() > to use the sas_device_list in a safe way. > > Cc: Christoph Hellwig > Cc: Bart Van Assche > Signed-off-by: Calvin Owens > --- > drivers/scsi/mpt2sas/mpt2sas_base.h | 22 +- > drivers/scsi/mpt2sas/mpt2sas_scsih.c | 434 ++++++++++++++++++++----------- > drivers/scsi/mpt2sas/mpt2sas_transport.c | 12 +- > 3 files changed, 315 insertions(+), 153 deletions(-) [ ... snip ... ] > @@ -2078,7 +2150,7 @@ _scsih_slave_configure(struct scsi_device *sdev) > } > > spin_lock_irqsave(&ioc->sas_device_lock, flags); > - sas_device = mpt2sas_scsih_sas_device_find_by_sas_address(ioc, > + sas_device = __mpt2sas_get_sdev_by_addr(ioc, > sas_device_priv_data->sas_target->sas_address); > if (!sas_device) { > spin_unlock_irqrestore(&ioc->sas_device_lock, flags); > @@ -2116,13 +2188,14 @@ _scsih_slave_configure(struct scsi_device *sdev) > if (!ssp_target) > _scsih_display_sata_capabilities(ioc, handle, sdev); > > - > _scsih_change_queue_depth(sdev, qdepth); > > if (ssp_target) { > sas_read_port_mode_page(sdev); > _scsih_enable_tlr(ioc, sdev); > } > + > + sas_device_put(sas_device); > return 0; > } Hi Calvin, Any reason why this sas_device_put is placed outside the sas_device lock? Most other instances in this patch were called just before unlocking. BTW I attempted testing, but needed to port to mpt3 and ended up with a driver that didn't boot :( Hopefully I can retry later this week, or find an older mpt2 box lying around. -- Joe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/