[lttng-dev] [PATCH lttng-tools 3/3 master, 2.10-rc] Introduce "--blocking-timeout" channel parameter
Jonathan Rajotte-Julien
jonathan.rajotte-julien at efficios.com
Fri May 26 18:18:14 UTC 2017
Hi,
Comments bellow, only kept relevant part of the patch.
On Fri, May 26, 2017 at 06:14:20PM +0200, Mathieu Desnoyers wrote:
> diff --git a/src/bin/lttng-sessiond/channel.c b/src/bin/lttng-sessiond/channel.c
> index 8a88ecc..5aa0cd8 100644
> --- a/src/bin/lttng-sessiond/channel.c
> +++ b/src/bin/lttng-sessiond/channel.c
> @@ -72,6 +72,7 @@ struct lttng_channel *channel_new_default_attr(int dom,
> chan->attr.switch_timer_interval = DEFAULT_KERNEL_CHANNEL_SWITCH_TIMER;
> chan->attr.read_timer_interval = DEFAULT_KERNEL_CHANNEL_READ_TIMER;
> chan->attr.live_timer_interval = DEFAULT_KERNEL_CHANNEL_LIVE_TIMER;
> + extended_attr->blocking_timeout = DEFAULT_KERNEL_CHANNEL_BLOCKING_TIMEOUT;
> extended_attr->monitor_timer_interval =
> DEFAULT_KERNEL_CHANNEL_MONITOR_TIMER;
> break;
> @@ -97,6 +98,7 @@ common_ust:
> DEFAULT_UST_UID_CHANNEL_READ_TIMER;
> chan->attr.live_timer_interval =
> DEFAULT_UST_UID_CHANNEL_LIVE_TIMER;
> + extended_attr->blocking_timeout = DEFAULT_UST_UID_CHANNEL_BLOCKING_TIMEOUT;
Put on two lines.
> extended_attr->monitor_timer_interval =
> DEFAULT_UST_UID_CHANNEL_MONITOR_TIMER;
> break;
> @@ -111,6 +113,7 @@ common_ust:
> DEFAULT_UST_PID_CHANNEL_READ_TIMER;
> chan->attr.live_timer_interval =
> DEFAULT_UST_PID_CHANNEL_LIVE_TIMER;
Same here.
> + extended_attr->blocking_timeout = DEFAULT_UST_PID_CHANNEL_BLOCKING_TIMEOUT;
> extended_attr->monitor_timer_interval =
> DEFAULT_UST_PID_CHANNEL_MONITOR_TIMER;
> break;
> @@ -217,6 +220,15 @@ static int channel_validate(struct lttng_channel *attr)
> return 0;
> }
>
> diff --git a/src/bin/lttng-sessiond/lttng-ust-abi.h b/src/bin/lttng-sessiond/lttng-ust-abi.h
> index 972de0c..687eb0b 100644
> --- a/src/bin/lttng-sessiond/lttng-ust-abi.h
> +++ b/src/bin/lttng-sessiond/lttng-ust-abi.h
> @@ -181,7 +181,12 @@ struct lttng_ust_channel_attr {
> unsigned int switch_timer_interval; /* usec */
> unsigned int read_timer_interval; /* usec */
> enum lttng_ust_output output; /* splice, mmap */
> - char padding[LTTNG_UST_CHANNEL_ATTR_PADDING];
> + union {
> + struct {
> + int64_t blocking_timeout; /* Retry timeout (usec) */
> + } s;
Should "s" be a more meaningful identifier here?
> + char padding[LTTNG_UST_CHANNEL_ATTR_PADDING];
> + } u;
> } LTTNG_PACKED;
>
> #define LTTNG_UST_TRACEPOINT_ITER_PADDING 16
> diff --git a/src/bin/lttng-sessiond/lttng-ust-ctl.h b/src/bin/lttng-sessiond/lttng-ust-ctl.h
> index cba0e27..1d67e85 100644
> --- a/src/bin/lttng-sessiond/lttng-ust-ctl.h
> +++ b/src/bin/lttng-sessiond/lttng-ust-ctl.h
> @@ -53,6 +53,7 @@ struct ustctl_consumer_channel_attr {
> enum lttng_ust_output output; /* splice, mmap */
> uint32_t chan_id; /* channel ID */
> unsigned char uuid[LTTNG_UST_UUID_LEN]; /* Trace session unique ID */
> + int64_t blocking_timeout; /* Retry timeout (usec) */
Only one/two tab here for style consistency or fix the previous lines.
> } LTTNG_PACKED;
>
> /*
> diff --git a/src/common/config/session.xsd b/src/common/config/session.xsd
> index 550fea0..35e56ca 100644
> --- a/src/common/config/session.xsd
> +++ b/src/common/config/session.xsd
> @@ -43,6 +43,14 @@ elementFormDefault="qualified" version="2.8">
> </xs:restriction>
> </xs:simpleType>
>
> +<!-- Maps to the range allowed for blocking timeout -->
> +<xs:simpleType name="blocking_timeout_type">
> + <xs:restriction base="xs:integer">
> + <xs:minInclusive value="-1" />
Please add a comment here regarding the restriction on 4294967296000. Which is
due to the (int32) milliseconds overflow check.
> + <xs:maxInclusive value="4294967296000" />
> + </xs:restriction>
> +</xs:simpleType>
> +
> <xs:simpleType name="channel_overwrite_mode_type">
> <xs:restriction base="xs:string">
> <xs:enumeration value="DISCARD"/>
> @@ -186,6 +194,7 @@ elementFormDefault="qualified" version="2.8">
> <xs:element name="subbuffer_count" type="uint64_type" default="4" minOccurs="0"/>
> <xs:element name="switch_timer_interval" type="uint32_type" default="0" minOccurs="0"/> <!-- usec -->
> <xs:element name="read_timer_interval" type="uint32_type"/> <!-- usec -->
> + <xs:element name="blocking_timeout" type="blocking_timeout_type" default="0" minOccurs="0" /> <!-- usec -->
> <xs:element name="output_type" type="event_output_type"/>
> <xs:element name="tracefile_size" type="uint64_type" default="0" minOccurs="0"/> <!-- bytes -->
> <xs:element name="tracefile_count" type="uint64_type" default="0" minOccurs="0"/>
> diff --git a/src/common/defaults.h b/src/common/defaults.h
> index e0d0d86..b810302 100644
> --- a/src/common/defaults.h
> +++ b/src/common/defaults.h
> @@ -221,6 +221,7 @@
> #define DEFAULT_KERNEL_CHANNEL_MONITOR_TIMER CONFIG_DEFAULT_KERNEL_CHANNEL_MONITOR_TIMER
> #define DEFAULT_KERNEL_CHANNEL_READ_TIMER CONFIG_DEFAULT_KERNEL_CHANNEL_READ_TIMER
> #define DEFAULT_KERNEL_CHANNEL_LIVE_TIMER CONFIG_DEFAULT_KERNEL_CHANNEL_LIVE_TIMER
> +#define DEFAULT_KERNEL_CHANNEL_BLOCKING_TIMEOUT CONFIG_DEFAULT_KERNEL_CHANNEL_BLOCKING_TIMEOUT
>
> /* User space defaults */
>
> @@ -244,6 +245,9 @@
> #define DEFAULT_UST_PID_CHANNEL_READ_TIMER CONFIG_DEFAULT_UST_PID_CHANNEL_READ_TIMER
> #define DEFAULT_UST_UID_CHANNEL_READ_TIMER CONFIG_DEFAULT_UST_UID_CHANNEL_READ_TIMER
>
> +#define DEFAULT_UST_PID_CHANNEL_BLOCKING_TIMEOUT CONFIG_DEFAULT_UST_PID_CHANNEL_BLOCKING_TIMEOUT
> +#define DEFAULT_UST_UID_CHANNEL_BLOCKING_TIMEOUT CONFIG_DEFAULT_UST_UID_CHANNEL_BLOCKING_TIMEOUT
> +
> /*
> * Default timeout value for the sem_timedwait() call. Blocking forever is not
> * wanted so a timeout is used to control the data flow and not freeze the
> diff --git a/src/common/mi-lttng-3.0.xsd b/src/common/mi-lttng-3.0.xsd
> index 56b65a3..c5614f8 100644
> --- a/src/common/mi-lttng-3.0.xsd
> +++ b/src/common/mi-lttng-3.0.xsd
> @@ -43,6 +43,14 @@ THE SOFTWARE.
> </xs:restriction>
> </xs:simpleType>
>
> + <!-- Maps to the range allowed for blocking timeout -->
> + <xs:simpleType name="blocking_timeout_type">
> + <xs:restriction base="xs:integer">
> + <xs:minInclusive value="-1" />
Same comment regarding the max value restriction found in session.xsd .
> + <xs:maxInclusive value="4294967296000" />
> + </xs:restriction>
> + </xs:simpleType>
> +
> <!-- Maps to the char name[LTTNG_SYMBOL_NAME_LEN] -->
> <xs:simpleType name="name_type">
> <xs:restriction base="xs:string">
> @@ -363,6 +371,7 @@ THE SOFTWARE.
> <xs:element name="discarded_events" type="tns:uint64_type" default="0" minOccurs="0" />
> <xs:element name="lost_packets" type="tns:uint64_type" default="0" minOccurs="0" />
> <xs:element name="monitor_timer_interval" type="tns:uint64_type" default="0" minOccurs="0" />
> + <xs:element name="blocking_timeout" type="tns:blocking_timeout_type" default="0" minOccurs="0" />
> </xs:all>
> </xs:complexType>
>
--
Jonathan Rajotte-Julien
EfficiOS
More information about the lttng-dev
mailing list