>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:
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "[email protected]"

Reply via email to