From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756531AbcJ1LbY (ORCPT ); Fri, 28 Oct 2016 07:31:24 -0400 Received: from mx2.suse.de ([195.135.220.15]:32884 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752497AbcJ1LbV (ORCPT ); Fri, 28 Oct 2016 07:31:21 -0400 Subject: Re: [PATCH v2 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly To: Steffen Maier , Johannes Thumshirn References: <2ea07f3f-88eb-b795-fa37-a223bf80e581@linux.vnet.ibm.com> <20161013162405.aoxy3bdkc4bqtwsk@linux-x5ow.site> <4b411836-e76f-b67a-3d49-ad3d51b8f216@linux.vnet.ibm.com> Cc: "Martin K . Petersen" , Christoph Hellwig , 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)" , Ulrich Weigand , Richard Biener From: Hannes Reinecke Message-ID: Date: Fri, 28 Oct 2016 13:31:12 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <4b411836-e76f-b67a-3d49-ad3d51b8f216@linux.vnet.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/28/2016 11:53 AM, Steffen Maier wrote: > > > On 10/13/2016 06:24 PM, Johannes Thumshirn wrote: >> On Thu, Oct 13, 2016 at 05:15:25PM +0200, Steffen Maier wrote: >>> I'm puzzled. >>> >>> $ git bisect start fc_bsg master > >>>> 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; > > Compiler optimization re-ordered above two lines and the first pointer > derefence is bsg_reply->result [field offset 0] where bsg_reply is NULL. > The assignment tries to write to memory at address NULL causing the > kernel page fault. > I spoke to our compiler people, and they strongly believed this not to be the case. Or, put it the other way round, if such a thing would happen it would be a compiler issue. Have you checked the compiler output? Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking hare@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hannes Reinecke Subject: Re: [PATCH v2 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly Date: Fri, 28 Oct 2016 13:31:12 +0200 Message-ID: References: <2ea07f3f-88eb-b795-fa37-a223bf80e581@linux.vnet.ibm.com> <20161013162405.aoxy3bdkc4bqtwsk@linux-x5ow.site> <4b411836-e76f-b67a-3d49-ad3d51b8f216@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <4b411836-e76f-b67a-3d49-ad3d51b8f216-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 , Johannes Thumshirn Cc: Benjamin Herrenschmidt , Heiko Carstens , Paul Mackerras , "open list:S390 ZFCP DRIVER" , Dick Kennedy , "James E.J. Bottomley" , Linux SCSI Mailinglist , Michael Ellerman , Christoph Hellwig , "open list:FCOE SUBSYSTEM (libfc, libfcoe, fcoe)" , Ulrich Weigand , Tyrel Datwyler , "supporter:QLOGIC QLA2XXX FC-SCSI DRIVER" , Richard Biener , Sudarsana Kalluru , "Martin K . Petersen" , Anil Gurumurthy , Linux Kernel Mailinglist List-Id: linux-scsi@vger.kernel.org On 10/28/2016 11:53 AM, Steffen Maier wrote: > = > = > On 10/13/2016 06:24 PM, Johannes Thumshirn wrote: >> On Thu, Oct 13, 2016 at 05:15:25PM +0200, Steffen Maier wrote: >>> I'm puzzled. >>> >>> $ git bisect start fc_bsg master > = >>>> 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 =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; > = > Compiler optimization re-ordered above two lines and the first pointer > derefence is bsg_reply->result [field offset 0] where bsg_reply is NULL. > The assignment tries to write to memory at address NULL causing the > kernel page fault. > = I spoke to our compiler people, and they strongly believed this not to be the case. Or, put it the other way round, if such a thing would happen it would be a compiler issue. Have you checked the compiler output? Cheers, Hannes -- = Dr. Hannes Reinecke Teamlead Storage & Networking hare-l3A5Bk7waGM@public.gmane.org +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N=FCrnberg GF: F. Imend=F6rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG N=FCrnberg) 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 3t51mV2Kn6zDvR7 for ; Fri, 28 Oct 2016 22:31:21 +1100 (AEDT) Subject: Re: [PATCH v2 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly To: Steffen Maier , Johannes Thumshirn References: <2ea07f3f-88eb-b795-fa37-a223bf80e581@linux.vnet.ibm.com> <20161013162405.aoxy3bdkc4bqtwsk@linux-x5ow.site> <4b411836-e76f-b67a-3d49-ad3d51b8f216@linux.vnet.ibm.com> Cc: "Martin K . Petersen" , Christoph Hellwig , 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)" , Ulrich Weigand , Richard Biener From: Hannes Reinecke Message-ID: Date: Fri, 28 Oct 2016 13:31:12 +0200 MIME-Version: 1.0 In-Reply-To: <4b411836-e76f-b67a-3d49-ad3d51b8f216@linux.vnet.ibm.com> Content-Type: text/plain; charset=windows-1252 List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 10/28/2016 11:53 AM, Steffen Maier wrote: > > > On 10/13/2016 06:24 PM, Johannes Thumshirn wrote: >> On Thu, Oct 13, 2016 at 05:15:25PM +0200, Steffen Maier wrote: >>> I'm puzzled. >>> >>> $ git bisect start fc_bsg master > >>>> 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; > > Compiler optimization re-ordered above two lines and the first pointer > derefence is bsg_reply->result [field offset 0] where bsg_reply is NULL. > The assignment tries to write to memory at address NULL causing the > kernel page fault. > I spoke to our compiler people, and they strongly believed this not to be the case. Or, put it the other way round, if such a thing would happen it would be a compiler issue. Have you checked the compiler output? Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking hare@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)