* [PATCH] Fix bug in FUSE where the attribute cache for a file was not cleared when a file is opened with O_TRUNC. @ 2010-10-29 23:41 Ken Sumrall 2010-10-30 0:02 ` Andrew Morton 0 siblings, 1 reply; 7+ messages in thread From: Ken Sumrall @ 2010-10-29 23:41 UTC (permalink / raw) To: linux-kernel Cc: Ken Sumrall, Miklos Szeredi, Andrew Morton, Jens Axboe, Anfei, Anand V. Avati, fuse-devel Signed-off-by: Ken Sumrall <ksumrall@android.com> --- fs/fuse/file.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index c822458..4fbe62c 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -134,6 +134,7 @@ EXPORT_SYMBOL_GPL(fuse_do_open); void fuse_finish_open(struct inode *inode, struct file *file) { struct fuse_file *ff = file->private_data; + struct fuse_conn *fc = get_fuse_conn(inode); if (ff->open_flags & FOPEN_DIRECT_IO) file->f_op = &fuse_direct_io_file_operations; @@ -141,6 +142,8 @@ void fuse_finish_open(struct inode *inode, struct file *file) invalidate_inode_pages2(inode->i_mapping); if (ff->open_flags & FOPEN_NONSEEKABLE) nonseekable_open(inode, file); + if (fc->atomic_o_trunc && (file->f_flags & O_TRUNC)) + fuse_invalidate_attr(inode); } int fuse_open_common(struct inode *inode, struct file *file, bool isdir) -- 1.7.3.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix bug in FUSE where the attribute cache for a file was not cleared when a file is opened with O_TRUNC. 2010-10-29 23:41 [PATCH] Fix bug in FUSE where the attribute cache for a file was not cleared when a file is opened with O_TRUNC Ken Sumrall @ 2010-10-30 0:02 ` Andrew Morton [not found] ` <AANLkTimJWX5WnG6oiUfBxiD=BBJ=Q5iRc73iar=gPKCy@mail.gmail.com> 0 siblings, 1 reply; 7+ messages in thread From: Andrew Morton @ 2010-10-30 0:02 UTC (permalink / raw) To: Ken Sumrall Cc: linux-kernel, Miklos Szeredi, Jens Axboe, Anfei, Anand V. Avati, fuse-devel On Fri, 29 Oct 2010 16:41:40 -0700 Ken Sumrall <ksumrall@android.com> wrote: > Signed-off-by: Ken Sumrall <ksumrall@android.com> > --- > fs/fuse/file.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c > index c822458..4fbe62c 100644 > --- a/fs/fuse/file.c > +++ b/fs/fuse/file.c > @@ -134,6 +134,7 @@ EXPORT_SYMBOL_GPL(fuse_do_open); > void fuse_finish_open(struct inode *inode, struct file *file) > { > struct fuse_file *ff = file->private_data; > + struct fuse_conn *fc = get_fuse_conn(inode); > > if (ff->open_flags & FOPEN_DIRECT_IO) > file->f_op = &fuse_direct_io_file_operations; > @@ -141,6 +142,8 @@ void fuse_finish_open(struct inode *inode, struct file *file) > invalidate_inode_pages2(inode->i_mapping); > if (ff->open_flags & FOPEN_NONSEEKABLE) > nonseekable_open(inode, file); > + if (fc->atomic_o_trunc && (file->f_flags & O_TRUNC)) > + fuse_invalidate_attr(inode); > } > > int fuse_open_common(struct inode *inode, struct file *file, bool isdir) What were the user-visible effects of this bug? ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <AANLkTimJWX5WnG6oiUfBxiD=BBJ=Q5iRc73iar=gPKCy@mail.gmail.com>]
* Fwd: [PATCH] Fix bug in FUSE where the attribute cache for a file was not cleared when a file is opened with O_TRUNC. [not found] ` <AANLkTimJWX5WnG6oiUfBxiD=BBJ=Q5iRc73iar=gPKCy@mail.gmail.com> @ 2010-10-30 0:37 ` Ken Sumrall 2010-10-30 11:31 ` Miklos Szeredi 1 sibling, 0 replies; 7+ messages in thread From: Ken Sumrall @ 2010-10-30 0:37 UTC (permalink / raw) To: linux-kernel Resending to the list, as it was rejected due to html content. ---------- Forwarded message ---------- From: Ken Sumrall <ksumrall@android.com> Date: Fri, Oct 29, 2010 at 5:31 PM Subject: Re: [PATCH] Fix bug in FUSE where the attribute cache for a file was not cleared when a file is opened with O_TRUNC. To: Andrew Morton <akpm@linux-foundation.org> Cc: linux-kernel@vger.kernel.org, Miklos Szeredi <miklos@szeredi.hu>, Jens Axboe <jens.axboe@oracle.com>, Anfei <anfei.zhou@gmail.com>, "Anand V. Avati" <avati@gluster.com>, fuse-devel@lists.sourceforge.net Here is a snippet of code from a test case we have: { FILE* pFile = NULL; int32_t data = 0x01020304; LOGE("WRITETEST start"); pFile = fopen("/sdcard/write_test", "wb"); if( NULL != pFile ) { LOGE("WRITETEST : starting position %d", ftell(pFile)); fseek(pFile, 0, SEEK_END); LOGE("WRITETEST : starting size %d", ftell(pFile)); fwrite(&data, sizeof(int32_t), 1, pFile); fclose(pFile); } else { LOGE("WRITETEST : failed to open the file"); } LOGE("WRITETEST end"); } fopen("wb") turns into open(O_TRUNC) in bionic (the Android libc). The code is odd in that it then does fseek(SEEK_END) on a newly truncated file. However, that is completely legal, and should set the file pointer to offset 0. If the file had previously existed, it's attributes are still in the FUSE attribute cache, so the fseek(SEEK_END) positions the file pointer to the end size of the file before it was truncated. This has probably never been seen before because most code that opens with O_TRUNC just starts writing, and doesn't query the attributes of the file till after a write. Writing clears the attribute cache for the file so it is fetched from the filesystem the next time it is requested. ___ Ken On Fri, Oct 29, 2010 at 5:02 PM, Andrew Morton <akpm@linux-foundation.org> wrote: > > On Fri, 29 Oct 2010 16:41:40 -0700 Ken Sumrall <ksumrall@android.com> wrote: > > > Signed-off-by: Ken Sumrall <ksumrall@android.com> > > --- > > fs/fuse/file.c | 3 +++ > > 1 files changed, 3 insertions(+), 0 deletions(-) > > > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c > > index c822458..4fbe62c 100644 > > --- a/fs/fuse/file.c > > +++ b/fs/fuse/file.c > > @@ -134,6 +134,7 @@ EXPORT_SYMBOL_GPL(fuse_do_open); > > void fuse_finish_open(struct inode *inode, struct file *file) > > { > > struct fuse_file *ff = file->private_data; > > + struct fuse_conn *fc = get_fuse_conn(inode); > > > > if (ff->open_flags & FOPEN_DIRECT_IO) > > file->f_op = &fuse_direct_io_file_operations; > > @@ -141,6 +142,8 @@ void fuse_finish_open(struct inode *inode, struct file *file) > > invalidate_inode_pages2(inode->i_mapping); > > if (ff->open_flags & FOPEN_NONSEEKABLE) > > nonseekable_open(inode, file); > > + if (fc->atomic_o_trunc && (file->f_flags & O_TRUNC)) > > + fuse_invalidate_attr(inode); > > } > > > > int fuse_open_common(struct inode *inode, struct file *file, bool isdir) > > What were the user-visible effects of this bug? > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix bug in FUSE where the attribute cache for a file was not cleared when a file is opened with O_TRUNC. [not found] ` <AANLkTimJWX5WnG6oiUfBxiD=BBJ=Q5iRc73iar=gPKCy@mail.gmail.com> 2010-10-30 0:37 ` Fwd: " Ken Sumrall @ 2010-10-30 11:31 ` Miklos Szeredi 2010-11-01 23:38 ` Ken Sumrall 2010-11-11 22:10 ` Andrew Morton 1 sibling, 2 replies; 7+ messages in thread From: Miklos Szeredi @ 2010-10-30 11:31 UTC (permalink / raw) To: Ken Sumrall Cc: akpm, linux-kernel, miklos, jens.axboe, anfei.zhou, avati, fuse-devel Ken, Thanks for the patch. Here's a slightly updated one. Does this also look OK? Miklos ---- Subject: fuse: fix attributes after open(O_TRUNC) From: Ken Sumrall <ksumrall@android.com> The attribute cache for a file was not being cleared when a file is opened with O_TRUNC. If the filesystem's open operation truncates the file ("atomic_o_trunc" feature flag is set) then the kernel should invalidate the cached st_mtime and st_ctime attributes. Also i_size should be explicitly be set to zero as it is used sometimes without refreshing the cache. Signed-off-by: Ken Sumrall <ksumrall@android.com> Cc: Anfei <anfei.zhou@gmail.com> Cc: "Anand V. Avati" <avati@gluster.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Miklos Szeredi <miklos@szeredi.hu> Cc: stable@kernel.org --- fs/fuse/file.c | 10 ++++++++++ 1 file changed, 10 insertions(+) Index: linux-2.6/fs/fuse/file.c =================================================================== --- linux-2.6.orig/fs/fuse/file.c 2010-10-27 14:04:02.000000000 +0200 +++ linux-2.6/fs/fuse/file.c 2010-10-30 13:14:07.000000000 +0200 @@ -134,6 +134,7 @@ EXPORT_SYMBOL_GPL(fuse_do_open); void fuse_finish_open(struct inode *inode, struct file *file) { struct fuse_file *ff = file->private_data; + struct fuse_conn *fc = get_fuse_conn(inode); if (ff->open_flags & FOPEN_DIRECT_IO) file->f_op = &fuse_direct_io_file_operations; @@ -141,6 +142,15 @@ void fuse_finish_open(struct inode *inod invalidate_inode_pages2(inode->i_mapping); if (ff->open_flags & FOPEN_NONSEEKABLE) nonseekable_open(inode, file); + if (fc->atomic_o_trunc && (file->f_flags & O_TRUNC)) { + struct fuse_inode *fi = get_fuse_inode(inode); + + spin_lock(&fc->lock); + fi->attr_version = ++fc->attr_version; + i_size_write(inode, 0); + spin_unlock(&fc->lock); + fuse_invalidate_attr(inode); + } } int fuse_open_common(struct inode *inode, struct file *file, bool isdir) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix bug in FUSE where the attribute cache for a file was not cleared when a file is opened with O_TRUNC. 2010-10-30 11:31 ` Miklos Szeredi @ 2010-11-01 23:38 ` Ken Sumrall 2010-11-11 22:10 ` Andrew Morton 1 sibling, 0 replies; 7+ messages in thread From: Ken Sumrall @ 2010-11-01 23:38 UTC (permalink / raw) To: Miklos Szeredi Cc: akpm, linux-kernel, jens.axboe, anfei.zhou, avati, fuse-devel I'm no FUSE expert, so I don't feel qualified to fully evaluate the changes, but the locking looks correct and non-blocking, and the code still passes our test case and doesn't crash, so it gets my vote of approval. ___ Ken On Sat, Oct 30, 2010 at 4:31 AM, Miklos Szeredi <miklos@szeredi.hu> wrote: > Ken, > > Thanks for the patch. > > Here's a slightly updated one. Does this also look OK? > > Miklos > > > ---- > Subject: fuse: fix attributes after open(O_TRUNC) > From: Ken Sumrall <ksumrall@android.com> > > The attribute cache for a file was not being cleared when a file is > opened with O_TRUNC. > > If the filesystem's open operation truncates the file > ("atomic_o_trunc" feature flag is set) then the kernel should > invalidate the cached st_mtime and st_ctime attributes. > > Also i_size should be explicitly be set to zero as it is used > sometimes without refreshing the cache. > > Signed-off-by: Ken Sumrall <ksumrall@android.com> > Cc: Anfei <anfei.zhou@gmail.com> > Cc: "Anand V. Avati" <avati@gluster.com> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > Signed-off-by: Miklos Szeredi <miklos@szeredi.hu> > Cc: stable@kernel.org > --- > > fs/fuse/file.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > Index: linux-2.6/fs/fuse/file.c > =================================================================== > --- linux-2.6.orig/fs/fuse/file.c 2010-10-27 14:04:02.000000000 +0200 > +++ linux-2.6/fs/fuse/file.c 2010-10-30 13:14:07.000000000 +0200 > @@ -134,6 +134,7 @@ EXPORT_SYMBOL_GPL(fuse_do_open); > void fuse_finish_open(struct inode *inode, struct file *file) > { > struct fuse_file *ff = file->private_data; > + struct fuse_conn *fc = get_fuse_conn(inode); > > if (ff->open_flags & FOPEN_DIRECT_IO) > file->f_op = &fuse_direct_io_file_operations; > @@ -141,6 +142,15 @@ void fuse_finish_open(struct inode *inod > invalidate_inode_pages2(inode->i_mapping); > if (ff->open_flags & FOPEN_NONSEEKABLE) > nonseekable_open(inode, file); > + if (fc->atomic_o_trunc && (file->f_flags & O_TRUNC)) { > + struct fuse_inode *fi = get_fuse_inode(inode); > + > + spin_lock(&fc->lock); > + fi->attr_version = ++fc->attr_version; > + i_size_write(inode, 0); > + spin_unlock(&fc->lock); > + fuse_invalidate_attr(inode); > + } > } > > int fuse_open_common(struct inode *inode, struct file *file, bool isdir) > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix bug in FUSE where the attribute cache for a file was not cleared when a file is opened with O_TRUNC. 2010-10-30 11:31 ` Miklos Szeredi 2010-11-01 23:38 ` Ken Sumrall @ 2010-11-11 22:10 ` Andrew Morton 2010-11-12 9:37 ` Miklos Szeredi 1 sibling, 1 reply; 7+ messages in thread From: Andrew Morton @ 2010-11-11 22:10 UTC (permalink / raw) To: Miklos Szeredi Cc: Ken Sumrall, linux-kernel, jens.axboe, anfei.zhou, avati, fuse-devel On Sat, 30 Oct 2010 13:31:23 +0200 Miklos Szeredi <miklos@szeredi.hu> wrote: > Subject: fuse: fix attributes after open(O_TRUNC) > From: Ken Sumrall <ksumrall@android.com> > > The attribute cache for a file was not being cleared when a file is > opened with O_TRUNC. > > If the filesystem's open operation truncates the file > ("atomic_o_trunc" feature flag is set) then the kernel should > invalidate the cached st_mtime and st_ctime attributes. > > Also i_size should be explicitly be set to zero as it is used > sometimes without refreshing the cache. > > Signed-off-by: Ken Sumrall <ksumrall@android.com> > Cc: Anfei <anfei.zhou@gmail.com> > Cc: "Anand V. Avati" <avati@gluster.com> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > Signed-off-by: Miklos Szeredi <miklos@szeredi.hu> > Cc: stable@kernel.org > --- > > fs/fuse/file.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > Index: linux-2.6/fs/fuse/file.c > =================================================================== > --- linux-2.6.orig/fs/fuse/file.c 2010-10-27 14:04:02.000000000 +0200 > +++ linux-2.6/fs/fuse/file.c 2010-10-30 13:14:07.000000000 +0200 > @@ -134,6 +134,7 @@ EXPORT_SYMBOL_GPL(fuse_do_open); > void fuse_finish_open(struct inode *inode, struct file *file) > { > struct fuse_file *ff = file->private_data; > + struct fuse_conn *fc = get_fuse_conn(inode); > > if (ff->open_flags & FOPEN_DIRECT_IO) > file->f_op = &fuse_direct_io_file_operations; > @@ -141,6 +142,15 @@ void fuse_finish_open(struct inode *inod > invalidate_inode_pages2(inode->i_mapping); > if (ff->open_flags & FOPEN_NONSEEKABLE) > nonseekable_open(inode, file); > + if (fc->atomic_o_trunc && (file->f_flags & O_TRUNC)) { > + struct fuse_inode *fi = get_fuse_inode(inode); > + > + spin_lock(&fc->lock); > + fi->attr_version = ++fc->attr_version; > + i_size_write(inode, 0); i_size_write() requires that i_mutex be held, as described at the i_size_write() definition site. Is it reliably the case that i_mutex is held here? > + spin_unlock(&fc->lock); > + fuse_invalidate_attr(inode); > + } > } > > int fuse_open_common(struct inode *inode, struct file *file, bool isdir) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix bug in FUSE where the attribute cache for a file was not cleared when a file is opened with O_TRUNC. 2010-11-11 22:10 ` Andrew Morton @ 2010-11-12 9:37 ` Miklos Szeredi 0 siblings, 0 replies; 7+ messages in thread From: Miklos Szeredi @ 2010-11-12 9:37 UTC (permalink / raw) To: Andrew Morton Cc: miklos, ksumrall, linux-kernel, jens.axboe, anfei.zhou, avati, fuse-devel On Thu, 11 Nov 2010, Andrew Morton wrote: > On Sat, 30 Oct 2010 13:31:23 +0200 > Miklos Szeredi <miklos@szeredi.hu> wrote: > > > Subject: fuse: fix attributes after open(O_TRUNC) > > From: Ken Sumrall <ksumrall@android.com> > > > > The attribute cache for a file was not being cleared when a file is > > opened with O_TRUNC. > > > > If the filesystem's open operation truncates the file > > ("atomic_o_trunc" feature flag is set) then the kernel should > > invalidate the cached st_mtime and st_ctime attributes. > > > > Also i_size should be explicitly be set to zero as it is used > > sometimes without refreshing the cache. > > > > Signed-off-by: Ken Sumrall <ksumrall@android.com> > > Cc: Anfei <anfei.zhou@gmail.com> > > Cc: "Anand V. Avati" <avati@gluster.com> > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > > Signed-off-by: Miklos Szeredi <miklos@szeredi.hu> > > Cc: stable@kernel.org > > --- > > > > fs/fuse/file.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > Index: linux-2.6/fs/fuse/file.c > > =================================================================== > > --- linux-2.6.orig/fs/fuse/file.c 2010-10-27 14:04:02.000000000 +0200 > > +++ linux-2.6/fs/fuse/file.c 2010-10-30 13:14:07.000000000 +0200 > > @@ -134,6 +134,7 @@ EXPORT_SYMBOL_GPL(fuse_do_open); > > void fuse_finish_open(struct inode *inode, struct file *file) > > { > > struct fuse_file *ff = file->private_data; > > + struct fuse_conn *fc = get_fuse_conn(inode); > > > > if (ff->open_flags & FOPEN_DIRECT_IO) > > file->f_op = &fuse_direct_io_file_operations; > > @@ -141,6 +142,15 @@ void fuse_finish_open(struct inode *inod > > invalidate_inode_pages2(inode->i_mapping); > > if (ff->open_flags & FOPEN_NONSEEKABLE) > > nonseekable_open(inode, file); > > + if (fc->atomic_o_trunc && (file->f_flags & O_TRUNC)) { > > + struct fuse_inode *fi = get_fuse_inode(inode); > > + > > + spin_lock(&fc->lock); > > + fi->attr_version = ++fc->attr_version; > > + i_size_write(inode, 0); > > i_size_write() requires that i_mutex be held, as described at the > i_size_write() definition site. > > Is it reliably the case that i_mutex is held here? No. AFAICS any lock will do, not just i_mutex. Fuse consistently uses fc->lock to protect i_size_write(). Thanks, Miklos ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-11-12 9:37 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-10-29 23:41 [PATCH] Fix bug in FUSE where the attribute cache for a file was not cleared when a file is opened with O_TRUNC Ken Sumrall 2010-10-30 0:02 ` Andrew Morton [not found] ` <AANLkTimJWX5WnG6oiUfBxiD=BBJ=Q5iRc73iar=gPKCy@mail.gmail.com> 2010-10-30 0:37 ` Fwd: " Ken Sumrall 2010-10-30 11:31 ` Miklos Szeredi 2010-11-01 23:38 ` Ken Sumrall 2010-11-11 22:10 ` Andrew Morton 2010-11-12 9:37 ` Miklos Szeredi
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.