* [PATCH] ext3: ext3_symlink should use GFP_NOFS allocations inside
@ 2006-03-09 17:19 Kirill Korotaev
2006-03-09 21:04 ` Al Viro
2006-03-10 13:02 ` Michael Tokarev
0 siblings, 2 replies; 7+ messages in thread
From: Kirill Korotaev @ 2006-03-09 17:19 UTC (permalink / raw)
To: Andrew Morton, Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 521 bytes --]
This patch fixes illegal __GFP_FS allocation inside ext3
transaction in ext3_symlink().
Such allocation may re-enter ext3 code from
try_to_free_pages. But JBD/ext3 code keeps a pointer to current
journal handle in task_struct and, hence, is not reentrable.
This bug led to "Assertion failure in journal_dirty_metadata()" messages.
http://bugzilla.openvz.org/show_bug.cgi?id=115
Signed-Off-By: Andrey Savochkin <saw@saw.sw.com.sg>
Signed-Off-By: Kirill Korotaev <dev@openvz.org>
Thanks,
Kirill
P.S. against 2.6.16-rc5
[-- Attachment #2: diff-ext3-nofssymlink-20060210 --]
[-- Type: text/plain, Size: 4813 bytes --]
--- ./fs/ext2/namei.c.symlnk 2006-03-03 10:51:01.000000000 +0300
+++ ./fs/ext2/namei.c 2006-03-09 19:38:49.000000000 +0300
@@ -185,7 +185,7 @@ static int ext2_symlink (struct inode *
inode->i_mapping->a_ops = &ext2_nobh_aops;
else
inode->i_mapping->a_ops = &ext2_aops;
- err = page_symlink(inode, symname, l);
+ err = page_symlink(inode, symname, l, GFP_KERNEL);
if (err)
goto out_fail;
} else {
--- ./fs/ext3/namei.c.symlnk 2006-03-03 10:51:01.000000000 +0300
+++ ./fs/ext3/namei.c 2006-03-09 19:38:49.000000000 +0300
@@ -2141,7 +2141,7 @@ retry:
* We have a transaction open. All is sweetness. It also sets
* i_size in generic_commit_write().
*/
- err = page_symlink(inode, symname, l);
+ err = page_symlink(inode, symname, l, GFP_NOFS);
if (err) {
ext3_dec_count(handle, inode);
ext3_mark_inode_dirty(handle, inode);
--- ./fs/hfsplus/dir.c.symlnk 2006-03-03 10:51:01.000000000 +0300
+++ ./fs/hfsplus/dir.c 2006-03-09 19:38:49.000000000 +0300
@@ -406,7 +406,7 @@ static int hfsplus_symlink(struct inode
if (!inode)
return -ENOSPC;
- res = page_symlink(inode, symname, strlen(symname) + 1);
+ res = page_symlink(inode, symname, strlen(symname) + 1, GFP_KERNEL);
if (res) {
inode->i_nlink = 0;
hfsplus_delete_inode(inode);
--- ./fs/hugetlbfs/inode.c.symlnk 2006-03-03 10:51:01.000000000 +0300
+++ ./fs/hugetlbfs/inode.c 2006-03-09 19:38:49.000000000 +0300
@@ -482,7 +482,7 @@ static int hugetlbfs_symlink(struct inod
gid, S_IFLNK|S_IRWXUGO, 0);
if (inode) {
int l = strlen(symname)+1;
- error = page_symlink(inode, symname, l);
+ error = page_symlink(inode, symname, l, GFP_KERNEL);
if (!error) {
d_instantiate(dentry, inode);
dget(dentry);
--- ./fs/minix/namei.c.symlnk 2006-01-03 06:21:10.000000000 +0300
+++ ./fs/minix/namei.c 2006-03-09 19:38:49.000000000 +0300
@@ -116,7 +116,7 @@ static int minix_symlink(struct inode *
inode->i_mode = S_IFLNK | 0777;
minix_set_inode(inode, 0);
- err = page_symlink(inode, symname, i);
+ err = page_symlink(inode, symname, i, GFP_KERNEL);
if (err)
goto out_fail;
--- ./fs/namei.c.symlnk 2006-03-03 10:51:01.000000000 +0300
+++ ./fs/namei.c 2006-03-09 19:42:09.000000000 +0300
@@ -2613,13 +2613,16 @@ void page_put_link(struct dentry *dentry
}
}
-int page_symlink(struct inode *inode, const char *symname, int len)
+int page_symlink(struct inode *inode, const char *symname, int len,
+ gfp_t gfp_mask)
{
struct address_space *mapping = inode->i_mapping;
- struct page *page = grab_cache_page(mapping, 0);
+ struct page *page;
int err = -ENOMEM;
char *kaddr;
+ page = find_or_create_page(mapping, 0,
+ mapping_gfp_mask(mapping) | gfp_mask);
if (!page)
goto fail;
err = mapping->a_ops->prepare_write(NULL, page, 0, len-1);
--- ./fs/ramfs/inode.c.symlnk 2006-03-03 10:51:01.000000000 +0300
+++ ./fs/ramfs/inode.c 2006-03-09 19:38:49.000000000 +0300
@@ -131,7 +131,7 @@ static int ramfs_symlink(struct inode *
inode = ramfs_get_inode(dir->i_sb, S_IFLNK|S_IRWXUGO, 0);
if (inode) {
int l = strlen(symname)+1;
- error = page_symlink(inode, symname, l);
+ error = page_symlink(inode, symname, l, GFP_KERNEL);
if (!error) {
if (dir->i_mode & S_ISGID)
inode->i_gid = dir->i_gid;
--- ./fs/sysv/namei.c.symlnk 2006-01-03 06:21:10.000000000 +0300
+++ ./fs/sysv/namei.c 2006-03-09 19:38:49.000000000 +0300
@@ -114,7 +114,7 @@ static int sysv_symlink(struct inode * d
goto out;
sysv_set_inode(inode, 0);
- err = page_symlink(inode, symname, l);
+ err = page_symlink(inode, symname, l, GFP_KERNEL);
if (err)
goto out_fail;
--- ./fs/ufs/namei.c.symlnk 2006-01-03 06:21:10.000000000 +0300
+++ ./fs/ufs/namei.c 2006-03-09 19:38:49.000000000 +0300
@@ -156,7 +156,7 @@ static int ufs_symlink (struct inode * d
/* slow symlink */
inode->i_op = &page_symlink_inode_operations;
inode->i_mapping->a_ops = &ufs_aops;
- err = page_symlink(inode, symname, l);
+ err = page_symlink(inode, symname, l, GFP_KERNEL);
if (err)
goto out_fail;
} else {
--- ./include/linux/fs.h.symlnk 2006-03-09 16:03:44.000000000 +0300
+++ ./include/linux/fs.h 2006-03-09 19:41:25.000000000 +0300
@@ -1669,7 +1669,8 @@ extern int vfs_follow_link(struct nameid
extern int page_readlink(struct dentry *, char __user *, int);
extern void *page_follow_link_light(struct dentry *, struct nameidata *);
extern void page_put_link(struct dentry *, struct nameidata *, void *);
-extern int page_symlink(struct inode *inode, const char *symname, int len);
+extern int page_symlink(struct inode *inode, const char *symname, int len,
+ gfp_t gfp_mask);
extern struct inode_operations page_symlink_inode_operations;
extern int generic_readlink(struct dentry *, char __user *, int);
extern void generic_fillattr(struct inode *, struct kstat *);
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ext3: ext3_symlink should use GFP_NOFS allocations inside
2006-03-09 17:19 Kirill Korotaev
@ 2006-03-09 21:04 ` Al Viro
2006-03-10 13:02 ` Michael Tokarev
1 sibling, 0 replies; 7+ messages in thread
From: Al Viro @ 2006-03-09 21:04 UTC (permalink / raw)
To: Kirill Korotaev; +Cc: Andrew Morton, Linux Kernel Mailing List
On Thu, Mar 09, 2006 at 08:19:15PM +0300, Kirill Korotaev wrote:
> This patch fixes illegal __GFP_FS allocation inside ext3
> transaction in ext3_symlink().
> Such allocation may re-enter ext3 code from
> try_to_free_pages. But JBD/ext3 code keeps a pointer to current
> journal handle in task_struct and, hence, is not reentrable.
New helper, please.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] ext3: ext3_symlink should use GFP_NOFS allocations inside
@ 2006-03-10 8:08 Kirill Korotaev
2006-03-10 8:31 ` Al Viro
0 siblings, 1 reply; 7+ messages in thread
From: Kirill Korotaev @ 2006-03-10 8:08 UTC (permalink / raw)
To: Andrew Morton, viro, linux-kernel, Andrey Savochkin, devel
[-- Attachment #1: Type: text/plain, Size: 597 bytes --]
Andrew,
Resending a patch with a separate helper as requested by Al Viro:
This patch fixes illegal __GFP_FS allocation inside ext3
transaction in ext3_symlink().
Such allocation may re-enter ext3 code from
try_to_free_pages. But JBD/ext3 code keeps a pointer to current
journal handle in task_struct and, hence, is not reentrable.
This bug led to "Assertion failure in journal_dirty_metadata()" messages.
http://bugzilla.openvz.org/show_bug.cgi?id=115
Signed-Off-By: Andrey Savochkin <saw@saw.sw.com.sg>
Signed-Off-By: Kirill Korotaev <dev@openvz.org>
Thanks,
Kirill
P.S. against 2.6.16-rc5
[-- Attachment #2: diff-ext3-nofssymlink-20060210 --]
[-- Type: text/plain, Size: 2462 bytes --]
--- ./fs/ext3/namei.c.symlnkfix 2006-03-10 10:24:05.000000000 +0300
+++ ./fs/ext3/namei.c 2006-03-10 10:24:49.000000000 +0300
@@ -2141,7 +2141,7 @@ retry:
* We have a transaction open. All is sweetness. It also sets
* i_size in generic_commit_write().
*/
- err = page_symlink(inode, symname, l);
+ err = __page_symlink(inode, symname, l, GFP_NOFS);
if (err) {
ext3_dec_count(handle, inode);
ext3_mark_inode_dirty(handle, inode);
--- ./fs/namei.c.symlnkfix 2006-03-10 10:24:05.000000000 +0300
+++ ./fs/namei.c 2006-03-10 10:34:58.000000000 +0300
@@ -2613,13 +2613,16 @@ void page_put_link(struct dentry *dentry
}
}
-int page_symlink(struct inode *inode, const char *symname, int len)
+int __page_symlink(struct inode *inode, const char *symname, int len,
+ gfp_t gfp_mask)
{
struct address_space *mapping = inode->i_mapping;
- struct page *page = grab_cache_page(mapping, 0);
+ struct page *page;
int err = -ENOMEM;
char *kaddr;
+ page = find_or_create_page(mapping, 0,
+ mapping_gfp_mask(mapping) | gfp_mask);
if (!page)
goto fail;
err = mapping->a_ops->prepare_write(NULL, page, 0, len-1);
@@ -2654,6 +2657,11 @@ fail:
return err;
}
+int page_symlink(struct inode *inode, const char *symname, int len)
+{
+ return __page_symlink(inode, symname, len, GFP_KERNEL);
+}
+
struct inode_operations page_symlink_inode_operations = {
.readlink = generic_readlink,
.follow_link = page_follow_link_light,
@@ -2672,6 +2680,7 @@ EXPORT_SYMBOL(lookup_one_len);
EXPORT_SYMBOL(page_follow_link_light);
EXPORT_SYMBOL(page_put_link);
EXPORT_SYMBOL(page_readlink);
+EXPORT_SYMBOL(__page_symlink);
EXPORT_SYMBOL(page_symlink);
EXPORT_SYMBOL(page_symlink_inode_operations);
EXPORT_SYMBOL(path_lookup);
--- ./include/linux/fs.h.symlnkfix 2006-03-10 10:24:05.000000000 +0300
+++ ./include/linux/fs.h 2006-03-10 10:27:40.000000000 +0300
@@ -1669,6 +1669,8 @@ extern int vfs_follow_link(struct nameid
extern int page_readlink(struct dentry *, char __user *, int);
extern void *page_follow_link_light(struct dentry *, struct nameidata *);
extern void page_put_link(struct dentry *, struct nameidata *, void *);
+extern int __page_symlink(struct inode *inode, const char *symname, int len,
+ gfp_t gfp_mask);
extern int page_symlink(struct inode *inode, const char *symname, int len);
extern struct inode_operations page_symlink_inode_operations;
extern int generic_readlink(struct dentry *, char __user *, int);
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ext3: ext3_symlink should use GFP_NOFS allocations inside
2006-03-10 8:08 [PATCH] ext3: ext3_symlink should use GFP_NOFS allocations inside Kirill Korotaev
@ 2006-03-10 8:31 ` Al Viro
2006-03-10 8:46 ` Kirill Korotaev
0 siblings, 1 reply; 7+ messages in thread
From: Al Viro @ 2006-03-10 8:31 UTC (permalink / raw)
To: Kirill Korotaev; +Cc: Andrew Morton, linux-kernel, Andrey Savochkin, devel
On Fri, Mar 10, 2006 at 11:08:29AM +0300, Kirill Korotaev wrote:
> + page = find_or_create_page(mapping, 0,
> + mapping_gfp_mask(mapping) | gfp_mask);
> +int page_symlink(struct inode *inode, const char *symname, int len)
> +{
> + return __page_symlink(inode, symname, len, GFP_KERNEL);
s/GFP_KERNEL/0/; if somebody has e.g. GFP_NOFS in their mapping flags,
you end up breaking their code. We really pass extra flags to be added
to default ones; page_symlink() should pass 0.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ext3: ext3_symlink should use GFP_NOFS allocations inside
2006-03-10 8:31 ` Al Viro
@ 2006-03-10 8:46 ` Kirill Korotaev
0 siblings, 0 replies; 7+ messages in thread
From: Kirill Korotaev @ 2006-03-10 8:46 UTC (permalink / raw)
To: Al Viro
Cc: Kirill Korotaev, Andrew Morton, linux-kernel, Andrey Savochkin,
devel
>>+ page = find_or_create_page(mapping, 0,
>>+ mapping_gfp_mask(mapping) | gfp_mask);
>
>
>>+int page_symlink(struct inode *inode, const char *symname, int len)
>>+{
>>+ return __page_symlink(inode, symname, len, GFP_KERNEL);
>
>
> s/GFP_KERNEL/0/; if somebody has e.g. GFP_NOFS in their mapping flags,
> you end up breaking their code. We really pass extra flags to be added
> to default ones; page_symlink() should pass 0.
thanks for noticing this.
fixed and resend.
Thanks,
Kirill
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ext3: ext3_symlink should use GFP_NOFS allocations inside
2006-03-09 17:19 Kirill Korotaev
2006-03-09 21:04 ` Al Viro
@ 2006-03-10 13:02 ` Michael Tokarev
2006-03-10 20:21 ` Andrew Morton
1 sibling, 1 reply; 7+ messages in thread
From: Michael Tokarev @ 2006-03-10 13:02 UTC (permalink / raw)
To: Kirill Korotaev; +Cc: Andrew Morton, Linux Kernel Mailing List
Kirill Korotaev wrote:
> This patch fixes illegal __GFP_FS allocation inside ext3
> transaction in ext3_symlink().
> Such allocation may re-enter ext3 code from
> try_to_free_pages. But JBD/ext3 code keeps a pointer to current
> journal handle in task_struct and, hence, is not reentrable.
>
> This bug led to "Assertion failure in journal_dirty_metadata()" messages.
>
> http://bugzilla.openvz.org/show_bug.cgi?id=115
Is it the same bug as http://marc.theaimsgroup.com/?t=113870577100003&r=1&w=2 ?
Thanks.
/mjt
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ext3: ext3_symlink should use GFP_NOFS allocations inside
2006-03-10 13:02 ` Michael Tokarev
@ 2006-03-10 20:21 ` Andrew Morton
0 siblings, 0 replies; 7+ messages in thread
From: Andrew Morton @ 2006-03-10 20:21 UTC (permalink / raw)
To: Michael Tokarev; +Cc: dev, linux-kernel
Michael Tokarev <mjt@tls.msk.ru> wrote:
>
> Kirill Korotaev wrote:
> > This patch fixes illegal __GFP_FS allocation inside ext3
> > transaction in ext3_symlink().
> > Such allocation may re-enter ext3 code from
> > try_to_free_pages. But JBD/ext3 code keeps a pointer to current
> > journal handle in task_struct and, hence, is not reentrable.
> >
> > This bug led to "Assertion failure in journal_dirty_metadata()" messages.
> >
> > http://bugzilla.openvz.org/show_bug.cgi?id=115
>
> Is it the same bug as http://marc.theaimsgroup.com/?t=113870577100003&r=1&w=2 ?
>
yes.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2006-03-10 20:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-03-10 8:08 [PATCH] ext3: ext3_symlink should use GFP_NOFS allocations inside Kirill Korotaev
2006-03-10 8:31 ` Al Viro
2006-03-10 8:46 ` Kirill Korotaev
-- strict thread matches above, loose matches on Subject: below --
2006-03-09 17:19 Kirill Korotaev
2006-03-09 21:04 ` Al Viro
2006-03-10 13:02 ` Michael Tokarev
2006-03-10 20:21 ` Andrew Morton
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).