* [PATCH 05/39] whiteout/NFSD: Don't return information about whiteouts to userspace [not found] <1272928358-20854-1-git-send-email-vaurora@redhat.com> @ 2010-05-03 23:12 ` Valerie Aurora 2010-05-03 23:37 ` Neil Brown 0 siblings, 1 reply; 6+ messages in thread From: Valerie Aurora @ 2010-05-03 23:12 UTC (permalink / raw) To: Alexander Viro Cc: linux-fsdevel, linux-kernel, Christoph Hellwig, Jan Blunck, David Woodhouse, Valerie Aurora, linux-nfs, J. Bruce Fields, Neil Brown From: Jan Blunck <jblunck@suse.de> Userspace isn't ready for handling another file type, so silently drop whiteout directory entries before they leave the kernel. Signed-off-by: Jan Blunck <jblunck@suse.de> Signed-off-by: David Woodhouse <dwmw2@infradead.org> Signed-off-by: Valerie Aurora <vaurora@redhat.com> Cc: linux-nfs@vger.kernel.org Cc: "J. Bruce Fields" <bfields@fieldses.org> Cc: Neil Brown <neilb@suse.de> --- fs/compat.c | 9 +++++++++ fs/nfsd/nfs3xdr.c | 5 +++++ fs/nfsd/nfs4xdr.c | 5 +++++ fs/nfsd/nfsxdr.c | 4 ++++ fs/readdir.c | 9 +++++++++ 5 files changed, 32 insertions(+), 0 deletions(-) diff --git a/fs/compat.c b/fs/compat.c index 4b6ed03..adec661 100644 --- a/fs/compat.c +++ b/fs/compat.c @@ -839,6 +839,9 @@ static int compat_fillonedir(void *__buf, const char *name, int namlen, struct compat_old_linux_dirent __user *dirent; compat_ulong_t d_ino; + if (d_type == DT_WHT) + return 0; + if (buf->result) return -EINVAL; d_ino = ino; @@ -910,6 +913,9 @@ static int compat_filldir(void *__buf, const char *name, int namlen, compat_ulong_t d_ino; int reclen = ALIGN(NAME_OFFSET(dirent) + namlen + 2, sizeof(compat_long_t)); + if (d_type == DT_WHT) + return 0; + buf->error = -EINVAL; /* only used if we fail.. */ if (reclen > buf->count) return -EINVAL; @@ -999,6 +1005,9 @@ static int compat_filldir64(void * __buf, const char * name, int namlen, loff_t int reclen = ALIGN(jj + namlen + 1, sizeof(u64)); u64 off; + if (d_type == DT_WHT) + return 0; + buf->error = -EINVAL; /* only used if we fail.. */ if (reclen > buf->count) return -EINVAL; diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c index 2a533a0..9b96f5a 100644 --- a/fs/nfsd/nfs3xdr.c +++ b/fs/nfsd/nfs3xdr.c @@ -885,6 +885,11 @@ encode_entry(struct readdir_cd *ccd, const char *name, int namlen, int elen; /* estimated entry length in words */ int num_entry_words = 0; /* actual number of words */ + if (d_type == DT_WHT) { + cd->common.err = nfs_ok; + return 0; + } + if (cd->offset) { u64 offset64 = offset; diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c index 34ccf81..2ddf144 100644 --- a/fs/nfsd/nfs4xdr.c +++ b/fs/nfsd/nfs4xdr.c @@ -2269,6 +2269,11 @@ nfsd4_encode_dirent(void *ccdv, const char *name, int namlen, return 0; } + if (d_type == DT_WHT) { + cd->common.err = nfs_ok; + return 0; + } + if (cd->offset) xdr_encode_hyper(cd->offset, (u64) offset); diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c index 4ce005d..0e57d4b 100644 --- a/fs/nfsd/nfsxdr.c +++ b/fs/nfsd/nfsxdr.c @@ -503,6 +503,10 @@ nfssvc_encode_entry(void *ccdv, const char *name, namlen, name, offset, ino); */ + if (d_type == DT_WHT) { + cd->common.err = nfs_ok; + return 0; + } if (offset > ~((u32) 0)) { cd->common.err = nfserr_fbig; return -EINVAL; diff --git a/fs/readdir.c b/fs/readdir.c index 7723401..3a48491 100644 --- a/fs/readdir.c +++ b/fs/readdir.c @@ -77,6 +77,9 @@ static int fillonedir(void * __buf, const char * name, int namlen, loff_t offset struct old_linux_dirent __user * dirent; unsigned long d_ino; + if (d_type == DT_WHT) + return 0; + if (buf->result) return -EINVAL; d_ino = ino; @@ -154,6 +157,9 @@ static int filldir(void * __buf, const char * name, int namlen, loff_t offset, unsigned long d_ino; int reclen = ALIGN(NAME_OFFSET(dirent) + namlen + 2, sizeof(long)); + if (d_type == DT_WHT) + return 0; + buf->error = -EINVAL; /* only used if we fail.. */ if (reclen > buf->count) return -EINVAL; @@ -239,6 +245,9 @@ static int filldir64(void * __buf, const char * name, int namlen, loff_t offset, struct getdents_callback64 * buf = (struct getdents_callback64 *) __buf; int reclen = ALIGN(NAME_OFFSET(dirent) + namlen + 1, sizeof(u64)); + if (d_type == DT_WHT) + return 0; + buf->error = -EINVAL; /* only used if we fail.. */ if (reclen > buf->count) return -EINVAL; -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 05/39] whiteout/NFSD: Don't return information about whiteouts to userspace 2010-05-03 23:12 ` [PATCH 05/39] whiteout/NFSD: Don't return information about whiteouts to userspace Valerie Aurora @ 2010-05-03 23:37 ` Neil Brown 2010-05-06 18:01 ` Valerie Aurora 0 siblings, 1 reply; 6+ messages in thread From: Neil Brown @ 2010-05-03 23:37 UTC (permalink / raw) To: Valerie Aurora Cc: Alexander Viro, linux-fsdevel, linux-kernel, Christoph Hellwig, Jan Blunck, David Woodhouse, linux-nfs, J. Bruce Fields On Mon, 3 May 2010 16:12:04 -0700 Valerie Aurora <vaurora@redhat.com> wrote: > From: Jan Blunck <jblunck@suse.de> > > Userspace isn't ready for handling another file type, so silently drop > whiteout directory entries before they leave the kernel. Feels very intrusive doesn't it.... Have you considered something like the following? NeilBrown diff --git a/fs/readdir.c b/fs/readdir.c index 7723401..4c5b347 100644 --- a/fs/readdir.c +++ b/fs/readdir.c @@ -19,10 +19,26 @@ #include <asm/uaccess.h> +struct readdir_info { + filldir_t filler; + void *data; +}; + +static int white_out(void *vrdi, const char *name, int namlen, + loff_t offset, u64 ino, unsigned int d_type) +{ + struct readdir_info *rdi = vrdi; + if (d_type == DT_WHT) + return 0; + return rdi->filler(rdi->data, name, namlen, offset, info, d_type); +} + int vfs_readdir(struct file *file, filldir_t filler, void *buf) { struct inode *inode = file->f_path.dentry->d_inode; int res = -ENOTDIR; + struct readir_info rdi = { filler, buf }; + if (!file->f_op || !file->f_op->readdir) goto out; @@ -36,7 +52,7 @@ int vfs_readdir(struct file *file, filldir_t filler, void *buf) res = -ENOENT; if (!IS_DEADDIR(inode)) { - res = file->f_op->readdir(file, buf, filler); + res = file->f_op->readdir(file, &rdi, white_out); file_accessed(file); } mutex_unlock(&inode->i_mutex); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 05/39] whiteout/NFSD: Don't return information about whiteouts to userspace 2010-05-03 23:37 ` Neil Brown @ 2010-05-06 18:01 ` Valerie Aurora 2010-05-06 21:18 ` Neil Brown 0 siblings, 1 reply; 6+ messages in thread From: Valerie Aurora @ 2010-05-06 18:01 UTC (permalink / raw) To: Neil Brown Cc: Alexander Viro, linux-fsdevel, linux-kernel, Christoph Hellwig, Jan Blunck, David Woodhouse, linux-nfs, J. Bruce Fields On Tue, May 04, 2010 at 09:37:31AM +1000, Neil Brown wrote: > On Mon, 3 May 2010 16:12:04 -0700 > Valerie Aurora <vaurora@redhat.com> wrote: > > > From: Jan Blunck <jblunck@suse.de> > > > > Userspace isn't ready for handling another file type, so silently drop > > whiteout directory entries before they leave the kernel. > > Feels very intrusive doesn't it.... > > Have you considered something like the following? Hrm, I see how that could be more elegant, but I'd rather avoid yet another layer of function pointer passing around. This code is already hard enough to review... -VAL > NeilBrown > > diff --git a/fs/readdir.c b/fs/readdir.c > index 7723401..4c5b347 100644 > --- a/fs/readdir.c > +++ b/fs/readdir.c > @@ -19,10 +19,26 @@ > > #include <asm/uaccess.h> > > +struct readdir_info { > + filldir_t filler; > + void *data; > +}; > + > +static int white_out(void *vrdi, const char *name, int namlen, > + loff_t offset, u64 ino, unsigned int d_type) > +{ > + struct readdir_info *rdi = vrdi; > + if (d_type == DT_WHT) > + return 0; > + return rdi->filler(rdi->data, name, namlen, offset, info, d_type); > +} > + > int vfs_readdir(struct file *file, filldir_t filler, void *buf) > { > struct inode *inode = file->f_path.dentry->d_inode; > int res = -ENOTDIR; > + struct readir_info rdi = { filler, buf }; > + > if (!file->f_op || !file->f_op->readdir) > goto out; > > @@ -36,7 +52,7 @@ int vfs_readdir(struct file *file, filldir_t filler, void *buf) > > res = -ENOENT; > if (!IS_DEADDIR(inode)) { > - res = file->f_op->readdir(file, buf, filler); > + res = file->f_op->readdir(file, &rdi, white_out); > file_accessed(file); > } > mutex_unlock(&inode->i_mutex); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 05/39] whiteout/NFSD: Don't return information about whiteouts to userspace 2010-05-06 18:01 ` Valerie Aurora @ 2010-05-06 21:18 ` Neil Brown 2010-05-17 19:51 ` Valerie Aurora 0 siblings, 1 reply; 6+ messages in thread From: Neil Brown @ 2010-05-06 21:18 UTC (permalink / raw) To: Valerie Aurora Cc: Alexander Viro, linux-fsdevel, linux-kernel, Christoph Hellwig, Jan Blunck, David Woodhouse, linux-nfs, J. Bruce Fields On Thu, 6 May 2010 14:01:51 -0400 Valerie Aurora <vaurora@redhat.com> wrote: > On Tue, May 04, 2010 at 09:37:31AM +1000, Neil Brown wrote: > > On Mon, 3 May 2010 16:12:04 -0700 > > Valerie Aurora <vaurora@redhat.com> wrote: > > > > > From: Jan Blunck <jblunck@suse.de> > > > > > > Userspace isn't ready for handling another file type, so silently drop > > > whiteout directory entries before they leave the kernel. > > > > Feels very intrusive doesn't it.... > > > > Have you considered something like the following? > > Hrm, I see how that could be more elegant, but I'd rather avoid yet > another layer of function pointer passing around. This code is > already hard enough to review... Yes, the extra indirection is a bit of a negative, but I don't think this patch is harder to review than the alternate. From a numerical perspective, with this patch you only need to look at the various places that ->readdir is called to be sure it is always correct. There are about 3. With the original you need to look at ever filldir function. Jan has found 9. And from a maintainability perspective, I think my approach is safer. Given that there are 9 filldir functions already, the chance that a need will be found for another seems good, and the chance that the coder will know to check for DT_WHT is a best even. Conversely if another call to ->readdir were added it is likely that nothing would need to be done. Of course just counting things doesn't give a completely picture but I think it can be indicative. NeilBrown > > -VAL > > > NeilBrown > > > > diff --git a/fs/readdir.c b/fs/readdir.c > > index 7723401..4c5b347 100644 > > --- a/fs/readdir.c > > +++ b/fs/readdir.c > > @@ -19,10 +19,26 @@ > > > > #include <asm/uaccess.h> > > > > +struct readdir_info { > > + filldir_t filler; > > + void *data; > > +}; > > + > > +static int white_out(void *vrdi, const char *name, int namlen, > > + loff_t offset, u64 ino, unsigned int d_type) > > +{ > > + struct readdir_info *rdi = vrdi; > > + if (d_type == DT_WHT) > > + return 0; > > + return rdi->filler(rdi->data, name, namlen, offset, info, d_type); > > +} > > + > > int vfs_readdir(struct file *file, filldir_t filler, void *buf) > > { > > struct inode *inode = file->f_path.dentry->d_inode; > > int res = -ENOTDIR; > > + struct readir_info rdi = { filler, buf }; > > + > > if (!file->f_op || !file->f_op->readdir) > > goto out; > > > > @@ -36,7 +52,7 @@ int vfs_readdir(struct file *file, filldir_t filler, void *buf) > > > > res = -ENOENT; > > if (!IS_DEADDIR(inode)) { > > - res = file->f_op->readdir(file, buf, filler); > > + res = file->f_op->readdir(file, &rdi, white_out); > > file_accessed(file); > > } > > mutex_unlock(&inode->i_mutex); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 05/39] whiteout/NFSD: Don't return information about whiteouts to userspace 2010-05-06 21:18 ` Neil Brown @ 2010-05-17 19:51 ` Valerie Aurora 0 siblings, 0 replies; 6+ messages in thread From: Valerie Aurora @ 2010-05-17 19:51 UTC (permalink / raw) To: Neil Brown Cc: Alexander Viro, linux-fsdevel, linux-kernel, Christoph Hellwig, Jan Blunck, David Woodhouse, linux-nfs, J. Bruce Fields On Fri, May 07, 2010 at 07:18:08AM +1000, Neil Brown wrote: > On Thu, 6 May 2010 14:01:51 -0400 > Valerie Aurora <vaurora@redhat.com> wrote: > > > On Tue, May 04, 2010 at 09:37:31AM +1000, Neil Brown wrote: > > > On Mon, 3 May 2010 16:12:04 -0700 > > > Valerie Aurora <vaurora@redhat.com> wrote: > > > > > > > From: Jan Blunck <jblunck@suse.de> > > > > > > > > Userspace isn't ready for handling another file type, so silently drop > > > > whiteout directory entries before they leave the kernel. > > > > > > Feels very intrusive doesn't it.... > > > > > > Have you considered something like the following? > > > > Hrm, I see how that could be more elegant, but I'd rather avoid yet > > another layer of function pointer passing around. This code is > > already hard enough to review... > > Yes, the extra indirection is a bit of a negative, but I don't think this > patch is harder to review than the alternate. > From a numerical perspective, with this patch you only need to look at the > various places that ->readdir is called to be sure it is always correct. > There are about 3. With the original you need to look at ever filldir > function. Jan has found 9. > > And from a maintainability perspective, I think my approach is safer. Given > that there are 9 filldir functions already, the chance that a need will be > found for another seems good, and the chance that the coder will know to > check for DT_WHT is a best even. Conversely if another call to ->readdir > were added it is likely that nothing would need to be done. > > Of course just counting things doesn't give a completely picture but I think > it can be indicative. Okay, good points. Let me try it out after getting this next rewrite done. Thanks, -VAL ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <1281282776-5447-1-git-send-email-vaurora@redhat.com>]
* [PATCH 05/39] whiteout/NFSD: Don't return information about whiteouts to userspace [not found] <1281282776-5447-1-git-send-email-vaurora@redhat.com> @ 2010-08-08 15:52 ` Valerie Aurora 0 siblings, 0 replies; 6+ messages in thread From: Valerie Aurora @ 2010-08-08 15:52 UTC (permalink / raw) To: Alexander Viro Cc: Miklos Szeredi, Jan Blunck, Christoph Hellwig, linux-kernel, linux-fsdevel, David Woodhouse, Valerie Aurora, linux-nfs, J. Bruce Fields, Neil Brown From: Jan Blunck <jblunck@suse.de> Userspace isn't ready for handling another file type, so silently drop whiteout directory entries before they leave the kernel. Signed-off-by: Jan Blunck <jblunck@suse.de> Signed-off-by: David Woodhouse <dwmw2@infradead.org> Signed-off-by: Valerie Aurora <vaurora@redhat.com> Cc: linux-nfs@vger.kernel.org Cc: "J. Bruce Fields" <bfields@fieldses.org> Cc: Neil Brown <neilb@suse.de> --- fs/compat.c | 9 +++++++++ fs/nfsd/nfs3xdr.c | 5 +++++ fs/nfsd/nfs4xdr.c | 5 +++++ fs/nfsd/nfsxdr.c | 4 ++++ fs/readdir.c | 9 +++++++++ 5 files changed, 32 insertions(+), 0 deletions(-) diff --git a/fs/compat.c b/fs/compat.c index 6490d21..7e7b3a4 100644 --- a/fs/compat.c +++ b/fs/compat.c @@ -912,6 +912,9 @@ static int compat_fillonedir(void *__buf, const char *name, int namlen, struct compat_old_linux_dirent __user *dirent; compat_ulong_t d_ino; + if (d_type == DT_WHT) + return 0; + if (buf->result) return -EINVAL; d_ino = ino; @@ -983,6 +986,9 @@ static int compat_filldir(void *__buf, const char *name, int namlen, compat_ulong_t d_ino; int reclen = ALIGN(NAME_OFFSET(dirent) + namlen + 2, sizeof(compat_long_t)); + if (d_type == DT_WHT) + return 0; + buf->error = -EINVAL; /* only used if we fail.. */ if (reclen > buf->count) return -EINVAL; @@ -1072,6 +1078,9 @@ static int compat_filldir64(void * __buf, const char * name, int namlen, loff_t int reclen = ALIGN(jj + namlen + 1, sizeof(u64)); u64 off; + if (d_type == DT_WHT) + return 0; + buf->error = -EINVAL; /* only used if we fail.. */ if (reclen > buf->count) return -EINVAL; diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c index 2a533a0..9b96f5a 100644 --- a/fs/nfsd/nfs3xdr.c +++ b/fs/nfsd/nfs3xdr.c @@ -885,6 +885,11 @@ encode_entry(struct readdir_cd *ccd, const char *name, int namlen, int elen; /* estimated entry length in words */ int num_entry_words = 0; /* actual number of words */ + if (d_type == DT_WHT) { + cd->common.err = nfs_ok; + return 0; + } + if (cd->offset) { u64 offset64 = offset; diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c index ac17a70..fb67254 100644 --- a/fs/nfsd/nfs4xdr.c +++ b/fs/nfsd/nfs4xdr.c @@ -2279,6 +2279,11 @@ nfsd4_encode_dirent(void *ccdv, const char *name, int namlen, return 0; } + if (d_type == DT_WHT) { + cd->common.err = nfs_ok; + return 0; + } + if (cd->offset) xdr_encode_hyper(cd->offset, (u64) offset); diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c index 4ce005d..0e57d4b 100644 --- a/fs/nfsd/nfsxdr.c +++ b/fs/nfsd/nfsxdr.c @@ -503,6 +503,10 @@ nfssvc_encode_entry(void *ccdv, const char *name, namlen, name, offset, ino); */ + if (d_type == DT_WHT) { + cd->common.err = nfs_ok; + return 0; + } if (offset > ~((u32) 0)) { cd->common.err = nfserr_fbig; return -EINVAL; diff --git a/fs/readdir.c b/fs/readdir.c index 7723401..3a48491 100644 --- a/fs/readdir.c +++ b/fs/readdir.c @@ -77,6 +77,9 @@ static int fillonedir(void * __buf, const char * name, int namlen, loff_t offset struct old_linux_dirent __user * dirent; unsigned long d_ino; + if (d_type == DT_WHT) + return 0; + if (buf->result) return -EINVAL; d_ino = ino; @@ -154,6 +157,9 @@ static int filldir(void * __buf, const char * name, int namlen, loff_t offset, unsigned long d_ino; int reclen = ALIGN(NAME_OFFSET(dirent) + namlen + 2, sizeof(long)); + if (d_type == DT_WHT) + return 0; + buf->error = -EINVAL; /* only used if we fail.. */ if (reclen > buf->count) return -EINVAL; @@ -239,6 +245,9 @@ static int filldir64(void * __buf, const char * name, int namlen, loff_t offset, struct getdents_callback64 * buf = (struct getdents_callback64 *) __buf; int reclen = ALIGN(NAME_OFFSET(dirent) + namlen + 1, sizeof(u64)); + if (d_type == DT_WHT) + return 0; + buf->error = -EINVAL; /* only used if we fail.. */ if (reclen > buf->count) return -EINVAL; -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-08-08 15:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1272928358-20854-1-git-send-email-vaurora@redhat.com>
2010-05-03 23:12 ` [PATCH 05/39] whiteout/NFSD: Don't return information about whiteouts to userspace Valerie Aurora
2010-05-03 23:37 ` Neil Brown
2010-05-06 18:01 ` Valerie Aurora
2010-05-06 21:18 ` Neil Brown
2010-05-17 19:51 ` Valerie Aurora
[not found] <1281282776-5447-1-git-send-email-vaurora@redhat.com>
2010-08-08 15:52 ` Valerie Aurora
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).