Re: [PATCH 5/6] fuse: truncate file if async dio failed
On Tue, Dec 18, 2012 at 9:12 AM, Maxim V. Patlasov wrote: > I did exactly the same before sending previous email :) In my tests it works > as expected too (modulo fuse_allow_task() that we can move up). I'll re-send > corrected patch soon. This patch was not yet re-sent. I applied the rest of them in this series (with small changes) and pushed to for-next. Thanks, Miklos -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/6] fuse: truncate file if async dio failed
On Tue, Dec 18, 2012 at 9:12 AM, Maxim V. Patlasov mpatla...@parallels.com wrote: I did exactly the same before sending previous email :) In my tests it works as expected too (modulo fuse_allow_task() that we can move up). I'll re-send corrected patch soon. This patch was not yet re-sent. I applied the rest of them in this series (with small changes) and pushed to for-next. Thanks, Miklos -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/6] fuse: truncate file if async dio failed
12/17/2012 11:04 PM, Brian Foster пишет: On 12/17/2012 09:13 AM, Maxim V. Patlasov wrote: Hi, 12/15/2012 12:16 AM, Brian Foster пишет: On 12/14/2012 10:21 AM, Maxim V. Patlasov wrote: ... + fuse_do_truncate() looks fairly close to fuse_do_setattr(). Is there any reason we couldn't make fuse_do_setattr() non-static, change the dentry parameter to an inode and use that? fuse_do_setattr() performs extra checks that fuse_do_truncate() needn't. Some of them are harmless, some not: fuse_allow_task() may return 0 if task credentials changed. E.g. super-user successfully opened a file, then setuid(other_user_uid), then write(2) to the file. write(2) doesn't check uid, but fuse_do_truncate() - via fuse_allow_task() - does. Conversely, what about the extra error handling bits in fuse_do_setattr() that do not appear in fuse_do_truncate() (i.e., the inode mode check, the change attributes call, updating the inode size, etc.)? It seems like we would want some of that code here. Yes, they won't harm. fuse_setattr() is the only caller of fuse_do_setattr(), so why not embed some of the initial checks (such as fuse_allow_task()) there? I suppose we could pull out some of the error handling checks there as well if they are considered harmful to this post-write error truncate situation. Makes sense. I like it especially because it allows to avoid code duplication (handling FUSE_SETATTR fuse-request). FWIW, I just tested a quick change that pulls up the fuse_allow_task() check (via instrumenting a write error) and it seems to work as expected. I can forward a patch if interested... I did exactly the same before sending previous email :) In my tests it works as expected too (modulo fuse_allow_task() that we can move up). I'll re-send corrected patch soon. Thanks, Maxim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/6] fuse: truncate file if async dio failed
12/17/2012 11:04 PM, Brian Foster пишет: On 12/17/2012 09:13 AM, Maxim V. Patlasov wrote: Hi, 12/15/2012 12:16 AM, Brian Foster пишет: On 12/14/2012 10:21 AM, Maxim V. Patlasov wrote: ... + fuse_do_truncate() looks fairly close to fuse_do_setattr(). Is there any reason we couldn't make fuse_do_setattr() non-static, change the dentry parameter to an inode and use that? fuse_do_setattr() performs extra checks that fuse_do_truncate() needn't. Some of them are harmless, some not: fuse_allow_task() may return 0 if task credentials changed. E.g. super-user successfully opened a file, then setuid(other_user_uid), then write(2) to the file. write(2) doesn't check uid, but fuse_do_truncate() - via fuse_allow_task() - does. Conversely, what about the extra error handling bits in fuse_do_setattr() that do not appear in fuse_do_truncate() (i.e., the inode mode check, the change attributes call, updating the inode size, etc.)? It seems like we would want some of that code here. Yes, they won't harm. fuse_setattr() is the only caller of fuse_do_setattr(), so why not embed some of the initial checks (such as fuse_allow_task()) there? I suppose we could pull out some of the error handling checks there as well if they are considered harmful to this post-write error truncate situation. Makes sense. I like it especially because it allows to avoid code duplication (handling FUSE_SETATTR fuse-request). FWIW, I just tested a quick change that pulls up the fuse_allow_task() check (via instrumenting a write error) and it seems to work as expected. I can forward a patch if interested... I did exactly the same before sending previous email :) In my tests it works as expected too (modulo fuse_allow_task() that we can move up). I'll re-send corrected patch soon. Thanks, Maxim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/6] fuse: truncate file if async dio failed
On 12/17/2012 09:13 AM, Maxim V. Patlasov wrote: > Hi, > > 12/15/2012 12:16 AM, Brian Foster пишет: >> On 12/14/2012 10:21 AM, Maxim V. Patlasov wrote: ... >>> + >> fuse_do_truncate() looks fairly close to fuse_do_setattr(). Is there any >> reason we couldn't make fuse_do_setattr() non-static, change the dentry >> parameter to an inode and use that? > > fuse_do_setattr() performs extra checks that fuse_do_truncate() needn't. > Some of them are harmless, some not: fuse_allow_task() may return 0 if > task credentials changed. E.g. super-user successfully opened a file, > then setuid(other_user_uid), then write(2) to the file. write(2) doesn't > check uid, but fuse_do_truncate() - via fuse_allow_task() - does. > Conversely, what about the extra error handling bits in fuse_do_setattr() that do not appear in fuse_do_truncate() (i.e., the inode mode check, the change attributes call, updating the inode size, etc.)? It seems like we would want some of that code here. fuse_setattr() is the only caller of fuse_do_setattr(), so why not embed some of the initial checks (such as fuse_allow_task()) there? I suppose we could pull out some of the error handling checks there as well if they are considered harmful to this post-write error truncate situation. FWIW, I just tested a quick change that pulls up the fuse_allow_task() check (via instrumenting a write error) and it seems to work as expected. I can forward a patch if interested... Brian > This non-POSIX behaviour (ftruncate(2) returning -1 with errno==EACCES) > was introduced long time ago: > >> commit e57ac68378a287d6336d187b26971f35f7ee7251 >> Author: Miklos Szeredi >> Date: Thu Oct 18 03:06:58 2007 -0700 >> >> fuse: fix allowing operations >> >> The following operation didn't check if sending the request was >> allowed: >> >> setattr >> listxattr >> statfs >> >> Some other operations don't explicitly do the check, but VFS calls >> ->permission() which checks this. >> >> Signed-off-by: Miklos Szeredi >> Signed-off-by: Andrew Morton >> Signed-off-by: Linus Torvalds > > and I'm not sure whether it was done intentionally or not. Maybe Miklos > could shed some light on it... > > Thanks, > Maxim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/6] fuse: truncate file if async dio failed
Hi, 12/15/2012 12:16 AM, Brian Foster пишет: On 12/14/2012 10:21 AM, Maxim V. Patlasov wrote: The patch improves error handling in fuse_direct_IO(): if we successfully submitted several fuse requests on behalf of synchronous direct write extending file and some of them failed, let's try to do our best to clean-up. Signed-off-by: Maxim Patlasov --- fs/fuse/file.c | 55 +-- 1 files changed, 53 insertions(+), 2 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index 05eed23..b6e9b8d 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -2340,6 +2340,53 @@ int fuse_notify_poll_wakeup(struct fuse_conn *fc, return 0; } +static void fuse_do_truncate(struct file *file) +{ + struct fuse_file *ff = file->private_data; + struct inode *inode = file->f_mapping->host; + struct fuse_conn *fc = get_fuse_conn(inode); + struct fuse_req *req; + struct fuse_setattr_in inarg; + struct fuse_attr_out outarg; + int err; + + req = fuse_get_req_nopages(fc); + if (IS_ERR(req)) { + printk(KERN_WARNING "failed to allocate req for truncate " + "(%ld)\n", PTR_ERR(req)); + return; + } + + memset(, 0, sizeof(inarg)); + memset(, 0, sizeof(outarg)); + + inarg.valid |= FATTR_SIZE; + inarg.size = i_size_read(inode); + + inarg.valid |= FATTR_FH; + inarg.fh = ff->fh; + + req->in.h.opcode = FUSE_SETATTR; + req->in.h.nodeid = get_node_id(inode); + req->in.numargs = 1; + req->in.args[0].size = sizeof(inarg); + req->in.args[0].value = + req->out.numargs = 1; + if (fc->minor < 9) + req->out.args[0].size = FUSE_COMPAT_ATTR_OUT_SIZE; + else + req->out.args[0].size = sizeof(outarg); + req->out.args[0].value = + + fuse_request_send(fc, req); + err = req->out.h.error; + fuse_put_request(fc, req); + + if (err) + printk(KERN_WARNING "failed to truncate to %lld with error " + "%d\n", i_size_read(inode), err); +} + fuse_do_truncate() looks fairly close to fuse_do_setattr(). Is there any reason we couldn't make fuse_do_setattr() non-static, change the dentry parameter to an inode and use that? fuse_do_setattr() performs extra checks that fuse_do_truncate() needn't. Some of them are harmless, some not: fuse_allow_task() may return 0 if task credentials changed. E.g. super-user successfully opened a file, then setuid(other_user_uid), then write(2) to the file. write(2) doesn't check uid, but fuse_do_truncate() - via fuse_allow_task() - does. This non-POSIX behaviour (ftruncate(2) returning -1 with errno==EACCES) was introduced long time ago: commit e57ac68378a287d6336d187b26971f35f7ee7251 Author: Miklos Szeredi Date: Thu Oct 18 03:06:58 2007 -0700 fuse: fix allowing operations The following operation didn't check if sending the request was allowed: setattr listxattr statfs Some other operations don't explicitly do the check, but VFS calls ->permission() which checks this. Signed-off-by: Miklos Szeredi Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds and I'm not sure whether it was done intentionally or not. Maybe Miklos could shed some light on it... Thanks, Maxim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/6] fuse: truncate file if async dio failed
On 12/17/2012 09:13 AM, Maxim V. Patlasov wrote: Hi, 12/15/2012 12:16 AM, Brian Foster пишет: On 12/14/2012 10:21 AM, Maxim V. Patlasov wrote: ... + fuse_do_truncate() looks fairly close to fuse_do_setattr(). Is there any reason we couldn't make fuse_do_setattr() non-static, change the dentry parameter to an inode and use that? fuse_do_setattr() performs extra checks that fuse_do_truncate() needn't. Some of them are harmless, some not: fuse_allow_task() may return 0 if task credentials changed. E.g. super-user successfully opened a file, then setuid(other_user_uid), then write(2) to the file. write(2) doesn't check uid, but fuse_do_truncate() - via fuse_allow_task() - does. Conversely, what about the extra error handling bits in fuse_do_setattr() that do not appear in fuse_do_truncate() (i.e., the inode mode check, the change attributes call, updating the inode size, etc.)? It seems like we would want some of that code here. fuse_setattr() is the only caller of fuse_do_setattr(), so why not embed some of the initial checks (such as fuse_allow_task()) there? I suppose we could pull out some of the error handling checks there as well if they are considered harmful to this post-write error truncate situation. FWIW, I just tested a quick change that pulls up the fuse_allow_task() check (via instrumenting a write error) and it seems to work as expected. I can forward a patch if interested... Brian This non-POSIX behaviour (ftruncate(2) returning -1 with errno==EACCES) was introduced long time ago: commit e57ac68378a287d6336d187b26971f35f7ee7251 Author: Miklos Szeredi mszer...@suse.cz Date: Thu Oct 18 03:06:58 2007 -0700 fuse: fix allowing operations The following operation didn't check if sending the request was allowed: setattr listxattr statfs Some other operations don't explicitly do the check, but VFS calls -permission() which checks this. Signed-off-by: Miklos Szeredi mszer...@suse.cz Signed-off-by: Andrew Morton a...@linux-foundation.org Signed-off-by: Linus Torvalds torva...@linux-foundation.org and I'm not sure whether it was done intentionally or not. Maybe Miklos could shed some light on it... Thanks, Maxim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/6] fuse: truncate file if async dio failed
Hi, 12/15/2012 12:16 AM, Brian Foster пишет: On 12/14/2012 10:21 AM, Maxim V. Patlasov wrote: The patch improves error handling in fuse_direct_IO(): if we successfully submitted several fuse requests on behalf of synchronous direct write extending file and some of them failed, let's try to do our best to clean-up. Signed-off-by: Maxim Patlasov mpatla...@parallels.com --- fs/fuse/file.c | 55 +-- 1 files changed, 53 insertions(+), 2 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index 05eed23..b6e9b8d 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -2340,6 +2340,53 @@ int fuse_notify_poll_wakeup(struct fuse_conn *fc, return 0; } +static void fuse_do_truncate(struct file *file) +{ + struct fuse_file *ff = file-private_data; + struct inode *inode = file-f_mapping-host; + struct fuse_conn *fc = get_fuse_conn(inode); + struct fuse_req *req; + struct fuse_setattr_in inarg; + struct fuse_attr_out outarg; + int err; + + req = fuse_get_req_nopages(fc); + if (IS_ERR(req)) { + printk(KERN_WARNING failed to allocate req for truncate + (%ld)\n, PTR_ERR(req)); + return; + } + + memset(inarg, 0, sizeof(inarg)); + memset(outarg, 0, sizeof(outarg)); + + inarg.valid |= FATTR_SIZE; + inarg.size = i_size_read(inode); + + inarg.valid |= FATTR_FH; + inarg.fh = ff-fh; + + req-in.h.opcode = FUSE_SETATTR; + req-in.h.nodeid = get_node_id(inode); + req-in.numargs = 1; + req-in.args[0].size = sizeof(inarg); + req-in.args[0].value = inarg; + req-out.numargs = 1; + if (fc-minor 9) + req-out.args[0].size = FUSE_COMPAT_ATTR_OUT_SIZE; + else + req-out.args[0].size = sizeof(outarg); + req-out.args[0].value = outarg; + + fuse_request_send(fc, req); + err = req-out.h.error; + fuse_put_request(fc, req); + + if (err) + printk(KERN_WARNING failed to truncate to %lld with error + %d\n, i_size_read(inode), err); +} + fuse_do_truncate() looks fairly close to fuse_do_setattr(). Is there any reason we couldn't make fuse_do_setattr() non-static, change the dentry parameter to an inode and use that? fuse_do_setattr() performs extra checks that fuse_do_truncate() needn't. Some of them are harmless, some not: fuse_allow_task() may return 0 if task credentials changed. E.g. super-user successfully opened a file, then setuid(other_user_uid), then write(2) to the file. write(2) doesn't check uid, but fuse_do_truncate() - via fuse_allow_task() - does. This non-POSIX behaviour (ftruncate(2) returning -1 with errno==EACCES) was introduced long time ago: commit e57ac68378a287d6336d187b26971f35f7ee7251 Author: Miklos Szeredi mszer...@suse.cz Date: Thu Oct 18 03:06:58 2007 -0700 fuse: fix allowing operations The following operation didn't check if sending the request was allowed: setattr listxattr statfs Some other operations don't explicitly do the check, but VFS calls -permission() which checks this. Signed-off-by: Miklos Szeredi mszer...@suse.cz Signed-off-by: Andrew Morton a...@linux-foundation.org Signed-off-by: Linus Torvalds torva...@linux-foundation.org and I'm not sure whether it was done intentionally or not. Maybe Miklos could shed some light on it... Thanks, Maxim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/6] fuse: truncate file if async dio failed
On 12/14/2012 10:21 AM, Maxim V. Patlasov wrote: > The patch improves error handling in fuse_direct_IO(): if we successfully > submitted several fuse requests on behalf of synchronous direct write > extending file and some of them failed, let's try to do our best to clean-up. > > Signed-off-by: Maxim Patlasov > --- > fs/fuse/file.c | 55 +-- > 1 files changed, 53 insertions(+), 2 deletions(-) > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c > index 05eed23..b6e9b8d 100644 > --- a/fs/fuse/file.c > +++ b/fs/fuse/file.c > @@ -2340,6 +2340,53 @@ int fuse_notify_poll_wakeup(struct fuse_conn *fc, > return 0; > } > > +static void fuse_do_truncate(struct file *file) > +{ > + struct fuse_file *ff = file->private_data; > + struct inode *inode = file->f_mapping->host; > + struct fuse_conn *fc = get_fuse_conn(inode); > + struct fuse_req *req; > + struct fuse_setattr_in inarg; > + struct fuse_attr_out outarg; > + int err; > + > + req = fuse_get_req_nopages(fc); > + if (IS_ERR(req)) { > + printk(KERN_WARNING "failed to allocate req for truncate " > +"(%ld)\n", PTR_ERR(req)); > + return; > + } > + > + memset(, 0, sizeof(inarg)); > + memset(, 0, sizeof(outarg)); > + > + inarg.valid |= FATTR_SIZE; > + inarg.size = i_size_read(inode); > + > + inarg.valid |= FATTR_FH; > + inarg.fh = ff->fh; > + > + req->in.h.opcode = FUSE_SETATTR; > + req->in.h.nodeid = get_node_id(inode); > + req->in.numargs = 1; > + req->in.args[0].size = sizeof(inarg); > + req->in.args[0].value = > + req->out.numargs = 1; > + if (fc->minor < 9) > + req->out.args[0].size = FUSE_COMPAT_ATTR_OUT_SIZE; > + else > + req->out.args[0].size = sizeof(outarg); > + req->out.args[0].value = > + > + fuse_request_send(fc, req); > + err = req->out.h.error; > + fuse_put_request(fc, req); > + > + if (err) > + printk(KERN_WARNING "failed to truncate to %lld with error " > +"%d\n", i_size_read(inode), err); > +} > + fuse_do_truncate() looks fairly close to fuse_do_setattr(). Is there any reason we couldn't make fuse_do_setattr() non-static, change the dentry parameter to an inode and use that? Brian > static ssize_t > fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov, > loff_t offset, unsigned long nr_segs) > @@ -2400,8 +2447,12 @@ fuse_direct_IO(int rw, struct kiocb *iocb, const > struct iovec *iov, > kfree(io); > } > > - if (rw == WRITE && ret > 0) > - fuse_write_update_size(inode, pos); > + if (rw == WRITE) { > + if (ret > 0) > + fuse_write_update_size(inode, pos); > + else if (ret < 0 && offset + count > i_size) > + fuse_do_truncate(file); > + } > > return ret; > } > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/6] fuse: truncate file if async dio failed
The patch improves error handling in fuse_direct_IO(): if we successfully submitted several fuse requests on behalf of synchronous direct write extending file and some of them failed, let's try to do our best to clean-up. Signed-off-by: Maxim Patlasov --- fs/fuse/file.c | 55 +-- 1 files changed, 53 insertions(+), 2 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index 05eed23..b6e9b8d 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -2340,6 +2340,53 @@ int fuse_notify_poll_wakeup(struct fuse_conn *fc, return 0; } +static void fuse_do_truncate(struct file *file) +{ + struct fuse_file *ff = file->private_data; + struct inode *inode = file->f_mapping->host; + struct fuse_conn *fc = get_fuse_conn(inode); + struct fuse_req *req; + struct fuse_setattr_in inarg; + struct fuse_attr_out outarg; + int err; + + req = fuse_get_req_nopages(fc); + if (IS_ERR(req)) { + printk(KERN_WARNING "failed to allocate req for truncate " + "(%ld)\n", PTR_ERR(req)); + return; + } + + memset(, 0, sizeof(inarg)); + memset(, 0, sizeof(outarg)); + + inarg.valid |= FATTR_SIZE; + inarg.size = i_size_read(inode); + + inarg.valid |= FATTR_FH; + inarg.fh = ff->fh; + + req->in.h.opcode = FUSE_SETATTR; + req->in.h.nodeid = get_node_id(inode); + req->in.numargs = 1; + req->in.args[0].size = sizeof(inarg); + req->in.args[0].value = + req->out.numargs = 1; + if (fc->minor < 9) + req->out.args[0].size = FUSE_COMPAT_ATTR_OUT_SIZE; + else + req->out.args[0].size = sizeof(outarg); + req->out.args[0].value = + + fuse_request_send(fc, req); + err = req->out.h.error; + fuse_put_request(fc, req); + + if (err) + printk(KERN_WARNING "failed to truncate to %lld with error " + "%d\n", i_size_read(inode), err); +} + static ssize_t fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov, loff_t offset, unsigned long nr_segs) @@ -2400,8 +2447,12 @@ fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov, kfree(io); } - if (rw == WRITE && ret > 0) - fuse_write_update_size(inode, pos); + if (rw == WRITE) { + if (ret > 0) + fuse_write_update_size(inode, pos); + else if (ret < 0 && offset + count > i_size) + fuse_do_truncate(file); + } return ret; } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/6] fuse: truncate file if async dio failed
The patch improves error handling in fuse_direct_IO(): if we successfully submitted several fuse requests on behalf of synchronous direct write extending file and some of them failed, let's try to do our best to clean-up. Signed-off-by: Maxim Patlasov mpatla...@parallels.com --- fs/fuse/file.c | 55 +-- 1 files changed, 53 insertions(+), 2 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index 05eed23..b6e9b8d 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -2340,6 +2340,53 @@ int fuse_notify_poll_wakeup(struct fuse_conn *fc, return 0; } +static void fuse_do_truncate(struct file *file) +{ + struct fuse_file *ff = file-private_data; + struct inode *inode = file-f_mapping-host; + struct fuse_conn *fc = get_fuse_conn(inode); + struct fuse_req *req; + struct fuse_setattr_in inarg; + struct fuse_attr_out outarg; + int err; + + req = fuse_get_req_nopages(fc); + if (IS_ERR(req)) { + printk(KERN_WARNING failed to allocate req for truncate + (%ld)\n, PTR_ERR(req)); + return; + } + + memset(inarg, 0, sizeof(inarg)); + memset(outarg, 0, sizeof(outarg)); + + inarg.valid |= FATTR_SIZE; + inarg.size = i_size_read(inode); + + inarg.valid |= FATTR_FH; + inarg.fh = ff-fh; + + req-in.h.opcode = FUSE_SETATTR; + req-in.h.nodeid = get_node_id(inode); + req-in.numargs = 1; + req-in.args[0].size = sizeof(inarg); + req-in.args[0].value = inarg; + req-out.numargs = 1; + if (fc-minor 9) + req-out.args[0].size = FUSE_COMPAT_ATTR_OUT_SIZE; + else + req-out.args[0].size = sizeof(outarg); + req-out.args[0].value = outarg; + + fuse_request_send(fc, req); + err = req-out.h.error; + fuse_put_request(fc, req); + + if (err) + printk(KERN_WARNING failed to truncate to %lld with error + %d\n, i_size_read(inode), err); +} + static ssize_t fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov, loff_t offset, unsigned long nr_segs) @@ -2400,8 +2447,12 @@ fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov, kfree(io); } - if (rw == WRITE ret 0) - fuse_write_update_size(inode, pos); + if (rw == WRITE) { + if (ret 0) + fuse_write_update_size(inode, pos); + else if (ret 0 offset + count i_size) + fuse_do_truncate(file); + } return ret; } -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/6] fuse: truncate file if async dio failed
On 12/14/2012 10:21 AM, Maxim V. Patlasov wrote: The patch improves error handling in fuse_direct_IO(): if we successfully submitted several fuse requests on behalf of synchronous direct write extending file and some of them failed, let's try to do our best to clean-up. Signed-off-by: Maxim Patlasov mpatla...@parallels.com --- fs/fuse/file.c | 55 +-- 1 files changed, 53 insertions(+), 2 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index 05eed23..b6e9b8d 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -2340,6 +2340,53 @@ int fuse_notify_poll_wakeup(struct fuse_conn *fc, return 0; } +static void fuse_do_truncate(struct file *file) +{ + struct fuse_file *ff = file-private_data; + struct inode *inode = file-f_mapping-host; + struct fuse_conn *fc = get_fuse_conn(inode); + struct fuse_req *req; + struct fuse_setattr_in inarg; + struct fuse_attr_out outarg; + int err; + + req = fuse_get_req_nopages(fc); + if (IS_ERR(req)) { + printk(KERN_WARNING failed to allocate req for truncate +(%ld)\n, PTR_ERR(req)); + return; + } + + memset(inarg, 0, sizeof(inarg)); + memset(outarg, 0, sizeof(outarg)); + + inarg.valid |= FATTR_SIZE; + inarg.size = i_size_read(inode); + + inarg.valid |= FATTR_FH; + inarg.fh = ff-fh; + + req-in.h.opcode = FUSE_SETATTR; + req-in.h.nodeid = get_node_id(inode); + req-in.numargs = 1; + req-in.args[0].size = sizeof(inarg); + req-in.args[0].value = inarg; + req-out.numargs = 1; + if (fc-minor 9) + req-out.args[0].size = FUSE_COMPAT_ATTR_OUT_SIZE; + else + req-out.args[0].size = sizeof(outarg); + req-out.args[0].value = outarg; + + fuse_request_send(fc, req); + err = req-out.h.error; + fuse_put_request(fc, req); + + if (err) + printk(KERN_WARNING failed to truncate to %lld with error +%d\n, i_size_read(inode), err); +} + fuse_do_truncate() looks fairly close to fuse_do_setattr(). Is there any reason we couldn't make fuse_do_setattr() non-static, change the dentry parameter to an inode and use that? Brian static ssize_t fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov, loff_t offset, unsigned long nr_segs) @@ -2400,8 +2447,12 @@ fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov, kfree(io); } - if (rw == WRITE ret 0) - fuse_write_update_size(inode, pos); + if (rw == WRITE) { + if (ret 0) + fuse_write_update_size(inode, pos); + else if (ret 0 offset + count i_size) + fuse_do_truncate(file); + } return ret; } -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/6] fuse: truncate file if async dio failed
The patch improves error handling in fuse_direct_IO(): if we successfully submitted several fuse requests on behalf of synchronous direct write extending file and some of them failed, let's try to do our best to clean-up. Signed-off-by: Maxim Patlasov --- fs/fuse/file.c | 55 +-- 1 files changed, 53 insertions(+), 2 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index ef6d3de..3e0fdb7 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -2341,6 +2341,53 @@ int fuse_notify_poll_wakeup(struct fuse_conn *fc, return 0; } +static void fuse_do_truncate(struct file *file) +{ + struct fuse_file *ff = file->private_data; + struct inode *inode = file->f_mapping->host; + struct fuse_conn *fc = get_fuse_conn(inode); + struct fuse_req *req; + struct fuse_setattr_in inarg; + struct fuse_attr_out outarg; + int err; + + req = fuse_get_req_nopages(fc); + if (IS_ERR(req)) { + printk(KERN_WARNING "failed to allocate req for truncate " + "(%ld)\n", PTR_ERR(req)); + return; + } + + memset(, 0, sizeof(inarg)); + memset(, 0, sizeof(outarg)); + + inarg.valid |= FATTR_SIZE; + inarg.size = i_size_read(inode); + + inarg.valid |= FATTR_FH; + inarg.fh = ff->fh; + + req->in.h.opcode = FUSE_SETATTR; + req->in.h.nodeid = get_node_id(inode); + req->in.numargs = 1; + req->in.args[0].size = sizeof(inarg); + req->in.args[0].value = + req->out.numargs = 1; + if (fc->minor < 9) + req->out.args[0].size = FUSE_COMPAT_ATTR_OUT_SIZE; + else + req->out.args[0].size = sizeof(outarg); + req->out.args[0].value = + + fuse_request_send(fc, req); + err = req->out.h.error; + fuse_put_request(fc, req); + + if (err) + printk(KERN_WARNING "failed to truncate to %lld with error " + "%d\n", i_size_read(inode), err); +} + static ssize_t fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov, loff_t offset, unsigned long nr_segs) @@ -2393,8 +2440,12 @@ fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov, ret = wait_on_sync_kiocb(iocb); - if (rw == WRITE && ret > 0) - fuse_write_update_size(inode, pos); + if (rw == WRITE) { + if (ret > 0) + fuse_write_update_size(inode, pos); + else if (ret < 0 && offset + count > i_size) + fuse_do_truncate(file); + } } return ret; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/6] fuse: truncate file if async dio failed
The patch improves error handling in fuse_direct_IO(): if we successfully submitted several fuse requests on behalf of synchronous direct write extending file and some of them failed, let's try to do our best to clean-up. Signed-off-by: Maxim Patlasov mpatla...@parallels.com --- fs/fuse/file.c | 55 +-- 1 files changed, 53 insertions(+), 2 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index ef6d3de..3e0fdb7 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -2341,6 +2341,53 @@ int fuse_notify_poll_wakeup(struct fuse_conn *fc, return 0; } +static void fuse_do_truncate(struct file *file) +{ + struct fuse_file *ff = file-private_data; + struct inode *inode = file-f_mapping-host; + struct fuse_conn *fc = get_fuse_conn(inode); + struct fuse_req *req; + struct fuse_setattr_in inarg; + struct fuse_attr_out outarg; + int err; + + req = fuse_get_req_nopages(fc); + if (IS_ERR(req)) { + printk(KERN_WARNING failed to allocate req for truncate + (%ld)\n, PTR_ERR(req)); + return; + } + + memset(inarg, 0, sizeof(inarg)); + memset(outarg, 0, sizeof(outarg)); + + inarg.valid |= FATTR_SIZE; + inarg.size = i_size_read(inode); + + inarg.valid |= FATTR_FH; + inarg.fh = ff-fh; + + req-in.h.opcode = FUSE_SETATTR; + req-in.h.nodeid = get_node_id(inode); + req-in.numargs = 1; + req-in.args[0].size = sizeof(inarg); + req-in.args[0].value = inarg; + req-out.numargs = 1; + if (fc-minor 9) + req-out.args[0].size = FUSE_COMPAT_ATTR_OUT_SIZE; + else + req-out.args[0].size = sizeof(outarg); + req-out.args[0].value = outarg; + + fuse_request_send(fc, req); + err = req-out.h.error; + fuse_put_request(fc, req); + + if (err) + printk(KERN_WARNING failed to truncate to %lld with error + %d\n, i_size_read(inode), err); +} + static ssize_t fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov, loff_t offset, unsigned long nr_segs) @@ -2393,8 +2440,12 @@ fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov, ret = wait_on_sync_kiocb(iocb); - if (rw == WRITE ret 0) - fuse_write_update_size(inode, pos); + if (rw == WRITE) { + if (ret 0) + fuse_write_update_size(inode, pos); + else if (ret 0 offset + count i_size) + fuse_do_truncate(file); + } } return ret; -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/