On Sun, 14 Jun 2015, Coly Li wrote: > ? 15/6/10 ??5:20, Mikulas Patocka ??: > > Hi > > > > Here I'm sending patches that add funcionality provided by dm-latency into > > dm-statistics framework. Dm-latency was introduced in this post > > https://www.redhat.com/archives/dm-devel/2015-February/msg00158.html , > > however, there is already similar framework dm-statistics in the kernel. > > In order to have cleaner and easier to maintain code, it is better to > > integrate dm-latency functionality into dm-statistics, rather than having > > two frameworks that do similar things. > > > > This patch series makes these changes: > > * it is possible to use precise time measurement with nanosecond > > granularity (previously, time was measured in jiffies) > > * it is possible to collect histogram of latencies. The histogram > > boundaries may be selected by the user and there is unlimited number of > > possible boundaries (in dm-latency, the histogram boundaries were fixed) > > * it is possible to use dm-statistics on multipath > > > > The documentation of these extensions is in > > Documentation/device-mapper/statistics.txt > > > > I'd like to ask people who designed dm-latency to check this patchset, try > > to use it and tell us if it provides sufficient functionality to them (the > > indended functinality of dm-latency was to predict hard disk failures from > > i/o request latency histogram). > > > > Hi Mikulas, > > I was in travel last week, sorry for late response. Here are some > comments from me, just for your information, > > 1, If I understand correctly, latency histogram boundaries are organized > by list in your implementation, and the list is searched in linear > order. We had a similar but a little bit simple implementation, with > very high I/O request work load on multiple hard disks, we observed 1% > CPU performance cost. This is why dm-latency uses a very simple static > array of counters to record I/O latency. I suggest you should do some > benchmarks to compare the performance cost. It is searched using binary search - so the complexity is logarithmic. > 2, There should be a maximum latency histogram boundaries limitation. > Maybe you do it in the patch and I missed it, that's should be my fault. There is no maximum - and there doesn't need to be any maximum because the array is allocated dynamically. > 3, in message_stats_create() I see more code to handle arguments > checking. hmm, I don't check this cold block very carefully, but I > suggest that string checking in kernel space should be very careful, if > you may simplify current designment and remove the code for arguments > checking, it might be better. I don't know why should I remove the code for arguments checking. We check arguments in the kernel, even if the function may be only called by root - to prevent bugs in userspace tools from causing kernel crash. > 4, there are some minor suggestions, I will add my comments in other > emails. > > For rest of the patch set, they are good to me. Thanks for your effort > to make a good start :-) > > Coly Mikulas -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
