lkml.org 
[lkml]   [2008]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: oops in file_storage.c
On Thu, 23 Oct 2008, Manish Lachwani wrote:

> Hi Alan,
>
> >
> > Hmm. On my system (2.6.27) the code has moved to fs/sync.c/do_fsync(),
> > and it has changed a fair amount. Maybe file_storage.c should be
> > changed to match.
>
> I have the latest git checkout of
> http://www.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> and it seems that do_fsync() is still quite similar to fsync_sub().
> Also, I checked older trees like 2.6.22 and do_fsync() is still in
> fs/sync.c

That just goes to show how old file_storage.c is.

> > In fact, the easiest approach would be to EXPORT do_fsync() and then
> > call it directly. I don't know whether people would like this, though.
> >
>
> I could try this. But I don't see much difference b/w do_fsync() and
> fsync_sub(). However in do_fsync(), filemap_fdatawrite() is called
> before the mutex_lock() and filemap_fdatawait() is called after the
> mutex_unlock().
>
> Here is do_fsync() -
>
> long do_fsync(struct file *file, int datasync)
> {
> int ret;
> int err;
> struct address_space *mapping = file->f_mapping;

file_storage does

inode = filp->f_path.dentry->d_inode;

instead, and uses inode in place of mapping->host and inode->i_mapping
in place of mapping. Should this be changed? I have no idea, beyond
feeling that the core kernel code has got to be more up-to-date. Same
goes for the ordering of the locks and the filemap_* calls.

> if (!file->f_op || !file->f_op->fsync) {
> /* Why? We can still call filemap_fdatawrite */
> ret = -EINVAL;
> goto out;
> }
>
> ret = filemap_fdatawrite(mapping);
>
> /*
> * We need to protect against concurrent writers, which could cause
> * livelocks in fsync_buffers_list().
> */
> mutex_lock(&mapping->host->i_mutex);
> err = file->f_op->fsync(file, file->f_path.dentry, datasync);
> if (!ret)
> ret = err;
> mutex_unlock(&mapping->host->i_mutex);
> err = filemap_fdatawait(mapping);
> if (!ret)
> ret = err;
> out:
> return ret;
> }

If the EXPORT route is acceptable, I'd prefer to use it.

Alan Stern



\
 
 \ /
  Last update: 2008-10-23 18:49    [W:0.037 / U:40.096 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site