Hi Noah,

On Thu, 2021-07-08 at 15:22 -0400, Noah Sanci via Elfutils-devel wrote:
> Please find the patch information for pr25978 attached.

Thanks for allowing zero as fd and mb size, updating the documentation
and adding comments for where I misunderstood what the code was doing
in the previous review. I like the trick using grep of the vlog to make
sure the reported metrics are correct (although that is slightly
cheating, the metrics report what the log logs). Just one question
about the last test:

> +########################################################################
> +## PR25978
> +# Ensure that the fdcache options are working.
> +grep "prefetch fds" vlog$PORT1
> +grep "prefetch mbs" vlog$PORT1
> +grep "fdcache fds" vlog$PORT1
> +grep "fdcache mbs" vlog$PORT1
> +# search the vlog to find what metric counts should be and check the correct 
> metrics
> +# were incrimented
> +wait_ready $PORT1 'fdcache_op_count{op="enqueue"}' $( grep -c 
> 'interned.*front=1' vlog$PORT1 )
> +wait_ready $PORT1 'fdcache_op_count{op="evict"}' $( grep -c 'evicted a=.*' 
> vlog$PORT1 )
> +wait_ready $PORT1 'fdcache_op_count{op="prefetch_enqueue"}' $( grep -c 
> 'interned.*front=0' vlog$PORT1 )
> +wait_ready $PORT1 'fdcache_op_count{op="prefetch_evict"}' $( grep -c 
> 'evicted from prefetch a=.*front=0' vlog$PORT1 || true )
> +# if all enqueued items are also evicted then we should expect prefetch to 
> be empty
> +if [ $PF_ENQUEUED -eq $PF_EVICTED ]; then
> +    wait_ready $PORT1 'fdcache_prefetch_bytes' 0
> +    wait_ready $PORT1 'fdcache_prefetch_count' 0
> +fi

PF_ENQUEUED and PF_EVICTED are not used or set anywhere, so the if is
always true, so we always check that the prefetch bytes and counts are
zero. Is this intended?

Thanks,

Mark

Reply via email to