From owner-freebsd-bugs@FreeBSD.ORG Thu Jul 12 10:10:02 2012 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 25B7E10657C1 for ; Thu, 12 Jul 2012 10:10:02 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id F1FAA8FC1D for ; Thu, 12 Jul 2012 10:10:01 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q6CAA1bQ095464 for ; Thu, 12 Jul 2012 10:10:01 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.5/8.14.5/Submit) id q6CAA1m1095463; Thu, 12 Jul 2012 10:10:01 GMT (envelope-from gnats) Resent-Date: Thu, 12 Jul 2012 10:10:01 GMT Resent-Message-Id: <201207121010.q6CAA1m1095463@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Steven Hartland Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 9EA60106564A for ; Thu, 12 Jul 2012 10:02:10 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from red.freebsd.org (red.freebsd.org [IPv6:2001:4f8:fff6::22]) by mx1.freebsd.org (Postfix) with ESMTP id 7FD838FC16 for ; Thu, 12 Jul 2012 10:02:10 +0000 (UTC) Received: from red.freebsd.org (localhost [127.0.0.1]) by red.freebsd.org (8.14.4/8.14.4) with ESMTP id q6CA2978069567 for ; Thu, 12 Jul 2012 10:02:09 GMT (envelope-from nobody@red.freebsd.org) Received: (from nobody@localhost) by red.freebsd.org (8.14.4/8.14.4/Submit) id q6CA2984069566; Thu, 12 Jul 2012 10:02:09 GMT (envelope-from nobody) Message-Id: <201207121002.q6CA2984069566@red.freebsd.org> Date: Thu, 12 Jul 2012 10:02:09 GMT From: Steven Hartland To: freebsd-gnats-submit@FreeBSD.org X-Send-Pr-Version: www-3.1 Cc: Subject: kern/169801: Patch to make changes to delete_method in scsi_da consistent X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Jul 2012 10:10:02 -0000 >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: