On Dec 18, 2024 / 15:20, Nilay Shroff wrote:
>
>
> On 12/18/24 07:54, Yu Kuai wrote:
> > 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"
> > } &
>
> Thank you for your review and suggestion!
>
> However, IMO this may not work always because the issue here's that the @pid is local
> variable and when the shell starts the background job/process, typically, all local
> variables are copied to the new job. Henceforth, any update made to @pid in the parent
> shell would not be visible to the background job.
> I think, for IPC between parent shell and background job, we may use temp file,
> named pipe or signals. As file is the easiest and simplest mechanism, for this
> simple test I preferred using file.
How about to refer to $BASHPID in the background job sub-shell? It shows the PID
of the background job, so we don't need IPC to pass the PID.
I think the hunk like below can do the trick, hopefully. If it works, it would
be cleaner to factor out the while loop to a helper function, like
_throtl_wait_for_cgroup_procs() or something.
diff --git a/tests/throtl/004 b/tests/throtl/004
index 6e28612..0a16764 100755
--- a/tests/throtl/004
+++ b/tests/throtl/004
@@ -22,6 +22,10 @@ test() {
{
sleep 0.1
+ while ! grep --quiet --word-regexp "$BASHPID" \
+ "$CGROUP2_DIR/$THROTL_DIR/cgroup.procs"; do
+ sleep 0.1
+ done
_throtl_issue_io write 10M 1
} &
