* hpfs_setattr error case avoids unlock_kernel ?
@ 2010-12-12 0:49 Dr. David Alan Gilbert
2010-12-13 16:03 ` Christoph Hellwig
0 siblings, 1 reply; 5+ messages in thread
From: Dr. David Alan Gilbert @ 2010-12-12 0:49 UTC (permalink / raw)
To: mikulas, hch; +Cc: linux-fsdevel
Hi,
'sparse' pointed out to me the code in fs/hpfs/inode.c:hpfs_setattr
changed by Christoph's patch 1025774ce411f2bd4b059ad7b53f0003569b74fa
'remove inode_setattr', and it does look like the error path
avoids an unlock_kernel (around like 284 of fs/hpfs/inode.c)
fs/hpfs/inode.c:265:5: warning: context imbalance in 'hpfs_setattr' - different lock contexts for basic block
--------------------------
error = inode_change_ok(inode, attr);
if (error)
goto out_unlock;
if ((attr->ia_valid & ATTR_SIZE) &&
attr->ia_size != i_size_read(inode)) {
error = vmtruncate(inode, attr->ia_size);
if (error)
************ return error;
}
setattr_copy(inode, attr);
mark_inode_dirty(inode);
hpfs_write_inode(inode);
out_unlock:
unlock_kernel();
return error;
--------------------------
it looks like that 1st 'return error' needs to change into a goto out_unlock;
Dave
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ gro.gilbert @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: hpfs_setattr error case avoids unlock_kernel ?
2010-12-12 0:49 hpfs_setattr error case avoids unlock_kernel ? Dr. David Alan Gilbert
@ 2010-12-13 16:03 ` Christoph Hellwig
2010-12-13 17:09 ` [PATCH] hpfs_setattr error case avoids unlock_kernel Dr. David Alan Gilbert
2011-02-07 15:31 ` hpfs_setattr error case avoids unlock_kernel ? Mikulas Patocka
0 siblings, 2 replies; 5+ messages in thread
From: Christoph Hellwig @ 2010-12-13 16:03 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: mikulas, hch, linux-fsdevel
On Sun, Dec 12, 2010 at 12:49:47AM +0000, Dr. David Alan Gilbert wrote:
> it looks like that 1st 'return error' needs to change into a goto out_unlock;
Yes. Care to send a patch to fix it?
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] hpfs_setattr error case avoids unlock_kernel
2010-12-13 16:03 ` Christoph Hellwig
@ 2010-12-13 17:09 ` Dr. David Alan Gilbert
2011-02-07 15:31 ` Mikulas Patocka
2011-02-07 15:31 ` hpfs_setattr error case avoids unlock_kernel ? Mikulas Patocka
1 sibling, 1 reply; 5+ messages in thread
From: Dr. David Alan Gilbert @ 2010-12-13 17:09 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: mikulas, linux-fsdevel
* Christoph Hellwig (hch@lst.de) wrote:
> On Sun, Dec 12, 2010 at 12:49:47AM +0000, Dr. David Alan Gilbert wrote:
> > it looks like that 1st 'return error' needs to change into a goto out_unlock;
>
> Yes. Care to send a patch to fix it?
This fixed a case that 'sparse' spotted where hpfs_setattr has an error return
that didn't go through it's path that unlocks.
This is against git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
version 6313e3c21743cc88bb5bd8aa72948ee1e83937b6.
Build tested only, I don't have an hpfs file system to test.
Dave
Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
---
diff --git a/fs/hpfs/inode.c b/fs/hpfs/inode.c
index 56f0da1..1ae35ba 100644
--- a/fs/hpfs/inode.c
+++ b/fs/hpfs/inode.c
@@ -281,7 +281,7 @@ int hpfs_setattr(struct dentry *dentry, struct iattr *attr)
attr->ia_size != i_size_read(inode)) {
error = vmtruncate(inode, attr->ia_size);
if (error)
- return error;
+ goto out_unlock;
}
setattr_copy(inode, attr);
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ gro.gilbert @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: hpfs_setattr error case avoids unlock_kernel ?
2010-12-13 16:03 ` Christoph Hellwig
2010-12-13 17:09 ` [PATCH] hpfs_setattr error case avoids unlock_kernel Dr. David Alan Gilbert
@ 2011-02-07 15:31 ` Mikulas Patocka
1 sibling, 0 replies; 5+ messages in thread
From: Mikulas Patocka @ 2011-02-07 15:31 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Dr. David Alan Gilbert, linux-fsdevel
On Mon, 13 Dec 2010, Christoph Hellwig wrote:
> On Sun, Dec 12, 2010 at 12:49:47AM +0000, Dr. David Alan Gilbert wrote:
> > it looks like that 1st 'return error' needs to change into a goto out_unlock;
>
> Yes. Care to send a patch to fix it?
Sorry, I am in a hospital and can't send anything. Do the patch yourself,
please.
Mikulas
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] hpfs_setattr error case avoids unlock_kernel
2010-12-13 17:09 ` [PATCH] hpfs_setattr error case avoids unlock_kernel Dr. David Alan Gilbert
@ 2011-02-07 15:31 ` Mikulas Patocka
0 siblings, 0 replies; 5+ messages in thread
From: Mikulas Patocka @ 2011-02-07 15:31 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: Christoph Hellwig, linux-fsdevel
ACK.
Mikulas
On Mon, 13 Dec 2010, Dr. David Alan Gilbert wrote:
> * Christoph Hellwig (hch@lst.de) wrote:
> > On Sun, Dec 12, 2010 at 12:49:47AM +0000, Dr. David Alan Gilbert wrote:
> > > it looks like that 1st 'return error' needs to change into a goto out_unlock;
> >
> > Yes. Care to send a patch to fix it?
>
>
> This fixed a case that 'sparse' spotted where hpfs_setattr has an error return
> that didn't go through it's path that unlocks.
>
> This is against git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> version 6313e3c21743cc88bb5bd8aa72948ee1e83937b6.
>
> Build tested only, I don't have an hpfs file system to test.
>
> Dave
>
> Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
>
> ---
> diff --git a/fs/hpfs/inode.c b/fs/hpfs/inode.c
> index 56f0da1..1ae35ba 100644
> --- a/fs/hpfs/inode.c
> +++ b/fs/hpfs/inode.c
> @@ -281,7 +281,7 @@ int hpfs_setattr(struct dentry *dentry, struct iattr *attr)
> attr->ia_size != i_size_read(inode)) {
> error = vmtruncate(inode, attr->ia_size);
> if (error)
> - return error;
> + goto out_unlock;
> }
>
> setattr_copy(inode, attr);
> --
> -----Open up your eyes, open up your mind, open up your code -------
> / Dr. David Alan Gilbert | Running GNU/Linux | Happy \
> \ gro.gilbert @ treblig.org | | In Hex /
> \ _________________________|_____ http://www.treblig.org |_______/
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-02-07 15:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-12 0:49 hpfs_setattr error case avoids unlock_kernel ? Dr. David Alan Gilbert
2010-12-13 16:03 ` Christoph Hellwig
2010-12-13 17:09 ` [PATCH] hpfs_setattr error case avoids unlock_kernel Dr. David Alan Gilbert
2011-02-07 15:31 ` Mikulas Patocka
2011-02-07 15:31 ` hpfs_setattr error case avoids unlock_kernel ? Mikulas Patocka
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).