kern/169801: Patch to make changes to delete_method in scsi_da consistent

[Steven Hartland]

>Number:         169801
>Category:       kern
>Synopsis:       Patch to make changes to delete_method in scsi_da consistent
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Thu Jul 12 10:10:01 UTC 2012
>Closed-Date:
>Last-Modified:
>Originator:     Steven Hartland
>Release:        8.3-RELEASE
>Organization:
Multiplay
>Environment:
FreeBSD build 8.3-RELEASE-p3 FreeBSD 8.3-RELEASE-p3 #3: Tue Jul  3 13:16:31 UTC 2012     root at build:/usr/obj/usr/src/sys/MULTIPLAY  amd64
>Description:
Current when delete_method is changed the corresponding DISKFLAG_CANDELETE in d_flags is not always maintained causing the higher layers to get out of date information about the state of delete support on the device.
>How-To-Repeat:
N/A
>Fix:
Apply the attached patch which routes all all changes via a new method dadeletemethodset that ensures these two variables are consistent.

Note this version of the patch is against HEAD not 8.3.

Already spoken to Alexander Motin regards this change to which he said "Have no objections"

Patch attached with submission follows:

Updates delete_method sysctl changes to always maintain disk d_flags
DISKFLAG_CANDELETE. While this change makes this layer consistent
other layers such as UFS and ZFS BIO_DELETE support may not notice
any change made manually via these device sysctls until the device
is reopened via a mount.
--- sys/cam/scsi/scsi_da.c.orig	2012-07-12 08:41:10.697622095 +0000
+++ sys/cam/scsi/scsi_da.c	2012-07-12 09:46:43.309378867 +0000
@@ -841,6 +841,8 @@
 static	void		dasysctlinit(void *context, int pending);
 static	int		dacmdsizesysctl(SYSCTL_HANDLER_ARGS);
 static	int		dadeletemethodsysctl(SYSCTL_HANDLER_ARGS);
+static	int		dadeletemethodset(struct da_softc *softc,
+					  da_delete_methods delete_method);
 static	periph_ctor_t	daregister;
 static	periph_dtor_t	dacleanup;
 static	periph_start_t	dastart;
@@ -1419,7 +1421,7 @@
 	 */
 	SYSCTL_ADD_PROC(&softc->sysctl_ctx, SYSCTL_CHILDREN(softc->sysctl_tree),
 		OID_AUTO, "delete_method", CTLTYPE_STRING | CTLFLAG_RW,
-		&softc->delete_method, 0, dadeletemethodsysctl, "A",
+		softc, 0, dadeletemethodsysctl, "A",
 		"BIO_DELETE execution method");
 	SYSCTL_ADD_PROC(&softc->sysctl_ctx, SYSCTL_CHILDREN(softc->sysctl_tree),
 		OID_AUTO, "minimum_cmd_size", CTLTYPE_INT | CTLFLAG_RW,
@@ -1496,14 +1498,33 @@
 }
 
 static int
+dadeletemethodset(struct da_softc *softc, da_delete_methods delete_method)
+{
+	if (0 > delete_method || DA_DELETE_MAX < delete_method)
+		return (EINVAL);
+
+	softc->delete_method = delete_method;
+
+	if (softc->delete_method > DA_DELETE_DISABLE)
+		softc->disk->d_flags |= DISKFLAG_CANDELETE;
+	else
+		softc->disk->d_flags &= ~DISKFLAG_CANDELETE;
+
+	return (0);
+}
+
+static int
 dadeletemethodsysctl(SYSCTL_HANDLER_ARGS)
 {
 	char buf[16];
 	int error;
 	const char *p;
 	int i, value;
+	struct da_softc *softc;
 
-	value = *(int *)arg1;
+	softc = (struct da_softc *)arg1;
+
+	value = softc->delete_method;
 	if (value < 0 || value > DA_DELETE_MAX)
 		p = "UNKNOWN";
 	else
@@ -1515,8 +1536,7 @@
 	for (i = 0; i <= DA_DELETE_MAX; i++) {
 		if (strcmp(buf, da_delete_method_names[i]) != 0)
 			continue;
-		*(int *)arg1 = i;
-		return (0);
+		return dadeletemethodset(softc, i);
 	}
 	return (EINVAL);
 }
@@ -1995,24 +2015,24 @@
 		if (softc->delete_method == DA_DELETE_UNMAP) {
 			xpt_print(ccb->ccb_h.path, "UNMAP is not supported, "
 			    "switching to WRITE SAME(16) with UNMAP.\n");
-			softc->delete_method = DA_DELETE_WS16;
+			dadeletemethodset(softc, DA_DELETE_WS16);
 		} else if (softc->delete_method == DA_DELETE_WS16) {
 			xpt_print(ccb->ccb_h.path,
 			    "WRITE SAME(16) with UNMAP is not supported, "
 			    "disabling BIO_DELETE.\n");
-			softc->delete_method = DA_DELETE_DISABLE;
+			dadeletemethodset(softc, DA_DELETE_DISABLE);
 		} else if (softc->delete_method == DA_DELETE_WS10) {
 			xpt_print(ccb->ccb_h.path,
 			    "WRITE SAME(10) with UNMAP is not supported, "
 			    "disabling BIO_DELETE.\n");
-			softc->delete_method = DA_DELETE_DISABLE;
+			dadeletemethodset(softc, DA_DELETE_DISABLE);
 		} else if (softc->delete_method == DA_DELETE_ZERO) {
 			xpt_print(ccb->ccb_h.path,
 			    "WRITE SAME(10) is not supported, "
 			    "disabling BIO_DELETE.\n");
-			softc->delete_method = DA_DELETE_DISABLE;
+			dadeletemethodset(softc, DA_DELETE_DISABLE);
 		} else
-			softc->delete_method = DA_DELETE_DISABLE;
+			dadeletemethodset(softc, DA_DELETE_DISABLE);
 		while ((bp = bioq_takefirst(&softc->delete_run_queue))
 		    != NULL)
 			bioq_disksort(&softc->delete_queue, bp);
@@ -2258,7 +2278,7 @@
 					  rcaplong, sizeof(*rcaplong));
 				if ((lalba & SRC16_LBPME_A)
 				 && softc->delete_method == DA_DELETE_NONE)
-					softc->delete_method = DA_DELETE_UNMAP;
+					dadeletemethodset(softc, DA_DELETE_UNMAP);
 				dp = &softc->params;
 				snprintf(announce_buf, sizeof(announce_buf),
 				        "%juMB (%ju %u byte sectors: %dH %dS/T "


>Release-Note:
>Audit-Trail:
>Unformatted: