[OmniOS-discuss] Ang: Re: Two panics now while writing eager zeros to zvols

[Dan McDonald]

> On Oct 10, 2015, at 4:49 AM, Johan Kragsterman <johan.kragsterman at capvert.se> wrote:
> 
> Dan, is this upstreamed at all...?

No it's not.  The following not-yet-upstreamed diff is a subset of a larger fix from illumos-nexenta:

diff --git a/usr/src/uts/common/io/comstar/lu/stmf_sbd/sbd_scsi.c b/usr/src/uts/common/io/comstar/lu/stmf_sbd/sbd_scsi.c
index cb6e115..7242d15 100644
--- a/usr/src/uts/common/io/comstar/lu/stmf_sbd/sbd_scsi.c
+++ b/usr/src/uts/common/io/comstar/lu/stmf_sbd/sbd_scsi.c
@@ -2347,6 +2347,7 @@ write_same_xfer_done:
                if (scmd->flags & SBD_SCSI_CMD_XFER_FAIL) {
                        stmf_scsilib_send_status(task, STATUS_CHECK,
                            STMF_SAA_WRITE_ERROR);
+                       ret = (int)SBD_FAILURE;
                } else {
                        ret = sbd_write_same_data(task, scmd);
                        if (ret != SBD_SUCCESS) {
@@ -2355,15 +2356,24 @@ write_same_xfer_done:
                        } else {
                                stmf_scsilib_send_status(task, STATUS_GOOD, 0);
                        }
+                       if ((scmd->flags & SBD_SCSI_CMD_TRANS_DATA) &&
+                           scmd->trans_data != NULL) {
+                               kmem_free(scmd->trans_data, scmd->trans_data_len);
+                               scmd->trans_data = NULL;
+                               scmd->trans_data_len = 0;
+                               scmd->flags &= ~SBD_SCSI_CMD_TRANS_DATA;
+                       }
                }
                /*
-                * Only way we should get here is via handle_write_same(),
-                * and that should make the following assertion always pass.
+                * Do the send_status afterwards, because of a potential
+                * double-free problem.
                 */
-               ASSERT((scmd->flags & SBD_SCSI_CMD_TRANS_DATA) &&
-                   scmd->trans_data != NULL);
-               kmem_free(scmd->trans_data, scmd->trans_data_len);
-               scmd->flags &= ~SBD_SCSI_CMD_TRANS_DATA;
+               if (ret != SBD_SUCCESS) {
+                       stmf_scsilib_send_status(task, STATUS_CHECK,
+                           STMF_SAA_WRITE_ERROR);
+               } else {
+                       stmf_scsilib_send_status(task, STATUS_GOOD, 0);
+               }
                return;
        }
        sbd_do_write_same_xfer(task, scmd, dbuf, dbuf_reusable);

It fixes a double-free in this path, which the eager-zeroes seems to tickle.

I'm attaching an stmf_sbd binary as well.  People affected by this can try this binary by:

1.) beadm create test-be

2.) beadm mount test-be /mnt

3.) cp stmf_sbd /mnt/kernel/drv/amd64/stmf_sbd

4.) bootadm update-archive -R /mnt

5.) Reboot, watch for grub

6.) Select "test-be" from the grub menu.  This will be a boot-once-into-the-new-BE test.

7.) Try your eager-zero test.

I look forward to seeing any experimental results.

Thanks,
Dan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: stmf_sbd
Type: application/octet-stream
Size: 164248 bytes
Desc: not available
URL: <https://omniosce.org/ml-archive/attachments/20151013/fb7db0da/attachment-0001.obj>