* [PATCH] nfsd/blocklayout: accept any minlength @ 2015-10-09 13:03 Christoph Hellwig 2015-10-09 15:28 ` J. Bruce Fields 0 siblings, 1 reply; 10+ messages in thread From: Christoph Hellwig @ 2015-10-09 13:03 UTC (permalink / raw) To: bfields; +Cc: linux-nfs, stable Recent Linux clients have started to send less than blocksize minglength requests. Given that minlength is just a hint except for the magic value of zero just don't check for it except for same zero value. Without this we'll hang forever during fsx runs. Cc: stable@vger.kernel.org Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/nfsd/blocklayout.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c index cdefaa3..c29d942 100644 --- a/fs/nfsd/blocklayout.c +++ b/fs/nfsd/blocklayout.c @@ -56,14 +56,6 @@ nfsd4_block_proc_layoutget(struct inode *inode, const struct svc_fh *fhp, u32 device_generation = 0; int error; - /* - * We do not attempt to support I/O smaller than the fs block size, - * or not aligned to it. - */ - if (args->lg_minlength < block_size) { - dprintk("pnfsd: I/O too small\n"); - goto out_layoutunavailable; - } if (seg->offset & (block_size - 1)) { dprintk("pnfsd: I/O misaligned\n"); goto out_layoutunavailable; -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] nfsd/blocklayout: accept any minlength 2015-10-09 13:03 [PATCH] nfsd/blocklayout: accept any minlength Christoph Hellwig @ 2015-10-09 15:28 ` J. Bruce Fields 2015-10-09 17:04 ` Christoph Hellwig 0 siblings, 1 reply; 10+ messages in thread From: J. Bruce Fields @ 2015-10-09 15:28 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linux-nfs, stable On Fri, Oct 09, 2015 at 03:03:26PM +0200, Christoph Hellwig wrote: > Recent Linux clients have started to send less than blocksize minglength > requests. Given that minlength is just a hint except for the magic value > of zero just don't check for it except for same zero value. Without > this we'll hang forever during fsx runs. OK, planning to apply for 4.3 just on the assumption that you know what you're doing, but: I don't get it--it looks like the worst that can happen here is we just reuturn LAYOUTUNAVAILABLE to LAYOUTGET. Shouldn't the client then just fall back on normal NFS IO? Why the hang? --b. > > Cc: stable@vger.kernel.org > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/nfsd/blocklayout.c | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c > index cdefaa3..c29d942 100644 > --- a/fs/nfsd/blocklayout.c > +++ b/fs/nfsd/blocklayout.c > @@ -56,14 +56,6 @@ nfsd4_block_proc_layoutget(struct inode *inode, const struct svc_fh *fhp, > u32 device_generation = 0; > int error; > > - /* > - * We do not attempt to support I/O smaller than the fs block size, > - * or not aligned to it. > - */ > - if (args->lg_minlength < block_size) { > - dprintk("pnfsd: I/O too small\n"); > - goto out_layoutunavailable; > - } > if (seg->offset & (block_size - 1)) { > dprintk("pnfsd: I/O misaligned\n"); > goto out_layoutunavailable; > -- > 1.9.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] nfsd/blocklayout: accept any minlength 2015-10-09 15:28 ` J. Bruce Fields @ 2015-10-09 17:04 ` Christoph Hellwig 2015-10-09 17:45 ` J. Bruce Fields 0 siblings, 1 reply; 10+ messages in thread From: Christoph Hellwig @ 2015-10-09 17:04 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs, stable On Fri, Oct 09, 2015 at 11:28:03AM -0400, J. Bruce Fields wrote: > OK, planning to apply for 4.3 just on the assumption that you know what > you're doing, but: I don't get it--it looks like the worst that can > happen here is we just reuturn LAYOUTUNAVAILABLE to LAYOUTGET. > Shouldn't the client then just fall back on normal NFS IO? Why the > hang? I've just retested with Trond's latest tree and can't reproduce the hang anymore. It used to fence the client due to a lack of response, but that might have been a different client bug that has now been fixed. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] nfsd/blocklayout: accept any minlength 2015-10-09 17:04 ` Christoph Hellwig @ 2015-10-09 17:45 ` J. Bruce Fields 2015-10-09 17:54 ` Trond Myklebust 2015-10-11 13:03 ` Christoph Hellwig 0 siblings, 2 replies; 10+ messages in thread From: J. Bruce Fields @ 2015-10-09 17:45 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linux-nfs, stable On Fri, Oct 09, 2015 at 07:04:00PM +0200, Christoph Hellwig wrote: > On Fri, Oct 09, 2015 at 11:28:03AM -0400, J. Bruce Fields wrote: > > OK, planning to apply for 4.3 just on the assumption that you know what > > you're doing, but: I don't get it--it looks like the worst that can > > happen here is we just reuturn LAYOUTUNAVAILABLE to LAYOUTGET. > > Shouldn't the client then just fall back on normal NFS IO? Why the > > hang? > > I've just retested with Trond's latest tree and can't reproduce the > hang anymore. It used to fence the client due to a lack of response, > but that might have been a different client bug that has now been fixed. OK, makes sense. This still looks like a harmless enough change, but is it still stable and 4.3 material? If it affected a released client then it's probably worth it even if it's really a client bug. If it's just something you saw once against an -rc1, I'd rather leave it for 4.4. --b. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] nfsd/blocklayout: accept any minlength 2015-10-09 17:45 ` J. Bruce Fields @ 2015-10-09 17:54 ` Trond Myklebust 2015-10-09 20:04 ` J. Bruce Fields 2015-10-11 13:03 ` Christoph Hellwig 1 sibling, 1 reply; 10+ messages in thread From: Trond Myklebust @ 2015-10-09 17:54 UTC (permalink / raw) To: J. Bruce Fields Cc: Christoph Hellwig, Linux NFS Mailing List, Stable Tree Mailing List On Fri, Oct 9, 2015 at 1:45 PM, J. Bruce Fields <bfields@fieldses.org> wrote: > > On Fri, Oct 09, 2015 at 07:04:00PM +0200, Christoph Hellwig wrote: > > On Fri, Oct 09, 2015 at 11:28:03AM -0400, J. Bruce Fields wrote: > > > OK, planning to apply for 4.3 just on the assumption that you know what > > > you're doing, but: I don't get it--it looks like the worst that can > > > happen here is we just reuturn LAYOUTUNAVAILABLE to LAYOUTGET. > > > Shouldn't the client then just fall back on normal NFS IO? Why the > > > hang? > > > > I've just retested with Trond's latest tree and can't reproduce the > > hang anymore. It used to fence the client due to a lack of response, > > but that might have been a different client bug that has now been fixed. > > OK, makes sense. > > This still looks like a harmless enough change, but is it still stable > and 4.3 material? > > If it affected a released client then it's probably worth it even if > it's really a client bug. If it's just something you saw once against > an -rc1, I'd rather leave it for 4.4. It's not a client bug. The server is supposed to comply with the requirements in table 13 of https://tools.ietf.org/html/rfc5661#section-18.43.3 Note that it should also be returning NFS4ERR_BADLAYOUT (or NFS4ERR_LAYOUTTRYLATER if loga_minlength == 0) instead of layoutunavailable if the loga_minlength request cannot be met. Cheers Trond ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] nfsd/blocklayout: accept any minlength 2015-10-09 17:54 ` Trond Myklebust @ 2015-10-09 20:04 ` J. Bruce Fields 2015-10-09 20:14 ` J. Bruce Fields 2015-10-11 13:08 ` Christoph Hellwig 0 siblings, 2 replies; 10+ messages in thread From: J. Bruce Fields @ 2015-10-09 20:04 UTC (permalink / raw) To: Trond Myklebust Cc: Christoph Hellwig, Linux NFS Mailing List, Stable Tree Mailing List On Fri, Oct 09, 2015 at 01:54:22PM -0400, Trond Myklebust wrote: > On Fri, Oct 9, 2015 at 1:45 PM, J. Bruce Fields <bfields@fieldses.org> wrote: > > > > On Fri, Oct 09, 2015 at 07:04:00PM +0200, Christoph Hellwig wrote: > > > On Fri, Oct 09, 2015 at 11:28:03AM -0400, J. Bruce Fields wrote: > > > > OK, planning to apply for 4.3 just on the assumption that you know what > > > > you're doing, but: I don't get it--it looks like the worst that can > > > > happen here is we just reuturn LAYOUTUNAVAILABLE to LAYOUTGET. > > > > Shouldn't the client then just fall back on normal NFS IO? Why the > > > > hang? > > > > > > I've just retested with Trond's latest tree and can't reproduce the > > > hang anymore. It used to fence the client due to a lack of response, > > > but that might have been a different client bug that has now been fixed. > > > > OK, makes sense. > > > > This still looks like a harmless enough change, but is it still stable > > and 4.3 material? > > > > If it affected a released client then it's probably worth it even if > > it's really a client bug. If it's just something you saw once against > > an -rc1, I'd rather leave it for 4.4. > > It's not a client bug. > > The server is supposed to comply with the requirements in table 13 of > https://tools.ietf.org/html/rfc5661#section-18.43.3 > Note that it should also be returning NFS4ERR_BADLAYOUT (or > NFS4ERR_LAYOUTTRYLATER if loga_minlength == 0) instead of > layoutunavailable if the loga_minlength request cannot be met. I had some ideas that layouts were something a server could decline just on random whim. Rereading that section.... OK, looks like I was confused, TRYLATER is the closest we come to random whim. So the following condition on the alignments of the offset also looks wrong. Christoph, should it be rounding the offset down instead of rejecting in that case? And other layoutunavailable cases might need review too. --b. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] nfsd/blocklayout: accept any minlength 2015-10-09 20:04 ` J. Bruce Fields @ 2015-10-09 20:14 ` J. Bruce Fields 2015-10-11 13:08 ` Christoph Hellwig 1 sibling, 0 replies; 10+ messages in thread From: J. Bruce Fields @ 2015-10-09 20:14 UTC (permalink / raw) To: Trond Myklebust Cc: Christoph Hellwig, Linux NFS Mailing List, Stable Tree Mailing List On Fri, Oct 09, 2015 at 04:04:38PM -0400, J. Bruce Fields wrote: > On Fri, Oct 09, 2015 at 01:54:22PM -0400, Trond Myklebust wrote: > > On Fri, Oct 9, 2015 at 1:45 PM, J. Bruce Fields <bfields@fieldses.org> wrote: > > > > > > On Fri, Oct 09, 2015 at 07:04:00PM +0200, Christoph Hellwig wrote: > > > > On Fri, Oct 09, 2015 at 11:28:03AM -0400, J. Bruce Fields wrote: > > > > > OK, planning to apply for 4.3 just on the assumption that you know what > > > > > you're doing, but: I don't get it--it looks like the worst that can > > > > > happen here is we just reuturn LAYOUTUNAVAILABLE to LAYOUTGET. > > > > > Shouldn't the client then just fall back on normal NFS IO? Why the > > > > > hang? > > > > > > > > I've just retested with Trond's latest tree and can't reproduce the > > > > hang anymore. It used to fence the client due to a lack of response, > > > > but that might have been a different client bug that has now been fixed. > > > > > > OK, makes sense. > > > > > > This still looks like a harmless enough change, but is it still stable > > > and 4.3 material? > > > > > > If it affected a released client then it's probably worth it even if > > > it's really a client bug. If it's just something you saw once against > > > an -rc1, I'd rather leave it for 4.4. > > > > It's not a client bug. > > > > The server is supposed to comply with the requirements in table 13 of > > https://tools.ietf.org/html/rfc5661#section-18.43.3 > > Note that it should also be returning NFS4ERR_BADLAYOUT (or > > NFS4ERR_LAYOUTTRYLATER if loga_minlength == 0) instead of > > layoutunavailable if the loga_minlength request cannot be met. > > I had some ideas that layouts were something a server could decline just > on random whim. Rereading that section.... OK, looks like I was > confused, TRYLATER is the closest we come to random whim. So, updated the changelog as follows, let me know if I've got anything wrong. --b. commit 8c3ad9cb7343 Author: Christoph Hellwig <hch@lst.de> Date: Fri Oct 9 15:03:26 2015 +0200 nfsd/blocklayout: accept any minlength Recent Linux clients have started to send GETLAYOUT requests with minlength less than blocksize. Servers aren't really allowed to impose this kind of restriction on layouts; see RFC 5661 section 18.43.3 for details. This has been observed to cause indefinite hangs on fsx runs on some clients. Cc: stable@vger.kernel.org Signed-off-by: Christoph Hellwig <hch@lst.de> Signed-off-by: J. Bruce Fields <bfields@redhat.com> diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c index cdefaa331a07..c29d9421bd5e 100644 --- a/fs/nfsd/blocklayout.c +++ b/fs/nfsd/blocklayout.c @@ -56,14 +56,6 @@ nfsd4_block_proc_layoutget(struct inode *inode, const struct svc_fh *fhp, u32 device_generation = 0; int error; - /* - * We do not attempt to support I/O smaller than the fs block size, - * or not aligned to it. - */ - if (args->lg_minlength < block_size) { - dprintk("pnfsd: I/O too small\n"); - goto out_layoutunavailable; - } if (seg->offset & (block_size - 1)) { dprintk("pnfsd: I/O misaligned\n"); goto out_layoutunavailable; ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] nfsd/blocklayout: accept any minlength 2015-10-09 20:04 ` J. Bruce Fields 2015-10-09 20:14 ` J. Bruce Fields @ 2015-10-11 13:08 ` Christoph Hellwig 2015-10-12 20:18 ` J. Bruce Fields 1 sibling, 1 reply; 10+ messages in thread From: Christoph Hellwig @ 2015-10-11 13:08 UTC (permalink / raw) To: J. Bruce Fields Cc: Trond Myklebust, Christoph Hellwig, Linux NFS Mailing List, Stable Tree Mailing List On Fri, Oct 09, 2015 at 04:04:38PM -0400, J. Bruce Fields wrote: > I had some ideas that layouts were something a server could decline just > on random whim. Rereading that section.... OK, looks like I was > confused, TRYLATER is the closest we come to random whim. > > So the following condition on the alignments of the offset also looks > wrong. Christoph, should it be rounding the offset down instead of > rejecting in that case? RFC5663 is very explicit about the extents being aligned, but doesn't say anything about LAYOUTGET requests. It's a bit of a gray area, but I think not handing out a layout is still the best thing to do as a client has to be really confused to ask for an unaligned layout and expect to get an aligned extent back. I just need to check for the best possible error value. > And other layoutunavailable cases might need review too. Will do. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] nfsd/blocklayout: accept any minlength 2015-10-11 13:08 ` Christoph Hellwig @ 2015-10-12 20:18 ` J. Bruce Fields 0 siblings, 0 replies; 10+ messages in thread From: J. Bruce Fields @ 2015-10-12 20:18 UTC (permalink / raw) To: Christoph Hellwig Cc: Trond Myklebust, Linux NFS Mailing List, Stable Tree Mailing List On Sun, Oct 11, 2015 at 03:08:22PM +0200, Christoph Hellwig wrote: > On Fri, Oct 09, 2015 at 04:04:38PM -0400, J. Bruce Fields wrote: > > I had some ideas that layouts were something a server could decline just > > on random whim. Rereading that section.... OK, looks like I was > > confused, TRYLATER is the closest we come to random whim. > > > > So the following condition on the alignments of the offset also looks > > wrong. Christoph, should it be rounding the offset down instead of > > rejecting in that case? > > RFC5663 is very explicit about the extents being aligned, but doesn't > say anything about LAYOUTGET requests. It's a bit of a gray area, but > I think not handing out a layout is still the best thing to do as a client > has to be really confused to ask for an unaligned layout and expect to get > an aligned extent back. http://tools.ietf.org/html/rfc5661#section-18.43.3 always allows a server to round down the offset, is there any downside to doing that? > I just need to check for the best possible error value. INVAL or BADLAYOUT, I guess. But just rounding down seems harmless and compatible with the letter of the spec. Though without any reason or a client to request an unaligned offset I guess it's a bit academic. --b. > > > And other layoutunavailable cases might need review too. > > Will do. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] nfsd/blocklayout: accept any minlength 2015-10-09 17:45 ` J. Bruce Fields 2015-10-09 17:54 ` Trond Myklebust @ 2015-10-11 13:03 ` Christoph Hellwig 1 sibling, 0 replies; 10+ messages in thread From: Christoph Hellwig @ 2015-10-11 13:03 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Christoph Hellwig, linux-nfs, stable On Fri, Oct 09, 2015 at 01:45:56PM -0400, J. Bruce Fields wrote: > If it affected a released client then it's probably worth it even if > it's really a client bug. If it's just something you saw once against > an -rc1, I'd rather leave it for 4.4. This one is a server bug uncovered by a client change that went into 4.3-rc. Additionally when I first reproduced this in Trond's tree it also caused a hang, but that is now gone. Given how little activity the server had around time I'm confident to blame that part on an unknown client bug. So yes, we really need it in ASAP and in stable all the way back. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-10-12 20:18 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-10-09 13:03 [PATCH] nfsd/blocklayout: accept any minlength Christoph Hellwig 2015-10-09 15:28 ` J. Bruce Fields 2015-10-09 17:04 ` Christoph Hellwig 2015-10-09 17:45 ` J. Bruce Fields 2015-10-09 17:54 ` Trond Myklebust 2015-10-09 20:04 ` J. Bruce Fields 2015-10-09 20:14 ` J. Bruce Fields 2015-10-11 13:08 ` Christoph Hellwig 2015-10-12 20:18 ` J. Bruce Fields 2015-10-11 13:03 ` Christoph Hellwig
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.