On Thu, Nov 18, 2021 at 02:06:41PM +0900, Damien Le Moal wrote: > On 2021/11/17 18:26, Niklas Cassel wrote: > > Increasing DEF_LOG_ENTRIES can be done independently of this patch. the option > log_entries defaults to DEF_LOG_ENTRIES and use this value as the min value too. > So changing DEF_LOG_ENTRIES will also have log_entries follow the change. Agreed. > > > While it would be hard to figure out a new default, e.g. bumping it to 32768 > > would mean 40 * 32768 = 1.26 MB, if all 6 logs are enabled = 7.5 MB per > > struct thread_data. Sure, someone can have thousands of threads, but I doubt > > that you would have logging enabled for all those threads. > > If you have logging enabled, the most common case is probably to have slat, > > clat and lat logs, which means 3 different logs: > > 1.26 MB * 3 = 3.76 MB per struct thread_data, however, this dynamic memory > > allocation will only be done if you have explicitly enabled any of the log > > files in the first place. > > > > TL;DR: I think that we can merge this patch AND increase the default number > > of log entries to e.g. 32768. (Or if we want to be quite conservative, > > perhaps only bump it to something like 16384, which would only be less than > > 2 MB per struct thread_data if 3 different log files are enabled.) > > Not sure. On embedded systems with slow storage (e.g. emmc SD card), increasing > the default log size may not be desired. > > DEF_LOG_ENTRIES as is seems to not be an issue for most cases. So I think we > should just leave it as is for now. The new log_entries option now allows larger > logs if one needs/want one. The problem IMO is that the buffer is so small, that there will be a IO quiesce + reallocation within the first couple of seconds. As you have shown, the IO quiesce can totally distort the tail latencies. It is nice that this option exist, but how should I know that my result is distorted, and that I need to use this option to increase the initial log buffer size? On my embedded system, I would personally prefer an error message saying: "Could not allocate log buffer, try reducing the buffer size. Note that this might inflate the tail latency result." Instead of silently distorting the tail latency result. It took us quite long to realize that it was the IO quiesce that distorted the result. We might be able to save others from the same pitfall. Doing a simple thing as just increasing the default by x2 or x4, might be worthwhile, as that will avoid one or two IO quiesce, and might reduce the impact on the steady state. However, in the end I guess that it is up to Jens to decide whatever he prefers. Kind regards, Niklas
