Return-Path: Received: from fieldses.org ([173.255.197.46]:44868 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726455AbeHTWRJ (ORCPT ); Mon, 20 Aug 2018 18:17:09 -0400 Date: Mon, 20 Aug 2018 15:00:22 -0400 To: Alan Swanson Cc: linux-nfs@vger.kernel.org Subject: Re: [PATCH V3] idmapd: Use inotify instead of dnotify Message-ID: <20180820190022.GB5468@fieldses.org> References: <20170621212338.32460-1-reiver@improbability.net> <20180819164342.14900-1-reiver@improbability.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <20180819164342.14900-1-reiver@improbability.net> From: bfields@fieldses.org (J. Bruce Fields) Sender: linux-nfs-owner@vger.kernel.org List-ID: On Sun, Aug 19, 2018 at 05:43:42PM +0100, Alan Swanson wrote: > Remove last use of dnotify in nfs-utils by bringing idmapd upto > date with (required) inotify use by gssd and blkmapd. OK by me.--b. > --- > utils/idmapd/idmapd.c | 32 +++++++++++++++++--------------- > 1 file changed, 17 insertions(+), 15 deletions(-) > > Got annoyed by the dnotify kernel requirement. Read the last > discussion "DNOTIFY to INOTIFY migration" posted in 2014 and while > still not officially depreciated, dnotify was officially "replaced" > by inotify in Linux 2.6.13 in 2005. Would be nice to sync this > requirement across the codebase. > > V2: Init wd to -1 not 0 > V3: Remove EV_PERSIST > > Have been using V3 patch in production for the last year since a > few days after last V2 email. > > diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c > index 4811e0f..2dd9c31 100644 > --- a/utils/idmapd/idmapd.c > +++ b/utils/idmapd/idmapd.c > @@ -36,7 +36,7 @@ > > #include > #include > -#include > +#include > #include > #include > #include > @@ -205,15 +205,16 @@ static void usage(char *progname) > int > main(int argc, char **argv) > { > - int fd = 0, opt, fg = 0, nfsdret = -1; > + int wd = -1, opt, fg = 0, nfsdret = -1; > struct idmap_clientq icq; > - struct event rootdirev, clntdirev, svrdirev; > + struct event rootdirev, clntdirev, svrdirev, inotifyev; > struct event initialize; > struct passwd *pw; > struct group *gr; > struct stat sb; > char *xpipefsdir = NULL; > int serverstart = 1, clientstart = 1; > + int inotify_fd; > int ret; > char *progname; > char *conf_path = NULL; > @@ -373,18 +374,15 @@ main(int argc, char **argv) > } > } > > - if ((fd = open(pipefsdir, O_RDONLY)) == -1) > - xlog_err("main: open(%s): %s", pipefsdir, strerror(errno)); > - > - if (fcntl(fd, F_SETSIG, SIGUSR1) == -1) > - xlog_err("main: fcntl(%s): %s", pipefsdir, strerror(errno)); > - > - if (fcntl(fd, F_NOTIFY, > - DN_CREATE | DN_DELETE | DN_MODIFY | DN_MULTISHOT) == -1) { > - xlog_err("main: fcntl(%s): %s", pipefsdir, strerror(errno)); > - if (errno == EINVAL) > - xlog_err("main: Possibly no Dnotify support in kernel."); > + inotify_fd = inotify_init1(IN_NONBLOCK); > + if (inotify_fd == -1) { > + xlog_err("Unable to initialise inotify_init1: %s\n", strerror(errno)); > + } else { > + wd = inotify_add_watch(inotify_fd, pipefsdir, IN_CREATE | IN_DELETE | IN_MODIFY); > + if (wd < 0) > + xlog_err("Unable to inotify_add_watch(%s): %s\n", pipefsdir, strerror(errno)); > } > + > TAILQ_INIT(&icq); > > /* These events are persistent */ > @@ -394,6 +392,10 @@ main(int argc, char **argv) > signal_add(&clntdirev, NULL); > signal_set(&svrdirev, SIGHUP, svrreopen, NULL); > signal_add(&svrdirev, NULL); > + if ( wd >= 0) { > + event_set(&inotifyev, inotify_fd, EV_READ, dirscancb, &icq); > + event_add(&inotifyev, NULL); > + } > > /* Fetch current state */ > /* (Delay till start of event_dispatch to avoid possibly losing > @@ -402,7 +404,7 @@ main(int argc, char **argv) > evtimer_add(&initialize, &now); > } > > - if (nfsdret != 0 && fd == 0) > + if (nfsdret != 0 && wd < 0) > xlog_err("main: Neither NFS client nor NFSd found"); > > daemon_ready(); > -- > 2.16.4