All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bart Van Assche <Bart.VanAssche@sandisk.com>
To: "martin.petersen@oracle.com" <martin.petersen@oracle.com>
Cc: "jejb@linux.vnet.ibm.com" <jejb@linux.vnet.ibm.com>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"steve@digidescorp.com" <steve@digidescorp.com>,
	"steve.magnani@digidescorp.com" <steve.magnani@digidescorp.com>
Subject: Re: [PATCH] sd: close hole in > 2T device rejection when !CONFIG_LBDAF
Date: Tue, 4 Apr 2017 23:54:55 +0000	[thread overview]
Message-ID: <1491350095.12081.3.camel@sandisk.com> (raw)
In-Reply-To: <yq1pogrvj9y.fsf@oracle.com>

On Tue, 2017-04-04 at 19:35 -0400, Martin K. Petersen wrote:
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index fb9b4d29af0b..6084c415c070 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2102,6 +2102,16 @@ static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device *sdp,
>  
>  #define READ_CAPACITY_RETRIES_ON_RESET	10
>  
> +static bool sd_addressable_capacity(u64 lba, unsigned int sector_size)
> +{
> +	u64 last_sector = lba + 1ULL << ilog2(sector_size) - 9;
> +
> +	if (sizeof(sector_t) == 4 && last_sector > 0xffffffffULL)
> +		return false;
> +
> +	return true;
> +}

Hello Martin,

How about replacing 0xffffffffULL with U32_MAX, adding parentheses in the
last_sector computation to make clear that + and - have precedence over <<
and adding a comment above sd_addressable_capacity() that explains its
purpose and also that the shift operation must not be replaced with a call
to logical_to_sectors()? Otherwise this patch looks fine to me.

Thanks,

Bart.

      reply	other threads:[~2017-04-04 23:55 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-27 15:22 [PATCH] sd: close hole in > 2T device rejection when !CONFIG_LBDAF Steven J. Magnani
2017-02-27 16:13 ` Bart Van Assche
2017-02-27 17:13   ` Steve Magnani
2017-02-27 18:57     ` Bart Van Assche
2017-02-28  3:18       ` Martin K. Petersen
2017-02-28  3:18         ` Martin K. Petersen
2017-02-28 13:53       ` Steve Magnani
2017-04-04 23:35       ` Martin K. Petersen
2017-04-04 23:35         ` Martin K. Petersen
2017-04-04 23:54         ` Bart Van Assche [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1491350095.12081.3.camel@sandisk.com \
    --to=bart.vanassche@sandisk.com \
    --cc=jejb@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=steve.magnani@digidescorp.com \
    --cc=steve@digidescorp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.