Getting ZFSOnLinux changes merged back into Illumos

[Matthew Ahrens]

I'm mostly going on the commit messages for the moment. Comments on each
commit below:

commit 86dd0fd9222b6103c6533036c47b908ece944460
Author: Brian Behlendorf <behlendorf1-i2BcT+NCU+***@public.gmane.org>
Date: Sun Aug 19 17:17:02 2012 -0700

Pre-allocate vdev I/O buffers


How do you know how many buffers to pre-allocate? I see the code using
zfs_vdev_max_pending, but that can be changed dynamically. There's also
code to fall back on kmem_cache_alloc() if the list is empty, which would
seem to negate any deadlock fix.

Comment says "perform a slow allocation" -- is this specifically referring
to having to reclaim memory while doing an allocation?

I'm pretty sure there are other allocations that happen in the write
pipeline. Why change just this one?

Are you sure that the problem can not be fixed by using KM_PUSHPAGE for
these allocations?

commit c93504f03a0881992689069a8f78e17933dcd5b3
Author: Brian Behlendorf <behlendorf1-i2BcT+NCU+***@public.gmane.org>
Date: Wed Jul 24 09:57:56 2013 -0700

Change l2arc_norw default to zero

fine by me

commit cb543e6b5e98546a5caec29ca4b25abec98560a2
Author: Richard Yao <ryao-aBrp7R+bbdUdnm+***@public.gmane.org>
Date: Tue Jul 9 10:45:30 2013 -0400

Remove b_thawed from arc_buf_hdr_t


This variable is used on illumos to track when a buffer was thawed. When
kmem_flags=0xf, the allocator records the stack trace when the allocation
(or free) happened. This could be made more clear with the use of
appropriately-named wrappers.

commit 3f4058cd15545a83b4e6e50cd7e29af45b54054a
Author: Richard Yao <ryao-aBrp7R+bbdUdnm+***@public.gmane.org>
Date: Sun Jul 7 13:39:59 2013 -0400

Remove arc_data_buf_alloc()/arc_data_buf_free()


looks good to me.

commit 92334b14ec378b1693573b52c09816bbade9cf3e
Author: Prakash Surya <surya1-i2BcT+NCU+***@public.gmane.org>
Date: Wed Jul 10 15:53:48 2013 -0700

Add new kstat for monitoring time in dmu_tx_assign

Neat idea. I would suggest separating out the histogram-in-a-kstat code
from this particular use of it.

commit 50210587563bb37c48d2624d11e158ab3ad30715
Author: Tim Chase <tim-5T/wZ+***@public.gmane.org>
Date: Tue Jul 9 07:15:26 2013 -0500

zdb: enhancement - Display SA xattrs.

SA xattrs are a linux-only on-disk format extension. How is the on-disk
compatibility of these handled? What happens if I move a pool from linux
to another OS that doesn't know about this representation?

commit b4f7f105275d996fbcb6abd65760307d2153a89b
Author: Ying Zhu <casualfisher-***@public.gmane.org>
Date: Sat Jun 29 15:03:49 2013 +0800

Improve code in arc_buf_remove_ref


This code does not do what the commit comment says it does. HDR_LOCK()
does not acquire a lock.

commit 64d7b6cf75e52a4698d9bdec61745573c39d2e5a
Author: Cyril Plisko <cyril.plisko-xfTqO4pScNVWk0Htik3J/***@public.gmane.org>
Date: Mon Jun 24 09:45:20 2013 +0300

Override default SPA config location via environment

looks good to me.

commit e2e229eb180fce8a67ba62c0e404d47e82c4c24d
Author: Steven Burgess <sburgess-***@public.gmane.org>
Date: Thu Jun 27 20:35:11 2013 -0400

Formating changes for zpool manpage

looks good to me

Fix incorrect assertions in ddt_phys_decref and ddt_sync_entry

I didn't realize that hole blocks showed up in the dedup table. Might want
to double check that. But the change sounds good anyway.

commit 937210a54b9c2d3dddc7221e31d5695e9720a055
Author: Brian Behlendorf <behlendorf1-i2BcT+NCU+***@public.gmane.org>
Date: Wed May 1 09:38:49 2013 -0700

Fix zinject list handlers

unclear if this is applicable to illumos, according to commit message.

commit 5dc6af0eec29b119b731c793037fd77214fc9438
Author: Brian Behlendorf <behlendorf1-i2BcT+NCU+***@public.gmane.org>
Date: Tue Mar 19 12:05:08 2013 -0700

Add zio_ddt_free()+ddt_phys_decref() error handling


I don't think we should drive on blindly when on-disk corruption is
detected. Use zfs_panic_recover() for this.

commit 2016ff96d1739b5ced1d37e7266720e7531b8212
Author: Brian Behlendorf <behlendorf1-i2BcT+NCU+***@public.gmane.org>
Date: Fri Feb 22 11:28:28 2013 -0800

Fix zdb.8 macro warning


looks good.

commit 0b4d1b5853791e1e447d74f0b229800d65b53071
Author: Eric Dillmann <eric-***@public.gmane.org>
Date: Thu Feb 14 00:11:59 2013 +0100

Add snapdev=[hidden|visible] dataset property


Idea looks good, but to preserve existing semantics, the default should be
"visible" (may not be an issue on Linux if the default there was always
"hidden").

commit b01615d5ac86913da1e092d0378bfb8f0e72af30
Author: Richard Yao <ryao-JuQBKiYWLL8cww2/***@public.gmane.org>
Date: Thu Feb 14 23:37:43 2013 -0500

Constify structures containing function pointers


seems fine.

commit dd26aa535b395735ca61ea2a3e618aded45eb05e
Author: Brian Behlendorf <behlendorf1-i2BcT+NCU+***@public.gmane.org>
Date: Mon Feb 4 16:35:54 2013 -0800

Cast 'zfs bad bloc' to ULL for x86

good fix.

commit f52b31eaf0301feeb308fbc46f696eb44d0ae523
Author: Brian Behlendorf <behlendorf1-i2BcT+NCU+***@public.gmane.org>
Date: Thu Jan 31 11:02:21 2013 -0800

Honor 80 character limit in 'zpool status'

looks good.

commit 14ee71efbc28086406bb255f2292b9535d845625
Author: Brian Behlendorf <behlendorf1-i2BcT+NCU+***@public.gmane.org>
Date: Mon Jan 28 09:53:51 2013 -0800

Use strerror() not strerror_r()

so libzfs can't be used from a multi-threaded program? That doesn't seem
like a great idea.

commit 38145d612963d0a5b80017d5d1d49c1d4f9637c2
Author: Darik Horn <dajhorn-m/***@public.gmane.org>
Date: Mon Jan 14 19:27:39 2013 -0600

Ensure that zfs diff prints unicode safely.

ok.

commit 15313c5e1866e81e2f4a30d2c50b43b5435e547a
Author: Dominik Honnef <dominikh-***@public.gmane.org>
Date: Fri Jan 4 20:09:20 2013 +0100

Fix duplicate words in zpool.8


looks good.

commit c3275b56a1470ed255441df6ff105d0c3c095d8b
Author: Brian Behlendorf <behlendorf1-i2BcT+NCU+***@public.gmane.org>
Date: Fri Nov 30 11:23:38 2012 -0800

Add load_nvlist() error handling

looks good

commit 30315d237bb23226476b348bc591589c80597351
Author: Brian Behlendorf <behlendorf1-i2BcT+NCU+***@public.gmane.org>
Date: Tue Nov 27 13:32:57 2012 -0800

Increase ZFS_OBJ_MTX_SZ to 256

ok.


commit 9dcb97198338ba2d8764dd5604b278118612f74d
Author: Brian Behlendorf <behlendorf1-i2BcT+NCU+***@public.gmane.org>
Date: Thu Oct 25 13:02:31 2012 -0700

Log I/Os longer than zio_delay_max (30s default)

seems reasonable; I'd want to get input from others who are more familiar
with FMA.

commit e95853a331529a6cb96fdf10476c53441e59f4e1
Author: Brian Behlendorf <behlendorf1-i2BcT+NCU+***@public.gmane.org>
Date: Tue Oct 23 13:48:22 2012 -0700

Add txgs-<pool> kstat file


I'm not sure that kstats are the best way to export this info. Will need
to investigate more.

commit e8fd45a0f975c6b8ae8cd644714fc21f14fac2bf
Author: Brian Behlendorf <behlendorf1-i2BcT+NCU+***@public.gmane.org>
Date: Fri Oct 26 10:01:49 2012 -0700

Add ddt_object_count() error handling

ok


--matt

Dear Everyone,
I did a cursory review of changes to the ZFSOnLinux repository made
during the past 12 months and identified a number of commits that
probably should be sent to Illumos.
I have attached a preliminary list. I did a really quick review, so I
expect that there are a few false positives included and probably a few
false negatives excluded.
I can do a more thorough review to produce a comprehensive list, but the
important thing is to work out the process to get the improvements made
in ZFSOnLinux either merged back into Illumos or rejected with some kind
of explanation why so that appropriate changes can be made in ZFSOnLinux
to reduce code differences.
Does anyone have any thoughts?
Yours truly,
Richard Yao

To unsubscribe from this group and stop receiving emails from it, send an email to zfs-devel+unsubscribe-VKpPRiiRko7s4Z89Ie/***@public.gmane.org

[Keith Wesolowski]

Post by Matthew Ahrens
commit 9dcb97198338ba2d8764dd5604b278118612f74d
Date: Thu Oct 25 13:02:31 2012 -0700
Log I/Os longer than zio_delay_max (30s default)
seems reasonable; I'd want to get input from others who are more familiar
with FMA.

This doesn't seem relevant: we don't even have zio_delay_max, and in
illumos if an I/O never completes we hit the deadman and panic (which
seems right to me, since I don't see how you recover from this). I'd
also be curious to know what's being done with this telemetry other than
exposure via 'zfs events'.

[Richard Elling]

Post by Keith Wesolowski

Post by Matthew Ahrens
commit 9dcb97198338ba2d8764dd5604b278118612f74d
Date: Thu Oct 25 13:02:31 2012 -0700
Log I/Os longer than zio_delay_max (30s default)
seems reasonable; I'd want to get input from others who are more familiar
with FMA.

This doesn't seem relevant: we don't even have zio_delay_max, and in
illumos if an I/O never completes we hit the deadman and panic (which
seems right to me, since I don't see how you recover from this). I'd
also be curious to know what's being done with this telemetry other than
exposure via 'zfs events'.

I can see where people want to know when I/Os take a suspiciously long time.
I could also argue that 30 seconds is an arbitrary and probably not useful guess
for the general case. Of course, the default in Solaris (sd) is 60 seconds, which is
equally arbitrary and arguably useless.

The question is: does this log I/Os that have not completed in 30 seconds, or does
it log I/Os that have completed in more than 30 seconds? The difference is subtle,
but important.
-- richard

--

***@RichardElling.com
[phone]












-------------------------------------------
illumos-zfs
Archives: https://www.listbox.com/member/archive/182191/=now
RSS Feed: https://www.listbox.com/member/archive/rss/182191/23047029-187a0c8d
Modify Your Subscription: https://www.listbox.com/member/?member_id=23047029&id_secret=23047029-2e85923f
Powered by Listbox: http://www.listbox.com

[Prakash Surya]

Post by Matthew Ahrens
commit 9dcb97198338ba2d8764dd5604b278118612f74d
Date:   Thu Oct 25 13:02:31 2012 -0700
   Log I/Os longer than zio_delay_max (30s default)
seems reasonable; I'd want to get input from others who are
more familiar
with FMA.
This doesn't seem relevant: we don't even have zio_delay_max, and in
illumos if an I/O never completes we hit the deadman and panic (which
seems right to me, since I don't see how you recover from this).  I'd
also be curious to know what's being done with this telemetry other than
exposure via 'zfs events'.
I can see where people want to know when I/Os take a suspiciously long time.
I could also argue that 30 seconds is an arbitrary and probably not useful guess
for the general case. Of course, the default in Solaris (sd) is 60 seconds, which is
equally arbitrary and arguably useless.
The question is: does this log I/Os that have not completed in 30 seconds, or does
it log I/Os that have completed in more than 30 seconds? The difference is subtle,
but important.

IIRC, it logs the error at IO completion time.
--
Cheers, Prakash

Post by Matthew Ahrens
 -- richard
--
[phone]
illumos-zfs | Archives [http://postlink.www.listbox.com/1598485/ [http://postlink.www.listbox.com/1598486/3379085af0f1cf7fc3708f04b4471ae2/23963346/
833487e62783d55fe81f119fb93ef644/23963346/ 8cdfacfb.png?uri=aHR0cHM6Ly93d3cubGlzdGJveC5jb20vaW1hZ2VzL2xpc3Rib3gtbG9nby1zbWFsbC5wbmc]
8cdfacfb.jpg?uri=aHR0cHM6Ly93d3cubGlzdGJveC5jb20vaW1hZ2VzL2ZlZWQtaWNvbi0xMHgxMC5qcGc]
| Modify Your Subscription

To unsubscribe from this group and stop receiving emails from it, send an email to zfs-devel+unsubscribe-VKpPRiiRko7s4Z89Ie/***@public.gmane.org