Write_schemata() uses fprintf() to write a bitmask into a schemata file inside resctrl FS. It checks fprintf() return value but it doesn't check fclose() return value. Error codes from fprintf() such as write errors, are flushed back to the user only after fclose() is executed which means any invalid bitmask can be written into the schemata file.
Rewrite write_schemata() to use syscalls instead of stdio functions to interact with the schemata file.
Change sprintf() to snprintf() in write_schemata().
In case of write() returning an error pass the string acquired with strerror() to the "reason" buffer.
Extend "reason" buffer by a factor of two so it can hold longer error messages.
The resctrlfs.c file defines functions that interact with the resctrl FS while resctrl_val.c file defines functions that perform measurements on the cache. Run_benchmark() fits logically into the second file before resctrl_val() function that uses it.
Move run_benchmark() from resctrlfs.c to resctrl_val.c just before resctrl_val() function definition.
Series is based on kselftest next branch.
Changelog v2: - Change sprintf() to snprintf() in write_schemata(). - Redo write_schemata() with syscalls instead of stdio functions. - Fix typos and missing dots in patch messages. - Branch printf attribute patch to a separate series.
[v1] https://lore.kernel.org/all/cover.1692880423.git.maciej.wieczor-retman@intel...
Wieczor-Retman, Maciej (2): selftests/resctrl: Fix schemata write error check selftests/resctrl: Move run_benchmark() to a more fitting file
tools/testing/selftests/resctrl/resctrl_val.c | 50 ++++++++++++ tools/testing/selftests/resctrl/resctrlfs.c | 76 ++++--------------- 2 files changed, 63 insertions(+), 63 deletions(-)
base-commit: 13eb52f6293dbda02890698d92f3d9913d8d5aeb
Writing bitmasks to the schemata can fail when the bitmask doesn't adhere to some constraints defined by what a particular CPU supports. Some example of constraints are max length or having contiguous bits. The driver should properly return errors when any rule concerning bitmask format is broken.
Resctrl FS returns error codes from fprintf() only when fclose() is called. Current error checking scheme allows invalid bitmasks to be written into schemata file and the selftest doesn't notice because the fclose() error code isn't checked.
Substitute fopen(), flose() and fprintf() with open(), close() and write() to avoid error code buffering between fprintf() and fclose().
Add newline to the end of the schema string so it satisfies rdt schemata writing requirements.
Remove newline character from the schemat string after writing it to the schemata file so it prints correctly before function return.
Pass the string generated with strerror() to the "reason" buffer so the error message is more verbose. Extend "reason" buffer so it can hold longer messages.
Changelog v2: - Rewrite patch message. - Double "reason" buffer size to fit longer error explanation. - Redo file interactions with syscalls instead of stdio functions.
Signed-off-by: Wieczor-Retman, Maciej maciej.wieczor-retman@intel.com --- tools/testing/selftests/resctrl/resctrlfs.c | 24 +++++++++++---------- 1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index bd36ee206602..0f9644e5a25e 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -488,9 +488,8 @@ int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp, */ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val) { - char controlgroup[1024], schema[1024], reason[64]; - int resource_id, ret = 0; - FILE *fp; + char controlgroup[1024], schema[1024], reason[128]; + int resource_id, fp, ret = 0;
if (strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) && strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) && @@ -518,27 +517,30 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val)
if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) || !strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) - sprintf(schema, "%s%d%c%s", "L3:", resource_id, '=', schemata); + snprintf(schema, sizeof(schema), "%s%d%c%s\n", "L3:", + resource_id, '=', schemata); if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) || !strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR))) - sprintf(schema, "%s%d%c%s", "MB:", resource_id, '=', schemata); + snprintf(schema, sizeof(schema), "%s%d%c%s\n", "MB:", + resource_id, '=', schemata);
- fp = fopen(controlgroup, "w"); + fp = open(controlgroup, O_WRONLY); if (!fp) { sprintf(reason, "Failed to open control group"); ret = -1;
goto out; } - - if (fprintf(fp, "%s\n", schema) < 0) { - sprintf(reason, "Failed to write schemata in control group"); - fclose(fp); + if (write(fp, schema, strlen(schema)) < 0) { + snprintf(reason, sizeof(reason), + "write() failed : %s", strerror(errno)); + close(fp); ret = -1;
goto out; } - fclose(fp); + close(fp); + schema[strcspn(schema, "\n")] = 0;
out: ksft_print_msg("Write schema "%s" to resctrl FS%s%s\n",
On Mon, 28 Aug 2023, Wieczor-Retman, Maciej wrote:
Writing bitmasks to the schemata can fail when the bitmask doesn't adhere to some constraints defined by what a particular CPU supports. Some example of constraints are max length or having contiguous bits. The driver should properly return errors when any rule concerning bitmask format is broken.
Resctrl FS returns error codes from fprintf() only when fclose() is called. Current error checking scheme allows invalid bitmasks to be written into schemata file and the selftest doesn't notice because the fclose() error code isn't checked.
Substitute fopen(), flose() and fprintf() with open(), close() and write() to avoid error code buffering between fprintf() and fclose().
Add newline to the end of the schema string so it satisfies rdt schemata writing requirements.
Remove newline character from the schemat string after writing it to the schemata file so it prints correctly before function return.
Pass the string generated with strerror() to the "reason" buffer so the error message is more verbose. Extend "reason" buffer so it can hold longer messages.
Changelog v2:
- Rewrite patch message.
- Double "reason" buffer size to fit longer error explanation.
- Redo file interactions with syscalls instead of stdio functions.
Signed-off-by: Wieczor-Retman, Maciej maciej.wieczor-retman@intel.com
tools/testing/selftests/resctrl/resctrlfs.c | 24 +++++++++++---------- 1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index bd36ee206602..0f9644e5a25e 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -488,9 +488,8 @@ int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp, */ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val) {
- char controlgroup[1024], schema[1024], reason[64];
- int resource_id, ret = 0;
- FILE *fp;
- char controlgroup[1024], schema[1024], reason[128];
- int resource_id, fp, ret = 0;
fp -> fd to follow the usual convention.
if (strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) && strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) && @@ -518,27 +517,30 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val) if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) || !strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
sprintf(schema, "%s%d%c%s", "L3:", resource_id, '=', schemata);
snprintf(schema, sizeof(schema), "%s%d%c%s\n", "L3:",resource_id, '=', schemata);
sprintf(schema, "%s%d%c%s", "MB:", resource_id, '=', schemata);
snprintf(schema, sizeof(schema), "%s%d%c%s\n", "MB:",resource_id, '=', schemata);
- fp = fopen(controlgroup, "w");
- fp = open(controlgroup, O_WRONLY); if (!fp) { sprintf(reason, "Failed to open control group"); ret = -1;
goto out; }
- if (fprintf(fp, "%s\n", schema) < 0) {
sprintf(reason, "Failed to write schemata in control group");fclose(fp);
- if (write(fp, schema, strlen(schema)) < 0) {
snprintf() returns you the length, just store the return value and there's no need to calculate it here.
snprintf(reason, sizeof(reason),"write() failed : %s", strerror(errno));close(fp);goto out; }
- fclose(fp);
- close(fp);
- schema[strcspn(schema, "\n")] = 0;
Here too you can use the known length/index instead of strcspr().
On 2023-08-28 at 13:43:05 +0300, Ilpo Järvinen wrote:
On Mon, 28 Aug 2023, Wieczor-Retman, Maciej wrote:
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index bd36ee206602..0f9644e5a25e 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -488,9 +488,8 @@ int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp, */ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val) {
- char controlgroup[1024], schema[1024], reason[64];
- int resource_id, ret = 0;
- FILE *fp;
- char controlgroup[1024], schema[1024], reason[128];
- int resource_id, fp, ret = 0;
fp -> fd to follow the usual convention.
Okay, I'll change it
- fp = fopen(controlgroup, "w");
- fp = open(controlgroup, O_WRONLY); if (!fp) { sprintf(reason, "Failed to open control group"); ret = -1;
goto out; }
- if (fprintf(fp, "%s\n", schema) < 0) {
sprintf(reason, "Failed to write schemata in control group");fclose(fp);
- if (write(fp, schema, strlen(schema)) < 0) {
snprintf() returns you the length, just store the return value and there's no need to calculate it here.
Thanks for the suggestion, tested it and it works fine, I'll add it in the next version.
snprintf(reason, sizeof(reason),"write() failed : %s", strerror(errno));close(fp);goto out; }
- fclose(fp);
- close(fp);
- schema[strcspn(schema, "\n")] = 0;
Here too you can use the known length/index instead of strcspr().
Same as above
Hi Maciej,
On 8/28/2023 2:56 AM, Wieczor-Retman, Maciej wrote:
Writing bitmasks to the schemata can fail when the bitmask doesn't adhere to some constraints defined by what a particular CPU supports. Some example of constraints are max length or having contiguous bits. The driver should properly return errors when any rule concerning bitmask format is broken.
Resctrl FS returns error codes from fprintf() only when fclose() is called. Current error checking scheme allows invalid bitmasks to be written into schemata file and the selftest doesn't notice because the fclose() error code isn't checked.
Substitute fopen(), flose() and fprintf() with open(), close() and write() to avoid error code buffering between fprintf() and fclose().
Add newline to the end of the schema string so it satisfies rdt schemata writing requirements.
I am not sure how to interpret the above because existing code already adds a newline to the end of the schema when the buffer is written to the schemata file. Also please use "resctrl schemata" since RDT is Intel specific and does not use schemata terminology.
Remove newline character from the schemat string after writing it to the schemata file so it prints correctly before function return.
schemat -> "schema" or "schemata"?
Pass the string generated with strerror() to the "reason" buffer so the error message is more verbose. Extend "reason" buffer so it can hold longer messages.
Changelog v2:
- Rewrite patch message.
- Double "reason" buffer size to fit longer error explanation.
- Redo file interactions with syscalls instead of stdio functions.
Please place the above "Changelog v2" snippet below the "---" lines below. This is text that should not end up in the kernel log.
Signed-off-by: Wieczor-Retman, Maciej maciej.wieczor-retman@intel.com
(list of changes should go here)
Reinette
Reinette
Hi, thanks for the comments!
On 2023-08-30 at 13:49:18 -0700, Reinette Chatre wrote:
Hi Maciej,
On 8/28/2023 2:56 AM, Wieczor-Retman, Maciej wrote:
Writing bitmasks to the schemata can fail when the bitmask doesn't adhere to some constraints defined by what a particular CPU supports. Some example of constraints are max length or having contiguous bits. The driver should properly return errors when any rule concerning bitmask format is broken.
Resctrl FS returns error codes from fprintf() only when fclose() is called. Current error checking scheme allows invalid bitmasks to be written into schemata file and the selftest doesn't notice because the fclose() error code isn't checked.
Substitute fopen(), flose() and fprintf() with open(), close() and write() to avoid error code buffering between fprintf() and fclose().
Add newline to the end of the schema string so it satisfies rdt schemata writing requirements.
I am not sure how to interpret the above because existing code already adds a newline to the end of the schema when the buffer is written to
Okay, true. I meant I moved it a few lines back but there is no real change there. I'll just remove this paragraph.
the schemata file. Also please use "resctrl schemata" since RDT is Intel specific and does not use schemata terminology.
Thank you, I'll change it.
Remove newline character from the schemat string after writing it to the schemata file so it prints correctly before function return.
schemat -> "schema" or "schemata"?
I'll stick with schema as that's the variable name, thanks for finding this typo.
Pass the string generated with strerror() to the "reason" buffer so the error message is more verbose. Extend "reason" buffer so it can hold longer messages.
Changelog v2:
- Rewrite patch message.
- Double "reason" buffer size to fit longer error explanation.
- Redo file interactions with syscalls instead of stdio functions.
Please place the above "Changelog v2" snippet below the "---" lines below. This is text that should not end up in the kernel log.
Yes, I realized I made this mistake a few hours after sending the patch. I'll correct it and make sure to double check it before sending next time.
Signed-off-by: Wieczor-Retman, Maciej maciej.wieczor-retman@intel.com
(list of changes should go here)
Reinette
Resctrlfs.c file contains mostly functions that interact in some way with resctrl FS entries while functions inside resctrl_val.c deal with measurements and benchmarking.
Run_benchmark() function is located in resctrlfs.c file even though it's purpose is not interacting with the resctrl FS but to execute cache checking logic.
Move run_benchmark() to resctrl_val.c just before resctrl_val() function that makes use of run_benchmark().
Remove return comment from kernel-doc since the function is type void.
Changelog v2: - Add dots at the end of patch msg sentences. - Remove "Return: void" from run_benchmark() kernel-doc comment.
Signed-off-by: Wieczor-Retman, Maciej maciej.wieczor-retman@intel.com --- tools/testing/selftests/resctrl/resctrl_val.c | 50 ++++++++++++++++++ tools/testing/selftests/resctrl/resctrlfs.c | 52 ------------------- 2 files changed, 50 insertions(+), 52 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index f0f6c5f6e98b..5c8dc0a7bab9 100644 --- a/tools/testing/selftests/resctrl/resctrl_val.c +++ b/tools/testing/selftests/resctrl/resctrl_val.c @@ -621,6 +621,56 @@ measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start) return 0; }
+/* + * run_benchmark - Run a specified benchmark or fill_buf (default benchmark) + * in specified signal. Direct benchmark stdio to /dev/null. + * @signum: signal number + * @info: signal info + * @ucontext: user context in signal handling + */ +void run_benchmark(int signum, siginfo_t *info, void *ucontext) +{ + int operation, ret, memflush; + char **benchmark_cmd; + size_t span; + bool once; + FILE *fp; + + benchmark_cmd = info->si_ptr; + + /* + * Direct stdio of child to /dev/null, so that only parent writes to + * stdio (console) + */ + fp = freopen("/dev/null", "w", stdout); + if (!fp) + PARENT_EXIT("Unable to direct benchmark status to /dev/null"); + + if (strcmp(benchmark_cmd[0], "fill_buf") == 0) { + /* Execute default fill_buf benchmark */ + span = strtoul(benchmark_cmd[1], NULL, 10); + memflush = atoi(benchmark_cmd[2]); + operation = atoi(benchmark_cmd[3]); + if (!strcmp(benchmark_cmd[4], "true")) + once = true; + else if (!strcmp(benchmark_cmd[4], "false")) + once = false; + else + PARENT_EXIT("Invalid once parameter"); + + if (run_fill_buf(span, memflush, operation, once)) + fprintf(stderr, "Error in running fill buffer\n"); + } else { + /* Execute specified benchmark */ + ret = execvp(benchmark_cmd[0], benchmark_cmd); + if (ret) + perror("wrong\n"); + } + + fclose(stdout); + PARENT_EXIT("Unable to run specified benchmark"); +} + /* * resctrl_val: execute benchmark and measure memory bandwidth on * the benchmark diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index 0f9644e5a25e..72dd8c3f655a 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -291,58 +291,6 @@ int taskset_benchmark(pid_t bm_pid, int cpu_no) return 0; }
-/* - * run_benchmark - Run a specified benchmark or fill_buf (default benchmark) - * in specified signal. Direct benchmark stdio to /dev/null. - * @signum: signal number - * @info: signal info - * @ucontext: user context in signal handling - * - * Return: void - */ -void run_benchmark(int signum, siginfo_t *info, void *ucontext) -{ - int operation, ret, memflush; - char **benchmark_cmd; - size_t span; - bool once; - FILE *fp; - - benchmark_cmd = info->si_ptr; - - /* - * Direct stdio of child to /dev/null, so that only parent writes to - * stdio (console) - */ - fp = freopen("/dev/null", "w", stdout); - if (!fp) - PARENT_EXIT("Unable to direct benchmark status to /dev/null"); - - if (strcmp(benchmark_cmd[0], "fill_buf") == 0) { - /* Execute default fill_buf benchmark */ - span = strtoul(benchmark_cmd[1], NULL, 10); - memflush = atoi(benchmark_cmd[2]); - operation = atoi(benchmark_cmd[3]); - if (!strcmp(benchmark_cmd[4], "true")) - once = true; - else if (!strcmp(benchmark_cmd[4], "false")) - once = false; - else - PARENT_EXIT("Invalid once parameter"); - - if (run_fill_buf(span, memflush, operation, once)) - fprintf(stderr, "Error in running fill buffer\n"); - } else { - /* Execute specified benchmark */ - ret = execvp(benchmark_cmd[0], benchmark_cmd); - if (ret) - perror("wrong\n"); - } - - fclose(stdout); - PARENT_EXIT("Unable to run specified benchmark"); -} - /* * create_grp - Create a group only if one doesn't exist * @grp_name: Name of the group
On Mon, 28 Aug 2023, Wieczor-Retman, Maciej wrote:
Resctrlfs.c file contains mostly functions that interact in some way with resctrl FS entries while functions inside resctrl_val.c deal with measurements and benchmarking.
Run_benchmark() function is located in resctrlfs.c file even though it's purpose is not interacting with the resctrl FS but to execute cache checking logic.
Move run_benchmark() to resctrl_val.c just before resctrl_val() function that makes use of run_benchmark().
Remove return comment from kernel-doc since the function is type void.
Changelog v2:
- Add dots at the end of patch msg sentences.
- Remove "Return: void" from run_benchmark() kernel-doc comment.
Signed-off-by: Wieczor-Retman, Maciej maciej.wieczor-retman@intel.com
Reviewed-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com
Hi Maciej,
On 8/28/2023 2:56 AM, Wieczor-Retman, Maciej wrote:
Resctrlfs.c file contains mostly functions that interact in some way with resctrl FS entries while functions inside resctrl_val.c deal with measurements and benchmarking.
Run_benchmark() function is located in resctrlfs.c file even though it's purpose is not interacting with the resctrl FS but to execute cache checking logic.
It looks like your editor may be automatically capitalize first words of sentences like Resctrlfs.c and Run_benchmark() above. Also please note that when using () to indicate a function it is not necessary to say it is a function. For example above can just be: "run_benchmark() is located ..." ... similarly you can just say "resctrlfs.c contains ...".
Move run_benchmark() to resctrl_val.c just before resctrl_val() function that makes use of run_benchmark().
Remove return comment from kernel-doc since the function is type void.
Changelog v2:
- Add dots at the end of patch msg sentences.
- Remove "Return: void" from run_benchmark() kernel-doc comment.
same comment about changelog.
Signed-off-by: Wieczor-Retman, Maciej maciej.wieczor-retman@intel.com
tools/testing/selftests/resctrl/resctrl_val.c | 50 ++++++++++++++++++ tools/testing/selftests/resctrl/resctrlfs.c | 52 ------------------- 2 files changed, 50 insertions(+), 52 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index f0f6c5f6e98b..5c8dc0a7bab9 100644 --- a/tools/testing/selftests/resctrl/resctrl_val.c +++ b/tools/testing/selftests/resctrl/resctrl_val.c @@ -621,6 +621,56 @@ measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start) return 0; } +/*
- run_benchmark - Run a specified benchmark or fill_buf (default benchmark)
in specified signal. Direct benchmark stdio to /dev/null.
- @signum: signal number
- @info: signal info
- @ucontext: user context in signal handling
- */
+void run_benchmark(int signum, siginfo_t *info, void *ucontext)
Can run_benchmark() now be made static and its declaration removed from the header file?
Reinette
Hi,
On 2023-08-30 at 13:51:29 -0700, Reinette Chatre wrote:
Hi Maciej,
On 8/28/2023 2:56 AM, Wieczor-Retman, Maciej wrote:
Resctrlfs.c file contains mostly functions that interact in some way with resctrl FS entries while functions inside resctrl_val.c deal with measurements and benchmarking.
Run_benchmark() function is located in resctrlfs.c file even though it's purpose is not interacting with the resctrl FS but to execute cache checking logic.
It looks like your editor may be automatically capitalize first words of sentences like Resctrlfs.c and Run_benchmark() above.
I'll fix it.
Also please note that when using () to indicate a function it is not necessary to say it is a function. For example above can just be: "run_benchmark() is located ..." ... similarly you can just say "resctrlfs.c contains ...".
Thanks for the tip, will apply it from now on.
Move run_benchmark() to resctrl_val.c just before resctrl_val() function that makes use of run_benchmark().
Remove return comment from kernel-doc since the function is type void.
Changelog v2:
- Add dots at the end of patch msg sentences.
- Remove "Return: void" from run_benchmark() kernel-doc comment.
same comment about changelog.
It'll be fixed next time.
Signed-off-by: Wieczor-Retman, Maciej maciej.wieczor-retman@intel.com
tools/testing/selftests/resctrl/resctrl_val.c | 50 ++++++++++++++++++ tools/testing/selftests/resctrl/resctrlfs.c | 52 ------------------- 2 files changed, 50 insertions(+), 52 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index f0f6c5f6e98b..5c8dc0a7bab9 100644 --- a/tools/testing/selftests/resctrl/resctrl_val.c +++ b/tools/testing/selftests/resctrl/resctrl_val.c @@ -621,6 +621,56 @@ measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start) return 0; } +/*
- run_benchmark - Run a specified benchmark or fill_buf (default benchmark)
in specified signal. Direct benchmark stdio to /dev/null.
- @signum: signal number
- @info: signal info
- @ucontext: user context in signal handling
- */
+void run_benchmark(int signum, siginfo_t *info, void *ucontext)
Can run_benchmark() now be made static and its declaration removed from the header file?
Thanks for noticing. Yes, from my side it seems turning it into static is okay. I tried it out on Ilpo's branches that I know he's currently working on and there were no errors either.
linux-kselftest-mirror@lists.linaro.org
-
Ilpo Järvinen -
Maciej Wieczór-Retman -
Reinette Chatre -
Wieczor-Retman, Maciej