I have made a number of what I would call non-structural changes to ZFS (no changes to file system semantics or space allocation) to improve the performance of ZFS, as imported in to FreeBSD, in my client''s environment. Pawel Dawidek has asked that I try to get my changes pushed upstream to minimize the pain involved in importing future ZFS versions. Among other things I found that there was a great deal of lock contention in prefetch. In light of the fact that prefetch is a purely opportunistic operation, it does not make sense to me to delay productive work in order to issue a prefetch request. In the following patch I have changed the locking operations to be ''trylock''s, failing the operation if the lock acquisition does not succeed. http://people.freebsd.org/~kmacy/dmu_zfetch_trylock.patch Constructive criticism, feedback, etc. is welcome. Thanks, Kip -- When harsh accusations depart too far from the truth, they leave bitter consequences. --Tacitus
The patch itself seems to be "purely opportunistic" :), hence two questions: 1. How does it impact prefetch efficiency? 2. If it does impact prefetch significantly, is there any other, less destructive, locking approach? Regards, Andrey On Tue, Aug 18, 2009 at 2:52 AM, Kip Macy<kmacy at freebsd.org> wrote:> I have made a number of what I would call non-structural changes to > ZFS (no changes to file system semantics or space allocation) to > improve the performance of ZFS, as imported in to FreeBSD, in my > client''s environment. Pawel Dawidek has asked that I try to get my > changes pushed upstream to minimize the pain involved in importing > future ZFS versions. > > Among other things I found that there was a great deal of lock > contention in prefetch. In light of the fact that prefetch is a purely > opportunistic operation, it does not make sense to me to delay > productive work in order to issue a prefetch request. In the following > patch I have changed the locking operations to be ''trylock''s, failing > the operation if the lock acquisition does not succeed. > > http://people.freebsd.org/~kmacy/dmu_zfetch_trylock.patch > > Constructive criticism, feedback, etc. is welcome. > > > Thanks, > Kip > > > -- > When harsh accusations depart too far from the truth, they leave > bitter consequences. > --Tacitus > _______________________________________________ > zfs-code mailing list > zfs-code at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/zfs-code >
On Tue, Aug 18, 2009 at 03:01, Andrey Kuzmin<andrey.v.kuzmin at gmail.com> wrote:> The patch itself seems to be "purely opportunistic" :), hence two questions: > 1. How does it impact prefetch efficiency?On my workloads it does not. Whereas lock contention with multiple threads accessing a file is quite high.> 2. If it does impact prefetch significantly, is there any other, less > destructive, locking approach?As the comment in the file indicates one could make the locking finer grained. -Kip
On Tue, Aug 18, 2009 at 10:57 PM, Kip Macy<kmacy at freebsd.org> wrote:> On Tue, Aug 18, 2009 at 03:01, Andrey Kuzmin<andrey.v.kuzmin at gmail.com> wrote: >> The patch itself seems to be "purely opportunistic" :), hence two questions: >> 1. How does it impact prefetch efficiency? > > On my workloads it does not. Whereas lock contention with multiple > threads accessing a file is quite high.If you could characterize "your workloads" and how much generic they are, it would be helpful to assess patch applicability. I could imagine multiple streams workload to suffer from opportunistic locking of the patch, but of course this needs further assessment.> >> 2. If it does impact prefetch significantly, is there any other, less >> destructive, locking approach? > > As the comment in the file indicates one could make the locking finer grained. >On quick dmu_zfetch.c code inspection, lock contention could be attributed to (1) O(streams) dmu_zfetch_find, done with reader lock held (which prevents concurrent stream modification), should manifest itself in contention on zf_rwlock (2) dmu_zfetch_dofetch running with stream lock held, should contend zst_lock. Judging by (unused) reference to avl_tree in zfetch typedefs below, (1) was understood at design time and could be easily fixed. http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/common/fs/zfs/sys/dmu_zfetch.h 46 typedef struct zstream { 47 uint64_t zst_offset; /* offset of starting block in range */ 48 uint64_t zst_len; /* length of range, in blocks */ 49 zfetch_dirn_t zst_direction; /* direction of prefetch */ 50 uint64_t zst_stride; /* length of stride, in blocks */ 51 uint64_t zst_ph_offset; /* prefetch offset, in blocks */ 52 uint64_t zst_cap; /* prefetch limit (cap), in blocks */ 53 kmutex_t zst_lock; /* protects stream */ 54 clock_t zst_last; /* lbolt of last prefetch */ 55 avl_node_t zst_node; /* embed avl node here */ 56 } zstream_t; 57 58 typedef struct zfetch { 59 krwlock_t zf_rwlock; /* protects zfetch structure */ 60 list_t zf_stream; /* AVL tree of zstream_t''s */ 61 struct dnode *zf_dnode; /* dnode that owns this zfetch */ 62 uint32_t zf_stream_cnt; /* # of active streams */ 63 uint64_t zf_alloc_fail; /* # of failed attempts to alloc strm */ 64 } zfetch_t; Regards, Andrey> > -Kip >