在 15/6/15 下午8:47, Mikulas Patocka 写道: > > 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. It seems I didn't understand dm stats code very well, I need more time to read and understand your patches :-) I will test the patches in our storage product environment for the performance number, hope it is good. > >> 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. Maybe I am too paranoid, IMHO dynamically allocated object should have a maximum limitation, in case of it occupies too much system resource (e.g memory here). > >> 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. > I didn't express myself well. I meant to minimize the number of parameters, so most of the arguments checking code could be removed. I didn't mean not checking the arguments, I meant that it could be better if much less parameters sent from user space. Coly -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
