Re: [lttng-dev] [PATCH urcu] fix: handle EINTR correctly in get_cpu_mask_from_sysfs
On 2024-05-02 10:32, Michael Jeanson wrote: On 2024-05-02 09:54, Mathieu Desnoyers wrote: On 2024-05-01 19:42, Benjamin Marzinski via lttng-dev wrote: If the read() in get_cpu_mask_from_sysfs() fails with EINTR, the code is supposed to retry, but the while loop condition has (bytes_read > 0), which is false when read() fails with EINTR. The result is that the code exits the loop, having only read part of the string. Use (bytes_read != 0) in the while loop condition instead, since the (bytes_read < 0) case is already handled in the loop. Thanks for the fix ! It is indeed the right thing to do. I would like to integrate this fix into the librseq and libside projects as well though, but I notice the the copy in liburcu is LGPLv2.1 whereas the copy in librseq and libside are MIT. Michael, should we first relicense the liburcu src/compat-smp.h implementation to MIT so it matches the license of the copies in librseq and libside ? Sure, please go ahead. For the records, we also have a copy of this code in lttng-ust, also under MIT license. So liburcu's copy is the only outlier there. Thanks, Mathieu Thanks, Mathieu Signed-off-by: Benjamin Marzinski --- src/compat-smp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compat-smp.h b/src/compat-smp.h index 31fa979..075a332 100644 --- a/src/compat-smp.h +++ b/src/compat-smp.h @@ -164,7 +164,7 @@ static inline int get_cpu_mask_from_sysfs(char *buf, size_t max_bytes, const cha total_bytes_read += bytes_read; assert(total_bytes_read <= max_bytes); - } while (max_bytes > total_bytes_read && bytes_read > 0); + } while (max_bytes > total_bytes_read && bytes_read != 0); /* * Make sure the mask read is a null terminated string. -- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com ___ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
Re: [lttng-dev] [PATCH urcu] fix: handle EINTR correctly in get_cpu_mask_from_sysfs
On 2024-05-02 09:54, Mathieu Desnoyers wrote: On 2024-05-01 19:42, Benjamin Marzinski via lttng-dev wrote: If the read() in get_cpu_mask_from_sysfs() fails with EINTR, the code is supposed to retry, but the while loop condition has (bytes_read > 0), which is false when read() fails with EINTR. The result is that the code exits the loop, having only read part of the string. Use (bytes_read != 0) in the while loop condition instead, since the (bytes_read < 0) case is already handled in the loop. Thanks for the fix ! It is indeed the right thing to do. I would like to integrate this fix into the librseq and libside projects as well though, but I notice the the copy in liburcu is LGPLv2.1 whereas the copy in librseq and libside are MIT. Michael, should we first relicense the liburcu src/compat-smp.h implementation to MIT so it matches the license of the copies in librseq and libside ? Sure, please go ahead. Thanks, Mathieu Signed-off-by: Benjamin Marzinski --- src/compat-smp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compat-smp.h b/src/compat-smp.h index 31fa979..075a332 100644 --- a/src/compat-smp.h +++ b/src/compat-smp.h @@ -164,7 +164,7 @@ static inline int get_cpu_mask_from_sysfs(char *buf, size_t max_bytes, const cha total_bytes_read += bytes_read; assert(total_bytes_read <= max_bytes); - } while (max_bytes > total_bytes_read && bytes_read > 0); + } while (max_bytes > total_bytes_read && bytes_read != 0); /* * Make sure the mask read is a null terminated string. ___ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
Re: [lttng-dev] [PATCH urcu] fix: handle EINTR correctly in get_cpu_mask_from_sysfs
On 2024-05-01 19:42, Benjamin Marzinski via lttng-dev wrote: If the read() in get_cpu_mask_from_sysfs() fails with EINTR, the code is supposed to retry, but the while loop condition has (bytes_read > 0), which is false when read() fails with EINTR. The result is that the code exits the loop, having only read part of the string. Use (bytes_read != 0) in the while loop condition instead, since the (bytes_read < 0) case is already handled in the loop. Thanks for the fix ! It is indeed the right thing to do. I would like to integrate this fix into the librseq and libside projects as well though, but I notice the the copy in liburcu is LGPLv2.1 whereas the copy in librseq and libside are MIT. Michael, should we first relicense the liburcu src/compat-smp.h implementation to MIT so it matches the license of the copies in librseq and libside ? Thanks, Mathieu Signed-off-by: Benjamin Marzinski --- src/compat-smp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compat-smp.h b/src/compat-smp.h index 31fa979..075a332 100644 --- a/src/compat-smp.h +++ b/src/compat-smp.h @@ -164,7 +164,7 @@ static inline int get_cpu_mask_from_sysfs(char *buf, size_t max_bytes, const cha total_bytes_read += bytes_read; assert(total_bytes_read <= max_bytes); - } while (max_bytes > total_bytes_read && bytes_read > 0); + } while (max_bytes > total_bytes_read && bytes_read != 0); /* * Make sure the mask read is a null terminated string. -- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com ___ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
[lttng-dev] [PATCH urcu] fix: handle EINTR correctly in get_cpu_mask_from_sysfs
If the read() in get_cpu_mask_from_sysfs() fails with EINTR, the code is supposed to retry, but the while loop condition has (bytes_read > 0), which is false when read() fails with EINTR. The result is that the code exits the loop, having only read part of the string. Use (bytes_read != 0) in the while loop condition instead, since the (bytes_read < 0) case is already handled in the loop. Signed-off-by: Benjamin Marzinski --- src/compat-smp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compat-smp.h b/src/compat-smp.h index 31fa979..075a332 100644 --- a/src/compat-smp.h +++ b/src/compat-smp.h @@ -164,7 +164,7 @@ static inline int get_cpu_mask_from_sysfs(char *buf, size_t max_bytes, const cha total_bytes_read += bytes_read; assert(total_bytes_read <= max_bytes); - } while (max_bytes > total_bytes_read && bytes_read > 0); + } while (max_bytes > total_bytes_read && bytes_read != 0); /* * Make sure the mask read is a null terminated string. -- 2.44.0 ___ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
