From patchwork Wed Jun 3 13:37:03 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Bottomley X-Patchwork-Id: 6538141 Return-Path: X-Original-To: patchwork-linux-scsi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 7B717C0020 for ; Wed, 3 Jun 2015 13:37:16 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id EA5D0206CE for ; Wed, 3 Jun 2015 13:37:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4AAAE206CB for ; Wed, 3 Jun 2015 13:37:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753406AbbFCNhO (ORCPT ); Wed, 3 Jun 2015 09:37:14 -0400 Received: from mx2.parallels.com ([199.115.105.18]:42908 "EHLO mx2.parallels.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752992AbbFCNhL (ORCPT ); Wed, 3 Jun 2015 09:37:11 -0400 Received: from mail.parallels.com ([199.115.105.252]) by mx2.parallels.com with esmtps (TLSv1:AES128-SHA:128) (Exim 4.85) (envelope-from ) id 1Z08qr-00070N-J8; Wed, 03 Jun 2015 06:37:01 -0700 Received: from US-EXCH1.sw.swsoft.com ([fe80::2036:e1e5:b63e:bd9b]) by US-EXCH1.sw.swsoft.com ([fe80::2036:e1e5:b63e:bd9b%10]) with mapi id 14.03.0224.002; Wed, 3 Jun 2015 06:37:03 -0700 From: James Bottomley To: "jslaby@suse.cz" CC: "geert@linux-m68k.org" , "linux-scsi@vger.kernel.org" , "dmarkh@cfl.rr.com" , "hare@suse.de" , "gregkh@linuxfoundation.org" , "stable@vger.kernel.org" , "stable-commits@vger.kernel.org" Subject: Re: Patch "sd: Disable support for 256 byte/sector disks" has been added to the 4.0-stable tree Thread-Topic: Patch "sd: Disable support for 256 byte/sector disks" has been added to the 4.0-stable tree Thread-Index: AQHQnceJwBiw7M5xMUCYJup+aaxZ/Z2a+VgAgABFLQA= Date: Wed, 3 Jun 2015 13:37:03 +0000 Message-ID: <1433338620.2251.57.camel@Odin.com> References: <14333133244847@kroah.com> <556EC8F4.7030507@suse.cz> In-Reply-To: <556EC8F4.7030507@suse.cz> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [50.46.149.214] Content-ID: <4E8880A1D044AC4B89FD9DADFF4AF1E2@sw.swsoft.com> MIME-Version: 1.0 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, 2015-06-03 at 11:29 +0200, Jiri Slaby wrote: > On 06/03/2015, 08:35 AM, gregkh@linuxfoundation.org wrote: > > From 74856fbf441929918c49ff262ace9835048e4e6a Mon Sep 17 00:00:00 2001 > > From: Mark Hounschell > > Date: Wed, 13 May 2015 10:49:09 +0200 > > Subject: sd: Disable support for 256 byte/sector disks > > > > From: Mark Hounschell > > > > commit 74856fbf441929918c49ff262ace9835048e4e6a upstream. > > > > 256 bytes per sector support has been broken since 2.6.X, > > and no-one stepped up to fix this. > > So disable support for it. > > > > Signed-off-by: Mark Hounschell > > Signed-off-by: Hannes Reinecke > > Signed-off-by: James Bottomley > > Signed-off-by: Greg Kroah-Hartman > > > > --- > > drivers/scsi/sd.c | 19 +++++-------------- > > 1 file changed, 5 insertions(+), 14 deletions(-) > > > > --- a/drivers/scsi/sd.c > > +++ b/drivers/scsi/sd.c > > @@ -1624,6 +1624,7 @@ static unsigned int sd_completed_bytes(s > > { > > u64 start_lba = blk_rq_pos(scmd->request); > > u64 end_lba = blk_rq_pos(scmd->request) + (scsi_bufflen(scmd) / 512); > > + u64 factor = scmd->device->sector_size / 512; > > u64 bad_lba; > > int info_valid; > > /* > > @@ -1645,16 +1646,9 @@ static unsigned int sd_completed_bytes(s > > if (scsi_bufflen(scmd) <= scmd->device->sector_size) > > return 0; > > > > - if (scmd->device->sector_size < 512) { > > - /* only legitimate sector_size here is 256 */ > > - start_lba <<= 1; > > - end_lba <<= 1; > > - } else { > > - /* be careful ... don't want any overflows */ > > - unsigned int factor = scmd->device->sector_size / 512; > > - do_div(start_lba, factor); > > - do_div(end_lba, factor); > > - } > > Hmm, you do 'unsigned int' -> 'u64' switch of factor type here. But this > commit: > commit ef80d1e18b014af08741cf688e3fdda1fb71363f > Author: Geert Uytterhoeven > Date: Mon Nov 4 10:21:05 2013 +0100 > > [SCSI] sd: Do not call do_div() with a 64-bit divisor > > did the switch in the opposite direction deliberately. > > So why did you do the change, given sector_size is uint? Primarily because no-one spotted the reversal and none of the static checkers warns about it. This is the trivial fix, but we should do something about the checkers. James diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 3b2fcb4..3e137dd 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1600,7 +1600,7 @@ static unsigned int sd_completed_bytes(struct scsi_cmnd *scmd) { u64 start_lba = blk_rq_pos(scmd->request); u64 end_lba = blk_rq_pos(scmd->request) + (scsi_bufflen(scmd) / 512); - u64 factor = scmd->device->sector_size / 512; + unsigned int factor = scmd->device->sector_size / 512; u64 bad_lba; int info_valid; /*