diff for duplicates of <20080819221209.GF8331@fieldses.org> diff --git a/a/1.txt b/N1/1.txt index df5d0f0..54ade88 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,71 +1,48 @@ -On Fri, Aug 15, 2008 at 10:32:25AM +0900, Fernando Luis V=C3=A1zquez Ca= -o wrote: +On Fri, Aug 15, 2008 at 10:32:25AM +0900, Fernando Luis Vázquez Cao wrote: > Hi Bruce! ->=20 +> > On Thu, 2008-08-14 at 15:06 -0400, J. Bruce Fields wrote: > > On Thu, Aug 14, 2008 at 08:08:16PM +0900, NAKANO Hiroaki wrote: -> > > lockd uses time_before() to determine whether the grace period ha= -s -> > > expired. This would seem to be enough to avoid timer wrap-around = -issues, +> > > lockd uses time_before() to determine whether the grace period has +> > > expired. This would seem to be enough to avoid timer wrap-around issues, > > > but, unfortunately, that is not the case. The time_* family of -> > > comparison functions can be safely used to compare jiffies relati= -vely -> > > close in time, but they stop working after approximately LONG_MAX= -/2 -> > > ticks. nfsd can suffer this problem because the time_before() com= -parison -> > > in lockd() is not performed until the first request comes in, whi= -ch -> > > means that if there is no lockd traffic for more than LONG_MAX/2 = -ticks +> > > comparison functions can be safely used to compare jiffies relatively +> > > close in time, but they stop working after approximately LONG_MAX/2 +> > > ticks. nfsd can suffer this problem because the time_before() comparison +> > > in lockd() is not performed until the first request comes in, which +> > > means that if there is no lockd traffic for more than LONG_MAX/2 ticks > > > we are screwed. -> > >=20 -> > > The implication of this is that once time_before() starts misbeha= -ving -> > > any attempt from a NFS client to execute fcntl() will be received= - with a -> > > NLM_LCK_DENIED_GRACE_PERIOD message for 25 days (assuming HZ=3D10= -00). In -> > > other words, the 50 seconds grace period could turn into a grace = -period +> > > +> > > The implication of this is that once time_before() starts misbehaving +> > > any attempt from a NFS client to execute fcntl() will be received with a +> > > NLM_LCK_DENIED_GRACE_PERIOD message for 25 days (assuming HZ=1000). In +> > > other words, the 50 seconds grace period could turn into a grace period > > > of 50 days or more. -> > >=20 -> > > This patch corrects this behavior by implementing grace period wi= -th a +> > > +> > > This patch corrects this behavior by implementing grace period with a > > > (retriggerable) timer. -> > >=20 -> > > Note: This bug was analyzed independently by Oda-san <oda@valinux= -=2Eco.jp> +> > > +> > > Note: This bug was analyzed independently by Oda-san <oda@valinux.co.jp> > > > and myself. -> >=20 -> > Good catch! Did you actually run across this in practice? I would= -'ve -> > thought it relatively unusual to have a lockd that didn't receive i= -ts +> > +> > Good catch! Did you actually run across this in practice? I would've +> > thought it relatively unusual to have a lockd that didn't receive its > > first lock request until 25 days after startup. -> Yes, we did find this problem in production. More often than one woul= -d -> wish, installing new software in a system that has been running witho= -ut +> Yes, we did find this problem in production. More often than one would +> wish, installing new software in a system that has been running without > a hiccup for weeks or months is the only thing you will need to bring > mayhem. ->=20 -> > I still have a mild preference for a work struct just in case we en= -d up +> +> > I still have a mild preference for a work struct just in case we end up > > wanting to do something slightly more complicated to end the grace > > period, but I don't really have anything in mind. -> For simplicity I think we could we get Nakano-san's patch merged firs= -t. -> If needed, moving to a work-based solution should be relatively easil= -y. +> For simplicity I think we could we get Nakano-san's patch merged first. +> If needed, moving to a work-based solution should be relatively easily. -There's no real difference; patches I've been planning on submitting fo= -r +There's no real difference; patches I've been planning on submitting for 2.6.28 follow. -(We could submit a patch for 2.6.27, since it's a bugfix, but this isn'= -t +(We could submit a patch for 2.6.27, since it's a bugfix, but this isn't a new regression, so existing users at least won't be made any worse off, and this doesn't crash the server, or anything similarly drastic. It's still serious, just not quite serious enough to submit at this diff --git a/a/content_digest b/N1/content_digest index 0fc86ae..d1cb858 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,86 +1,62 @@ "ref\048A41220.8030203@oss.ntt.co.jp\0" "ref\020080814190652.GE23859@fieldses.org\0" "ref\01218763945.5291.19.camel@sebastian.kern.oss.ntt.co.jp\0" - "ref\01218763945.5291.19.camel-xpvPi5bcW5X5OjGIXfuPlhrrLbDL3r4M6qtp775pBPw@public.gmane.org\0" "From\0J. Bruce Fields <bfields@fieldses.org>\0" "Subject\0Re: [PATCH]lockd: fix handling of grace period after long periods of inactivity\0" "Date\0Tue, 19 Aug 2008 18:12:09 -0400\0" - "To\0Fernando Luis V\303\241zquez Cao <fernando-gVGce1chcLdL9jVzuh4AOg@public.gmane.org>\0" - "Cc\0NAKANO Hiroaki <nakano.hiroaki-gVGce1chcLdL9jVzuh4AOg@public.gmane.org>" + "To\0Fernando Luis V\303\241zquez Cao <fernando@oss.ntt.co.jp>\0" + "Cc\0NAKANO Hiroaki <nakano.hiroaki@oss.ntt.co.jp>" Trond.Myklebust@netapp.com neilb@suse.de linux-nfs@vger.kernel.org " linux-kernel@vger.kernel.org\0" "\00:1\0" "b\0" - "On Fri, Aug 15, 2008 at 10:32:25AM +0900, Fernando Luis V=C3=A1zquez Ca=\n" - "o wrote:\n" + "On Fri, Aug 15, 2008 at 10:32:25AM +0900, Fernando Luis V\303\241zquez Cao wrote:\n" "> Hi Bruce!\n" - ">=20\n" + "> \n" "> On Thu, 2008-08-14 at 15:06 -0400, J. Bruce Fields wrote:\n" "> > On Thu, Aug 14, 2008 at 08:08:16PM +0900, NAKANO Hiroaki wrote:\n" - "> > > lockd uses time_before() to determine whether the grace period ha=\n" - "s\n" - "> > > expired. This would seem to be enough to avoid timer wrap-around =\n" - "issues,\n" + "> > > lockd uses time_before() to determine whether the grace period has\n" + "> > > expired. This would seem to be enough to avoid timer wrap-around issues,\n" "> > > but, unfortunately, that is not the case. The time_* family of\n" - "> > > comparison functions can be safely used to compare jiffies relati=\n" - "vely\n" - "> > > close in time, but they stop working after approximately LONG_MAX=\n" - "/2\n" - "> > > ticks. nfsd can suffer this problem because the time_before() com=\n" - "parison\n" - "> > > in lockd() is not performed until the first request comes in, whi=\n" - "ch\n" - "> > > means that if there is no lockd traffic for more than LONG_MAX/2 =\n" - "ticks\n" + "> > > comparison functions can be safely used to compare jiffies relatively\n" + "> > > close in time, but they stop working after approximately LONG_MAX/2\n" + "> > > ticks. nfsd can suffer this problem because the time_before() comparison\n" + "> > > in lockd() is not performed until the first request comes in, which\n" + "> > > means that if there is no lockd traffic for more than LONG_MAX/2 ticks\n" "> > > we are screwed.\n" - "> > >=20\n" - "> > > The implication of this is that once time_before() starts misbeha=\n" - "ving\n" - "> > > any attempt from a NFS client to execute fcntl() will be received=\n" - " with a\n" - "> > > NLM_LCK_DENIED_GRACE_PERIOD message for 25 days (assuming HZ=3D10=\n" - "00). In\n" - "> > > other words, the 50 seconds grace period could turn into a grace =\n" - "period\n" + "> > > \n" + "> > > The implication of this is that once time_before() starts misbehaving\n" + "> > > any attempt from a NFS client to execute fcntl() will be received with a\n" + "> > > NLM_LCK_DENIED_GRACE_PERIOD message for 25 days (assuming HZ=1000). In\n" + "> > > other words, the 50 seconds grace period could turn into a grace period\n" "> > > of 50 days or more.\n" - "> > >=20\n" - "> > > This patch corrects this behavior by implementing grace period wi=\n" - "th a\n" + "> > > \n" + "> > > This patch corrects this behavior by implementing grace period with a\n" "> > > (retriggerable) timer.\n" - "> > >=20\n" - "> > > Note: This bug was analyzed independently by Oda-san <oda@valinux=\n" - "=2Eco.jp>\n" + "> > > \n" + "> > > Note: This bug was analyzed independently by Oda-san <oda@valinux.co.jp>\n" "> > > and myself.\n" - "> >=20\n" - "> > Good catch! Did you actually run across this in practice? I would=\n" - "'ve\n" - "> > thought it relatively unusual to have a lockd that didn't receive i=\n" - "ts\n" + "> > \n" + "> > Good catch! Did you actually run across this in practice? I would've\n" + "> > thought it relatively unusual to have a lockd that didn't receive its\n" "> > first lock request until 25 days after startup.\n" - "> Yes, we did find this problem in production. More often than one woul=\n" - "d\n" - "> wish, installing new software in a system that has been running witho=\n" - "ut\n" + "> Yes, we did find this problem in production. More often than one would\n" + "> wish, installing new software in a system that has been running without\n" "> a hiccup for weeks or months is the only thing you will need to bring\n" "> mayhem.\n" - ">=20\n" - "> > I still have a mild preference for a work struct just in case we en=\n" - "d up\n" + "> \n" + "> > I still have a mild preference for a work struct just in case we end up\n" "> > wanting to do something slightly more complicated to end the grace\n" "> > period, but I don't really have anything in mind.\n" - "> For simplicity I think we could we get Nakano-san's patch merged firs=\n" - "t.\n" - "> If needed, moving to a work-based solution should be relatively easil=\n" - "y.\n" + "> For simplicity I think we could we get Nakano-san's patch merged first.\n" + "> If needed, moving to a work-based solution should be relatively easily.\n" "\n" - "There's no real difference; patches I've been planning on submitting fo=\n" - "r\n" + "There's no real difference; patches I've been planning on submitting for\n" "2.6.28 follow.\n" "\n" - "(We could submit a patch for 2.6.27, since it's a bugfix, but this isn'=\n" - "t\n" + "(We could submit a patch for 2.6.27, since it's a bugfix, but this isn't\n" "a new regression, so existing users at least won't be made any worse\n" "off, and this doesn't crash the server, or anything similarly drastic.\n" "It's still serious, just not quite serious enough to submit at this\n" @@ -88,4 +64,4 @@ "\n" --b. -ef4247560fd30440040fec217cd46f8f1e4dfcf32eed16fa3fed3fe08c50d702 +a0dcc034bd930b065302dd122194bc660220258d23ce6b66f82f1a1f70d8e743
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.