On 01/25/2012 03:28 PM, Jeff Layton wrote:
> 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?
>
I still think it going to needlessly suck up CPU cycles...
The way I handled this in the rpc.idmapd daemon was to do the
reopen on a SIGHUP signal. Then in NFS server initscript
I did the following:
/usr/bin/pkill -HUP rpc.idmapd
Thoughts?
steved.
--
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
