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 >
