| Message ID | 20241217131440.1980151-3-nilay@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | throtl: fix IO block-size and race while submitting IO | expand |
Hi, 在 2024/12/17 21:14, Nilay Shroff 写道: > This commit helps fix the above race condition by touching a temp file. The > the existence of the temp file is then polled by the background process at > regular interval. Until the temp file is created, the background process > would not forward progress and starts submitting IO and from the main > thread we'd touch temp file only after we write PID of the background > process into cgroup.procs. It's right sleep 0.1 is not appropriate here, and this can work. However, I think reading cgroup.procs directly is better, something like following: _throtl_test_io() { - local pid + local pid="none" { local rw=$1 local bs=$2 local count=$3 - sleep 0.1 + while ! cat $CGROUP2_DIR/$THROTL_DIR/cgroup.procs | grep $pid; do + sleep 0.1 _throtl_issue_io "$rw" "$bs" "$count" } & Thanks, Kuai
On 12/17/24 6:24 PM, Yu Kuai wrote: > - sleep 0.1 > + while ! cat $CGROUP2_DIR/$THROTL_DIR/cgroup.procs | grep > $pid; do > + sleep 0.1 > _throtl_issue_io "$rw" "$bs" "$count" > } & If this approach is used, please add the options -q (quiet) and -w (match whole words only) to the grep command. Thanks, Bart.
diff --git a/tests/throtl/004 b/tests/throtl/004 index 6e28612..44b33ec 100755 --- a/tests/throtl/004 +++ b/tests/throtl/004 @@ -21,12 +21,18 @@ test() { _throtl_set_limits wbps=$((1024 * 1024)) { - sleep 0.1 + while true; do + if [[ -e "$TMPDIR/test-io" ]]; then + break + fi + sleep 0.1 + done _throtl_issue_io write 10M 1 } & local pid=$! echo "$pid" > "$CGROUP2_DIR/$THROTL_DIR/cgroup.procs" + touch "$TMPDIR/test-io" sleep 0.6 echo 0 > "/sys/kernel/config/nullb/$THROTL_DEV/power" diff --git a/tests/throtl/005 b/tests/throtl/005 index 0778258..4797ea3 100755 --- a/tests/throtl/005 +++ b/tests/throtl/005 @@ -20,12 +20,19 @@ test() { _throtl_set_limits wbps=$((512 * 1024)) { - sleep 0.1 + while true; do + if [[ -e "$TMPDIR/test-io" ]]; then + break + fi + sleep 0.1 + done + _throtl_issue_io write 1M 1 } & local pid=$! echo "$pid" > "$CGROUP2_DIR/$THROTL_DIR/cgroup.procs" + touch "$TMPDIR/test-io" sleep 1 _throtl_set_limits wbps=$((256 * 1024)) diff --git a/tests/throtl/rc b/tests/throtl/rc index 330e6b9..a9c1889 100644 --- a/tests/throtl/rc +++ b/tests/throtl/rc @@ -104,11 +104,18 @@ _throtl_test_io() { local bs=$2 local count=$3 - sleep 0.1 + while true; do + if [[ -e "$TMPDIR/test-io" ]]; then + break + fi + sleep 0.1 + done _throtl_issue_io "$rw" "$bs" "$count" } & pid=$! echo "$pid" > "$CGROUP2_DIR/$THROTL_DIR/cgroup.procs" + touch "$TMPDIR/test-io" wait $pid + rm "$TMPDIR/test-io" }
The throttle test cases uses _throtl_issue_io function to submit IO to the device. This function typically runs in the background process however before this function starts execution and submit IO, we need to set the PID of the background process into cgroup.procs. The current implementation adds sleep 0.1 before _throtl_issue_io and it's assumed that during this sleep time of 0.1 second we shall be able to write the PID of the background process to cgroup.procs. However this may not be always true as background process might starts running after sleep of 0.1 seconds (and hence start submitting IO) before we could actually write the PID of background process into cgroup.procs. This commit helps fix the above race condition by touching a temp file. The the existence of the temp file is then polled by the background process at regular interval. Until the temp file is created, the background process would not forward progress and starts submitting IO and from the main thread we'd touch temp file only after we write PID of the background process into cgroup.procs. Signed-off-by: Nilay Shroff <nilay@linux.ibm.com> --- tests/throtl/004 | 8 +++++++- tests/throtl/005 | 9 ++++++++- tests/throtl/rc | 9 ++++++++- 3 files changed, 23 insertions(+), 3 deletions(-)