From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756561AbcJMQZU (ORCPT ); Thu, 13 Oct 2016 12:25:20 -0400 Received: from mx2.suse.de ([195.135.220.15]:55107 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753729AbcJMQZC (ORCPT ); Thu, 13 Oct 2016 12:25:02 -0400 Date: Thu, 13 Oct 2016 18:24:05 +0200 From: Johannes Thumshirn To: Steffen Maier Cc: "Martin K . Petersen" , Christoph Hellwig , Hannes Reinecke , Linux Kernel Mailinglist , Linux SCSI Mailinglist , Martin Schwidefsky , Heiko Carstens , Anil Gurumurthy , Sudarsana Kalluru , "James E.J. Bottomley" , Tyrel Datwyler , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Johannes Thumshirn , James Smart , Dick Kennedy , "supporter:QLOGIC QLA2XXX FC-SCSI DRIVER" , "open list:S390 ZFCP DRIVER" , "open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)" , "open list:FCOE SUBSYSTEM (libfc, libfcoe, fcoe)" Subject: Re: [PATCH v2 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly Message-ID: <20161013162405.aoxy3bdkc4bqtwsk@linux-x5ow.site> References: <2ea07f3f-88eb-b795-fa37-a223bf80e581@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <2ea07f3f-88eb-b795-fa37-a223bf80e581@linux.vnet.ibm.com> User-Agent: Mutt/1.6.2 (2016-07-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Oct 13, 2016 at 05:15:25PM +0200, Steffen Maier wrote: > I'm puzzled. > > $ git bisect start fc_bsg master > Bisecting: 8 revisions left to test after this (roughly 3 steps) > [005d51510eee6102636d5dbb06310531c5d46151] scsi: fc: implement kref backed > reference counting > $ git bisect bad > Bisecting: 3 revisions left to test after this (roughly 2 steps) > [bef6da201de1bb81bb4d9511f9a155862efc251f] scsi: Unify interfaces of > fc_bsg_jobdone and bsg_job_done > $ git bisect bad > Bisecting: 1 revision left to test after this (roughly 1 step) > [3087864ce3d7282f59021245d8a5f83ef1caef18] scsi: don't use > fc_bsg_job::request and fc_bsg_job::reply directly > $ git bisect bad > Bisecting: 0 revisions left to test after this (roughly 0 steps) > [81aea44720d22d2e0c4a2613ae8b1c256ef6b0cb] scsi: Get rid of struct > fc_bsg_buffer > > $ git bisect good > > 3087864ce3d7282f59021245d8a5f83ef1caef18 is the first bad commit > > commit 3087864ce3d7282f59021245d8a5f83ef1caef18 > > Author: Johannes Thumshirn > > Date: Wed Oct 12 15:06:28 2016 +0200 > > > > scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly > > > > Don't use fc_bsg_job::request and fc_bsg_job::reply directly, but use > > helper variables bsg_request and bsg_reply. This will be helpfull when > > transitioning to bsg-lib. > > > > Signed-off-by: Johannes Thumshirn > > > > :040000 040000 140c4b6829d5cfaec4079716e0795f63f8bc3bd2 0d9fe225615679550be91fbd9f84c09ab1e280fc M drivers > > From there (on the reverse bisect path) I get the following Oops, > except for the full patch set having another stack trace as in my previous > mail (dying in zfcp code). > [...] > > > @@ -3937,6 +3944,7 @@ fc_bsg_request_handler(struct request_queue *q, struct Scsi_Host *shost, > > struct request *req; > > struct fc_bsg_job *job; > > enum fc_dispatch_result ret; > > + struct fc_bsg_reply *bsg_reply; > > > > if (!get_device(dev)) > > return; > > @@ -3973,8 +3981,9 @@ fc_bsg_request_handler(struct request_queue *q, struct Scsi_Host *shost, > > /* check if we have the msgcode value at least */ > > if (job->request_len < sizeof(uint32_t)) { > > BUG_ON(job->reply_len < sizeof(uint32_t)); > > - job->reply->reply_payload_rcv_len = 0; > > - job->reply->result = -ENOMSG; > > + bsg_reply = job->reply; > > + bsg_reply->reply_payload_rcv_len = 0; > > + bsg_reply->result = -ENOMSG; > > job->reply_len = sizeof(uint32_t); > > fc_bsg_jobdone(job); > > spin_lock_irq(q->queue_lock); > > Ahm and what exactly can break here? It's just assigning variables. Now I'm puzzled too. I'll have to look into it tomorrow. Byte, Johannes -- Johannes Thumshirn Storage jthumshirn@suse.de +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Johannes Thumshirn Subject: Re: [PATCH v2 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly Date: Thu, 13 Oct 2016 18:24:05 +0200 Message-ID: <20161013162405.aoxy3bdkc4bqtwsk@linux-x5ow.site> References: <2ea07f3f-88eb-b795-fa37-a223bf80e581@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <2ea07f3f-88eb-b795-fa37-a223bf80e581-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: fcoe-devel-bounces-s9riP+hp16TNLxjTenLetw@public.gmane.org Sender: "fcoe-devel" To: Steffen Maier Cc: "open list:S390 ZFCP DRIVER" , Dick Kennedy , "James E.J. Bottomley" , "Martin K . Petersen" , Linux SCSI Mailinglist , Michael Ellerman , Anil Gurumurthy , "supporter:QLOGIC QLA2XXX FC-SCSI DRIVER" , Sudarsana Kalluru , Heiko Carstens , Linux Kernel Mailinglist , "open list:FCOE SUBSYSTEM (libfc, libfcoe, fcoe)" , Christoph Hellwig , James Smart , Paul Mackerras , Benjamin Herrenschmidt , Martin Schwidefsky , Johannes Thumshirn List-Id: linux-scsi@vger.kernel.org On Thu, Oct 13, 2016 at 05:15:25PM +0200, Steffen Maier wrote: > I'm puzzled. > = > $ git bisect start fc_bsg master > Bisecting: 8 revisions left to test after this (roughly 3 steps) > [005d51510eee6102636d5dbb06310531c5d46151] scsi: fc: implement kref backed > reference counting > $ git bisect bad > Bisecting: 3 revisions left to test after this (roughly 2 steps) > [bef6da201de1bb81bb4d9511f9a155862efc251f] scsi: Unify interfaces of > fc_bsg_jobdone and bsg_job_done > $ git bisect bad > Bisecting: 1 revision left to test after this (roughly 1 step) > [3087864ce3d7282f59021245d8a5f83ef1caef18] scsi: don't use > fc_bsg_job::request and fc_bsg_job::reply directly > $ git bisect bad > Bisecting: 0 revisions left to test after this (roughly 0 steps) > [81aea44720d22d2e0c4a2613ae8b1c256ef6b0cb] scsi: Get rid of struct > fc_bsg_buffer > > $ git bisect good > > 3087864ce3d7282f59021245d8a5f83ef1caef18 is the first bad commit > > commit 3087864ce3d7282f59021245d8a5f83ef1caef18 > > Author: Johannes Thumshirn > > Date: Wed Oct 12 15:06:28 2016 +0200 > > = > > scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly > > = > > Don't use fc_bsg_job::request and fc_bsg_job::reply directly, but u= se > > helper variables bsg_request and bsg_reply. This will be helpfull = when > > transitioning to bsg-lib. > > = > > Signed-off-by: Johannes Thumshirn > > = > > :040000 040000 140c4b6829d5cfaec4079716e0795f63f8bc3bd2 0d9fe2256156795= 50be91fbd9f84c09ab1e280fc M drivers > = > From there (on the reverse bisect path) I get the following Oops, > except for the full patch set having another stack trace as in my previous > mail (dying in zfcp code). > = [...] > = > > @@ -3937,6 +3944,7 @@ fc_bsg_request_handler(struct request_queue *q, s= truct Scsi_Host *shost, > > struct request *req; > > struct fc_bsg_job *job; > > enum fc_dispatch_result ret; > > + struct fc_bsg_reply *bsg_reply; > > = > > if (!get_device(dev)) > > return; > > @@ -3973,8 +3981,9 @@ fc_bsg_request_handler(struct request_queue *q, s= truct Scsi_Host *shost, > > /* check if we have the msgcode value at least */ > > if (job->request_len < sizeof(uint32_t)) { > > BUG_ON(job->reply_len < sizeof(uint32_t)); > > - job->reply->reply_payload_rcv_len =3D 0; > > - job->reply->result =3D -ENOMSG; > > + bsg_reply =3D job->reply; > > + bsg_reply->reply_payload_rcv_len =3D 0; > > + bsg_reply->result =3D -ENOMSG; > > job->reply_len =3D sizeof(uint32_t); > > fc_bsg_jobdone(job); > > spin_lock_irq(q->queue_lock); > > = Ahm and what exactly can break here? It's just assigning variables. Now I'm puzzled too. I'll have to look into it tomorrow. Byte, Johannes -- = Johannes Thumshirn Storage jthumshirn-l3A5Bk7waGM@public.gmane.org +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N=FCrnberg GF: Felix Imend=F6rffer, Jane Smithard, Graham Norton HRB 21284 (AG N=FCrnberg) Key fingerprint =3D EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) (using TLSv1 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3svwzS0DSZzDt0d for ; Fri, 14 Oct 2016 03:24:20 +1100 (AEDT) Date: Thu, 13 Oct 2016 18:24:05 +0200 From: Johannes Thumshirn To: Steffen Maier Cc: "Martin K . Petersen" , Christoph Hellwig , Hannes Reinecke , Linux Kernel Mailinglist , Linux SCSI Mailinglist , Martin Schwidefsky , Heiko Carstens , Anil Gurumurthy , Sudarsana Kalluru , "James E.J. Bottomley" , Tyrel Datwyler , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Johannes Thumshirn , James Smart , Dick Kennedy , "supporter:QLOGIC QLA2XXX FC-SCSI DRIVER" , "open list:S390 ZFCP DRIVER" , "open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)" , "open list:FCOE SUBSYSTEM (libfc, libfcoe, fcoe)" Subject: Re: [PATCH v2 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly Message-ID: <20161013162405.aoxy3bdkc4bqtwsk@linux-x5ow.site> References: <2ea07f3f-88eb-b795-fa37-a223bf80e581@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 In-Reply-To: <2ea07f3f-88eb-b795-fa37-a223bf80e581@linux.vnet.ibm.com> List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Thu, Oct 13, 2016 at 05:15:25PM +0200, Steffen Maier wrote: > I'm puzzled. > > $ git bisect start fc_bsg master > Bisecting: 8 revisions left to test after this (roughly 3 steps) > [005d51510eee6102636d5dbb06310531c5d46151] scsi: fc: implement kref backed > reference counting > $ git bisect bad > Bisecting: 3 revisions left to test after this (roughly 2 steps) > [bef6da201de1bb81bb4d9511f9a155862efc251f] scsi: Unify interfaces of > fc_bsg_jobdone and bsg_job_done > $ git bisect bad > Bisecting: 1 revision left to test after this (roughly 1 step) > [3087864ce3d7282f59021245d8a5f83ef1caef18] scsi: don't use > fc_bsg_job::request and fc_bsg_job::reply directly > $ git bisect bad > Bisecting: 0 revisions left to test after this (roughly 0 steps) > [81aea44720d22d2e0c4a2613ae8b1c256ef6b0cb] scsi: Get rid of struct > fc_bsg_buffer > > $ git bisect good > > 3087864ce3d7282f59021245d8a5f83ef1caef18 is the first bad commit > > commit 3087864ce3d7282f59021245d8a5f83ef1caef18 > > Author: Johannes Thumshirn > > Date: Wed Oct 12 15:06:28 2016 +0200 > > > > scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly > > > > Don't use fc_bsg_job::request and fc_bsg_job::reply directly, but use > > helper variables bsg_request and bsg_reply. This will be helpfull when > > transitioning to bsg-lib. > > > > Signed-off-by: Johannes Thumshirn > > > > :040000 040000 140c4b6829d5cfaec4079716e0795f63f8bc3bd2 0d9fe225615679550be91fbd9f84c09ab1e280fc M drivers > > From there (on the reverse bisect path) I get the following Oops, > except for the full patch set having another stack trace as in my previous > mail (dying in zfcp code). > [...] > > > @@ -3937,6 +3944,7 @@ fc_bsg_request_handler(struct request_queue *q, struct Scsi_Host *shost, > > struct request *req; > > struct fc_bsg_job *job; > > enum fc_dispatch_result ret; > > + struct fc_bsg_reply *bsg_reply; > > > > if (!get_device(dev)) > > return; > > @@ -3973,8 +3981,9 @@ fc_bsg_request_handler(struct request_queue *q, struct Scsi_Host *shost, > > /* check if we have the msgcode value at least */ > > if (job->request_len < sizeof(uint32_t)) { > > BUG_ON(job->reply_len < sizeof(uint32_t)); > > - job->reply->reply_payload_rcv_len = 0; > > - job->reply->result = -ENOMSG; > > + bsg_reply = job->reply; > > + bsg_reply->reply_payload_rcv_len = 0; > > + bsg_reply->result = -ENOMSG; > > job->reply_len = sizeof(uint32_t); > > fc_bsg_jobdone(job); > > spin_lock_irq(q->queue_lock); > > Ahm and what exactly can break here? It's just assigning variables. Now I'm puzzled too. I'll have to look into it tomorrow. Byte, Johannes -- Johannes Thumshirn Storage jthumshirn@suse.de +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850