Date: Thu, 12 Jul 2012 10:02:09 GMT From: Steven Hartland <steven.hartland@multiplay.co.uk> To: freebsd-gnats-submit@FreeBSD.org Subject: kern/169801: Patch to make changes to delete_method in scsi_da consistent Message-ID: <201207121002.q6CA2984069566@red.freebsd.org> Resent-Message-ID: <201207121010.q6CAA1m1095463@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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@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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201207121002.q6CA2984069566>