Messages in this thread Patch in this message | | | Date | Thu, 23 Oct 2008 10:55:32 -0700 | From | "Manish Lachwani" <> | Subject | Re: oops in file_storage.c |
| |
Hi Alan,
Yes, I am trying out the changes below. Will let you know how it goes. If it works well, I will send an updated patch.
diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c index 3e807f0..844f992 100644 --- a/drivers/usb/gadget/file_storage.c +++ b/drivers/usb/gadget/file_storage.c @@ -1927,26 +1927,11 @@ static int do_write(struct fsg_dev *fsg) static int fsync_sub(struct lun *curlun) { struct file *filp = curlun->filp; - struct inode *inode; - int rc, err;
if (curlun->ro || !filp) return 0; - if (!filp->f_op || !filp->f_op->fsync) - return -EINVAL;
- inode = filp->f_path.dentry->d_inode; - mutex_lock(&inode->i_mutex); - rc = filemap_fdatawrite(inode->i_mapping); - err = filp->f_op->fsync(filp, filp->f_path.dentry, 1); - if (!rc) - rc = err; - err = filemap_fdatawait(inode->i_mapping); - if (!rc) - rc = err; - mutex_unlock(&inode->i_mutex); - VLDBG(curlun, "fdatasync -> %d\n", rc); - return rc; + return do_fsync(filp, 0); }
static void fsync_all(struct fsg_dev *fsg) diff --git a/fs/sync.c b/fs/sync.c index e700eb1..53ff1f3 100644 --- a/fs/sync.c +++ b/fs/sync.c @@ -111,6 +111,7 @@ long do_fsync(struct file *file, int datasync) out: return ret; } +EXPORT_SYMBOL(do_fsync);
static long __do_fsync(unsigned int fd, int datasync) {
On Thu, Oct 23, 2008 at 9:46 AM, Alan Stern <stern@rowland.harvard.edu> wrote: > 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 > >
| |