[illumos-Developer] iSCSI UNMAP with DKIOCFREE

[George Wilson]

On 2/23/11 11:59 AM, Dan McDonald wrote:
> On Wed, Feb 23, 2011 at 09:38:47AM -0800, George Wilson wrote:
>> Dan,
>>
>> The latest changes look good. I've also finished looking at
>> dmu_free_long_range() and as feared we're going to need to log an
>> intention for this in the zil so that we can replay this if a  crash
>> were ever to happen.
> So I see two possibilities after an initial exploration:
>
> 1.) Have DKIOCFREE code do a distinct ZIL intention.  I see two possible
>      places...
>
> 		rl = zfs_range_lock(&zv->zv_znode, df.df_start, df.df_length,
> 		    RL_WRITER);
                             tx = dmu_tx_create();
                             error = dmu_tx_assign(tx, TXG_WAIT);
                             if (error) {
                                 zfs_range_unlock();
                                 dmu_tx_abort(tx);
                                 break;
                             }
                             zvol_log_truncate(zv, tx, ...);

> 		error = dmu_free_long_range(zv->zv_objset, ZVOL_OBJ,
> 		    df.df_start, df.df_length);
> 		/* XXX KEBE ASKS log an intention ZIL here? */
> 		zfs_range_unlock(rl);
>
                             /* Treat all zvol free operations as 
synchronous */
                             zil_commit();
> 		if (error == 0) {
> 			/* XXX KEBE ASKS or log an intention ZIL here? */
>
> 			if (df.df_flags&  DF_WAIT_SYNC) {
> 				txg_wait_synced(
> 				    dmu_objset_pool(zv->zv_objset), 0);
> 			}
> 		}
>

I think the above is the cleanest way.

- George

>      If I understand other such code in zvol.c (e.g. zvol_write()), I need a
>      dmu tx object, a call to zvol_log_write(), and then dmu_tx_commit().
>      zvol_write() does it inside the range lock, so probably at the first
>      place.
>
> 2.) We could modify dmu_free_long_range() or ..._long_range_impl() to commit
>      to the zil itself, but that seems crufty and interface-breaking.
>
>      Unlike the dmu_write...() calls zvol_write() makes, dmu_free_long_range()
>      requires no dmu_tx_t, as it allocates and commits one itself.  And it
>      does so multiple times if "length" comprises multiple chunks (see
>      dmu_free_long_range_impl()).
>
>
> IMHO either one of these feels wrong w.r.t. the integrity of the system.
> Perhaps we introduce dmu_free_long_range_zil() which also logs to the zil,
> and then dmu_free_long_range_impl() takes another flag indicating zil
> commitment?  That'd be option #3...
>
> Again... any clues are, as always, welcome.
>
> Thanks,
> Dan
>
> _______________________________________________
> Developer mailing list
> Developer at lists.illumos.org
> http://lists.illumos.org/m/listinfo/developer