On Wed, 25 Jan 2012 14:31:10 -0500
Steve Dickson <SteveD@xxxxxxxxxx> wrote:
>
>
> On 01/25/2012 02:09 PM, Jeff Layton wrote:
> > On Wed, 25 Jan 2012 13:16:24 -0500
> > Steve Dickson <SteveD@xxxxxxxxxx> wrote:
> >
> >> Hey Jeff,
> >>
> >> Commit inline...
> >>
> >> On 01/23/2012 03:02 PM, Jeff Layton wrote:
> >>> This can happen if nfsd is shut down and restarted. If that occurs,
> >>> then reopen the pipe so we're not waiting for data on the defunct
> >>> pipe.
> >>>
> >>> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> >>> ---
> >>> utils/nfsdcld/nfsdcld.c | 84 +++++++++++++++++++++++++++++++++++++++++-----
> >>> 1 files changed, 74 insertions(+), 10 deletions(-)
> >>>
> >>> diff --git a/utils/nfsdcld/nfsdcld.c b/utils/nfsdcld/nfsdcld.c
> >>> index b0c08e2..0dc5b37 100644
> >>> --- a/utils/nfsdcld/nfsdcld.c
> >>> +++ b/utils/nfsdcld/nfsdcld.c
> >>> @@ -57,6 +57,8 @@ struct cld_client {
> >>>
> >>> /* global variables */
> >>> static char *pipepath = DEFAULT_CLD_PATH;
> >>> +static int inotify_fd = -1;
> >>> +static struct event pipedir_event;
> >>>
> >>> static struct option longopts[] =
> >>> {
> >>> @@ -68,8 +70,10 @@ static struct option longopts[] =
> >>> { NULL, 0, 0, 0 },
> >>> };
> >>>
> >>> +
> >>> /* forward declarations */
> >>> static void cldcb(int UNUSED(fd), short which, void *data);
> >>> +static void cld_pipe_reopen(struct cld_client *clnt);
> >>>
> >>> static void
> >>> usage(char *progname)
> >>> @@ -80,10 +84,62 @@ usage(char *progname)
> >>>
> >>> #define INOTIFY_EVENT_MAX (sizeof(struct inotify_event) + NAME_MAX)
> >>>
> >>> +static void
> >>> +cld_inotify_cb(int UNUSED(fd), short which, void *data)
> >>> +{
> >>> + int ret, oldfd;
> >>> + char evbuf[INOTIFY_EVENT_MAX];
> >>> + char *dirc = NULL, *pname;
> >>> + struct inotify_event *event = (struct inotify_event *)evbuf;
> >>> + struct cld_client *clnt = data;
> >>> +
> >>> + if (which != EV_READ)
> >>> + return;
> >>> +
> >>> + dirc = strndup(pipepath, PATH_MAX);
> >>> + if (!dirc) {
> >>> + xlog_err("%s: unable to allocate memory", __func__);
> >>> + goto out;
> >>> + }
> >>> +
> >>> + ret = read(inotify_fd, evbuf, INOTIFY_EVENT_MAX);
> >>> + if (ret < 0) {
> >>> + xlog_err("%s: read from inotify fd failed: %m", __func__);
> >>> + goto out;
> >>> + }
> >>> +
> >>> + /* check to see if we have a filename in the evbuf */
> >>> + if (!event->len)
> >>> + goto out;
> >>> +
> >>> + pname = basename(dirc);
> >>> +
> >>> + /* does the filename match our pipe? */
> >>> + if (strncmp(pname, event->name, event->len))
> >>> + goto out;
> >>> +
> >>> + /*
> >>> + * reopen the pipe. The old fd is not closed until the new one is
> >>> + * opened, so we know they should be different if the reopen is
> >>> + * successful.
> >>> + */
> >>> + oldfd = clnt->cl_fd;
> >>> + do {
> >>> + cld_pipe_reopen(clnt);
> >>> + } while (oldfd == clnt->cl_fd);
> >> Doesn't this have a potential for an infinite loop?
> >>
> >> steved.
> >
> >
> > Yes. If reopening the new pipe continually fails then it will loop
> > forever.
> Would it be more accurate to say it would be spinning forever?
> Since there is no sleep or delay in cld_pipe_reopen, what's
> going to stop the daemon from spinning in a CPU bound loop?
>
Well, not spinning in a userspace loop...it'll continually be cycling on
an open() call that's not working for whatever reason. We sort of have
to loop on that though. I think the best we can do is add a sleep(1) in
there or something. Would that be sufficient?
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
