From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Snyder To: Kevin Buettner Cc: gdb-patches@sources.redhat.com Subject: Re: [PATCH RFA #2] Retry open()s in procfs.c Date: Mon, 16 Apr 2001 15:20:00 -0000 Message-id: <3ADB702B.B7BDB8A9@cygnus.com> References: <1010414015521.ZM21085@ocotillo.lan> X-SW-Source: 2001-04/msg00161.html Kevin Buettner wrote: > > I request approval for committing the patch below. Yes, this looks great. Please put it in. > > This patch supercedes the patch I submitted in: > > http://sources.redhat.com/ml/gdb-patches/2001-04/msg00133.html > > In addition to handling the original problem that I was seeing on AIX5 > (which involved a race between GDB and the kernel in opening/creating > /proc/PID/ctl), this patch handles other conditions in which it may be > desirable to retry an open() in procfs.c. In particular, I hope that > it will also fix the problem reported by Damien Kick in: > > http://sources.redhat.com/ml/gdb/2001-04/msg00078.html > > (Damien, I would appreciate it if you'd give this patch a try and > let me know if it fixes the problem that you reported.) > > I've tested these changes on AIX5 and it does gracefully handle the > race condition noted above. (I added a temporary printf() just before > the sleep() to verify that the ``else'' clause does indeed get hit and > operates as expected.) I've also run the GDB testsuite on both > i586-sco-sysv5uw7.1.1 and i386-pc-solaris2.8 and see no regressions. > > Okay to commit? > > * procfs.c (open_with_retry): New function. > (open_procinfo_files, load_syscalls, proc_iterate_over_mappings) > (proc_get_LDT_entry): Call open_with_retry() instead of open(). > > Index: procfs.c > =================================================================== > RCS file: /cvs/src/src/gdb/procfs.c,v > retrieving revision 1.26 > diff -u -p -r1.26 procfs.c > --- procfs.c 2001/03/27 02:01:11 1.26 > +++ procfs.c 2001/04/14 01:37:56 > @@ -458,6 +458,40 @@ find_procinfo_or_die (int pid, int tid) > return pi; > } > > +/* open_with_retry() is a wrapper for open(). The appropriate > + open() call is attempted; if unsuccessful, it will be retried as > + many times as needed for the EAGAIN and EINTR conditions. > + > + For other conditions, open_with_retry() will retry the open() a > + limited number of times. In addition, a short sleep is imposed > + prior to retrying the open(). The reason for this sleep is to give > + the kernel a chance to catch up and create the file in question in > + the event that GDB "wins" the race to open a file before the kernel > + has created it. */ > + > +static int > +open_with_retry (const char *pathname, int flags) > +{ > + int retries_remaining, status; > + > + retries_remaining = 2; > + > + while (1) > + { > + status = open (pathname, flags); > + > + if (status >= 0 || retries_remaining == 0) > + break; > + else if (errno != EINTR && errno != EAGAIN) > + { > + retries_remaining--; > + sleep (1); > + } > + } > + > + return status; > +} > + > /* > * Function: open_procinfo_files > * > @@ -543,7 +577,7 @@ open_procinfo_files (procinfo *pi, int w > strcat (tmp, "/lwpctl"); > else > strcat (tmp, "/ctl"); > - fd = open (tmp, O_WRONLY); > + fd = open_with_retry (tmp, O_WRONLY); > if (fd <= 0) > return 0; /* fail */ > pi->ctl_fd = fd; > @@ -552,7 +586,7 @@ open_procinfo_files (procinfo *pi, int w > if (pi->tid) > return 0; /* there is no 'as' file descriptor for an lwp */ > strcat (tmp, "/as"); > - fd = open (tmp, O_RDWR); > + fd = open_with_retry (tmp, O_RDWR); > if (fd <= 0) > return 0; /* fail */ > pi->as_fd = fd; > @@ -562,7 +596,7 @@ open_procinfo_files (procinfo *pi, int w > strcat (tmp, "/lwpstatus"); > else > strcat (tmp, "/status"); > - fd = open (tmp, O_RDONLY); > + fd = open_with_retry (tmp, O_RDONLY); > if (fd <= 0) > return 0; /* fail */ > pi->status_fd = fd; > @@ -586,12 +620,13 @@ open_procinfo_files (procinfo *pi, int w > > > #ifdef PIOCTSTATUS /* OSF */ > - if ((fd = open (pi->pathname, O_RDWR)) == 0) /* Only one FD; just open it. */ > + /* Only one FD; just open it. */ > + if ((fd = open_with_retry (pi->pathname, O_RDWR)) == 0) > return 0; > #else /* Sol 2.5, Irix, other? */ > if (pi->tid == 0) /* Master procinfo for the process */ > { > - fd = open (pi->pathname, O_RDWR); > + fd = open_with_retry (pi->pathname, O_RDWR); > if (fd <= 0) > return 0; /* fail */ > } > @@ -843,7 +878,7 @@ load_syscalls (procinfo *pi) > > /* Open the file descriptor for the sysent file */ > sprintf (pathname, "/proc/%d/sysent", pi->pid); > - sysent_fd = open (pathname, O_RDONLY); > + sysent_fd = open_with_retry (pathname, O_RDONLY); > if (sysent_fd < 0) > { > error ("load_syscalls: Can't open /proc/%d/sysent", pi->pid); > @@ -2874,7 +2909,7 @@ proc_iterate_over_mappings (int (*func) > #ifdef NEW_PROC_API > /* Open map fd. */ > sprintf (pathname, "/proc/%d/map", pi->pid); > - if ((map_fd = open (pathname, O_RDONLY)) < 0) > + if ((map_fd = open_with_retry (pathname, O_RDONLY)) < 0) > proc_error (pi, "proc_iterate_over_mappings (open)", __LINE__); > > /* Make sure it gets closed again. */ > @@ -2903,7 +2938,7 @@ proc_iterate_over_mappings (int (*func) > { > sprintf (name, "/proc/%d/object/%s", pi->pid, map->pr_mapname); > /* Note: caller's responsibility to close this fd! */ > - fd = open (name, O_RDONLY); > + fd = open_with_retry (name, O_RDONLY); > /* Note: we don't test the above call for failure; > we just pass the FD on as given. Sometimes there is > no file, so the ioctl may return failure, but that's > @@ -2982,7 +3017,7 @@ proc_get_LDT_entry (procinfo *pi, int ke > > /* Open the file descriptor for the LDT table. */ > sprintf (pathname, "/proc/%d/ldt", pi->pid); > - if ((fd = open (pathname, O_RDONLY)) < 0) > + if ((fd = open_with_retry (pathname, O_RDONLY)) < 0) > { > proc_warn (pi, "proc_get_LDT_entry (open)", __LINE__); > return NULL;