getting to 4K disk blocks in ZFS

Justin T. Gibbs gibbs at scsiguy.com
Sat Sep 20 07:15:52 UTC 2014


On Sep 20, 2014, at 12:21 AM, Justin T. Gibbs <gibbs at scsiguy.com> wrote:

> On Sep 19, 2014, at 5:07 PM, Justin T. Gibbs <gibbs at scsiguy.com> wrote:
> 
>> On Sep 11, 2014, at 5:32 PM, Willem Jan Withagen <wjw at digiware.nl> wrote:
>> 
>>> On 11-9-2014 19:49, Peter Wemm wrote:
>>>>> Another downside is 1/4th of uberblocks, 32 vs 128.
>>>>> Also, automatic sector size detection works great for me and I've never had
>>>>> a need to manually tweak ashift.
>>>> 
>>>> Unfortunately, I have.  Same drive connected two different ways:
>>>> 
>>>> da12 at mps1 bus 0 scbus1 target 11 lun 0
>>>> da12: <ATA ST4000VN000-1H41 SC43> Fixed Direct Access SCSI-6 device 
>>>> da12: 600.000MB/s transfers
>>>> da12: Command Queueing enabled
>>>> da12: 3815447MB (7814037168 512 byte sectors: 255H 63S/T 486401C)
>>>> 
>>>> ada1 at ahcich1 bus 0 scbus3 target 0 lun 0
>>>> ada1: <ATA ST4000VN000-1H41 SC43> ATA-8 SATA 3.x device
>>>> ada1: 600.000MB/s transfers (SATA 3.x, UDMA6, PIO 8192bytes)
>>>> ada1: Command Queueing enabled
>>>> ada1: 3815447MB (7814037168 512 byte sectors:  16H 63S/T 16383C)
>>>> ada1: quirks=0x1<4K>
>>>> 
>>>> The 4k flag is missing when it's on the sas controller.  The Ident strings are 
>>>> changed.
>>>> 
>>>> This came up elsewhere recently.
>>> 
>>> I reported the same fact for the new set of WD REDs I installed.
>>> Seems that ada and da have different quirks tables...
>>> So disks on SATA connectors on the motherboard are diagnosed as being 4Kb.
>>> The disks on my twa don't get the quirk and are considered 512b
>>> 
>>> —WjW
>> 
>> I’m surprised that we have to constantly add quirks.  Are these drives really failing to report their ata params correctly?  Is there a reason we don’t currently utilize the ata params data (which is already fetched for trim/unmap detection) to also set lbppbe (logical block per physical block exponent) and lalba (lowest aligned lba)?  We may find that many of the existing quirks are unnecessary if we fix the probe code.
>> 
>>>> Justin
> 
> 
> Here’s a start at using the ata_params sector size data.  I think it needs to go a bit further and detect situations where the SCSI controller’s emulation gets the logical sector size wrong and fail to attach - but I’m out of steam for tonight.
> 
> Note that this patch is against Spectra Logic’s FreeBSD/stable/10’ish tree, so may not apply cleanly for you as is.  I can rebase it against head tomorrow if there is interest and someone else doesn’t beat me to it.

Hmm.  Attachments aren’t allowed?  Here’s the inlined diff.  Hopefully Apple Mail won’t mangle it.

—
Justin

--- //SpectraBSD/stable/sys/cam/ata/ata_all.c	2014-08-01 21:08:39.000000000 -0600
+++ /usr/home/justing/perforce/SpectraBSD/sys/cam/ata/ata_all.c	2014-08-01 21:08:39.000000000 -0600
@@ -335,7 +335,25 @@
 	printf("<%s %s %s %s>", vendor, product, revision, fw);
 }
 
+/* (L)ogical (B)locks (P)er (P)hysical (B)lock (E)xponent. */
+uint8_t
+ata_lbppbe(struct ata_params *ident_data)
+{
+	if ((ident_data->pss & ATA_PSS_VALID_MASK) != ATA_PSS_VALID_VALUE)
+		return (0);
+	return (ident_data->pss & ATA_PSS_LSPPS);
+}
+
+/* (L)owest (A)ligned (L)ogical (B)lock (A)ddress. */
 uint32_t
+ata_lalba(struct ata_params *ident_data)
+{
+	if ((ident_data->lsalign & 0xc000) != 0x4000)
+		return (0);
+	return (ident_data->lsalign & 0x3fff);
+}
+
+uint32_t
 ata_logical_sector_size(struct ata_params *ident_data)
 {
 	if ((ident_data->pss & ATA_PSS_VALID_MASK) == ATA_PSS_VALID_VALUE &&
@@ -346,28 +364,17 @@
 	return (512);
 }
 
-uint64_t
+uint32_t
 ata_physical_sector_size(struct ata_params *ident_data)
 {
-	if ((ident_data->pss & ATA_PSS_VALID_MASK) == ATA_PSS_VALID_VALUE) {
-		if (ident_data->pss & ATA_PSS_MULTLS) {
-			return ((uint64_t)ata_logical_sector_size(ident_data) *
-			    (1 << (ident_data->pss & ATA_PSS_LSPPS)));
-		} else {
-			return (uint64_t)ata_logical_sector_size(ident_data);
-		}
-	}
-	return (512);
+	return (ata_logical_sector_size(ident_data) << ata_lbppbe(ident_data));
 }
 
 uint64_t
 ata_logical_sector_offset(struct ata_params *ident_data)
 {
-	if ((ident_data->lsalign & 0xc000) == 0x4000) {
-		return ((uint64_t)ata_logical_sector_size(ident_data) *
-		    (ident_data->lsalign & 0x3fff));
-	}
-	return (0);
+	return ((uint64_t)ata_logical_sector_size(ident_data) *
+	    ata_lalba(ident_data));
 }
 
 void
--- //SpectraBSD/stable/sys/cam/ata/ata_all.h	2014-08-01 21:08:39.000000000 -0600
+++ /usr/home/justing/perforce/SpectraBSD/sys/cam/ata/ata_all.h	2014-08-01 21:08:39.000000000 -0600
@@ -111,8 +111,10 @@
 void	ata_print_ident(struct ata_params *ident_data);
 void	ata_print_ident_short(struct ata_params *ident_data);
 
+uint8_t		ata_lbppbe(struct ata_params *ident_data);
+uint32_t	ata_lalba(struct ata_params *ident_data);
 uint32_t	ata_logical_sector_size(struct ata_params *ident_data);
-uint64_t	ata_physical_sector_size(struct ata_params *ident_data);
+uint32_t	ata_physical_sector_size(struct ata_params *ident_data);
 uint64_t	ata_logical_sector_offset(struct ata_params *ident_data);
 
 void	ata_28bit_cmd(struct ccb_ataio *ataio, uint8_t cmd, uint8_t features,
--- //SpectraBSD/stable/sys/cam/scsi/scsi_da.c	2014-09-19 23:21:40.000000000 -0600
+++ /usr/home/justing/perforce/SpectraBSD/sys/cam/scsi/scsi_da.c	2014-09-19 23:21:40.000000000 -0600
@@ -2039,6 +2041,24 @@
 	daschedule(periph);
 	wakeup(&softc->disk->d_mediasize);
 	if ((softc->flags & DA_FLAG_ANNOUNCED) == 0) {
+
+		/*
+		 * Create our sysctl variables, now that we know
+		 * we have successfully attached.
+		 */
+		/* increase the refcount */
+		if (cam_periph_acquire(periph) == CAM_REQ_CMP) {
+			taskqueue_enqueue(taskqueue_thread,
+					  &softc->sysctl_task);
+			xpt_announce_periph(periph,
+					    softc->announce_buf);
+			xpt_announce_quirks(periph, softc->quirks.flags,
+			    DA_Q_BIT_STRING);
+		} else {
+			xpt_print(periph->path, "fatal error, "
+			    "could not acquire reference count\n");
+		}
+
 		softc->flags |= DA_FLAG_ANNOUNCED;
 		cam_periph_unhold(periph);
 	} else
@@ -3360,26 +3381,6 @@
 			}
 		}
 		free(csio->data_ptr, M_SCSIDA);
-		if (softc->announce_buf[0] != '\0' &&
-		    ((softc->flags & DA_FLAG_ANNOUNCED) == 0)) {
-			/*
-			 * Create our sysctl variables, now that we know
-			 * we have successfully attached.
-			 */
-			/* increase the refcount */
-			if (cam_periph_acquire(periph) == CAM_REQ_CMP) {
-				taskqueue_enqueue(taskqueue_thread,
-						  &softc->sysctl_task);
-				xpt_announce_periph(periph,
-						    softc->announce_buf);
-				xpt_announce_quirks(periph, softc->quirks.flags,
-				    DA_Q_BIT_STRING);
-			} else {
-				xpt_print(periph->path, "fatal error, "
-				    "could not acquire reference count\n");
-			}
-		}
-
 		/* We already probed the device. */
 		if (softc->flags & DA_FLAG_PROBED) {
 			daprobedone(periph, done_ccb);
@@ -3633,10 +3578,13 @@
 	}
 	case DA_CCB_PROBE_ATA:
 	{
-		int i;
+		struct scsi_read_capacity_data_long rcaplong;
 		struct ata_params *ata_params;
+		struct disk_params *dp;
 		int16_t *ptr;
+		int i;
 
+		dp = &softc->params;
 		ata_params = (struct ata_params *)csio->data_ptr;
 		ptr = (uint16_t *)ata_params;
 
@@ -3658,6 +3606,27 @@
 			 */
 			if (ata_params->media_rotation_rate == 1)
 				softc->sort_io_queue = 0;
+
+			/*
+			 * Perform our own emulation of read capacity data
+			 * rather than rely on the SCSI controller to get
+			 * it right.
+			 */
+			memset(&rcaplong, 0, sizeof(rcaplong));
+			rcaplong.prot_lbppbe = ata_lbppbe(ata_params);
+			scsi_ulto2b(ata_lalba(ata_params),
+				    rcaplong.lalba_lbp);
+			dasetgeom(periph, ata_logical_sector_size(ata_params),
+				  dp->sectors - 1, &rcaplong, sizeof(rcaplong));
+			snprintf(softc->announce_buf,
+				sizeof(softc->announce_buf),
+				"%juMB (%ju %u byte sectors: %dH %dS/T "
+				"%dC)", (uintmax_t)
+				(((uintmax_t)dp->secsize *
+				dp->sectors) / (1024*1024)),
+				(uintmax_t)dp->sectors,
+				dp->secsize, dp->heads,
+				dp->secs_per_track, dp->cylinders);
 		} else {
 			int error;
 			error = daerror(done_ccb, CAM_RETRY_SELTO,




More information about the freebsd-stable mailing list