Re: Kernel bug: Bad page state: related to generic symlink code and mmap
Hi Linus, On Fri, 19 Aug 2005, Linus Torvalds wrote: On Fri, 19 Aug 2005, Anton Altaparmakov wrote: Yes, sure. I have applied your patch to our 2.6.11.4 tree (with the one liner change I emailed you just now) and have kicked off a compile. Actually, hold on. The original patch had another problem: it returned an uninitialized page pointer when page_getlink() failed. I copied the fix to that problem from your new patch by hand and recompiled the kernel (that way I didn't have to rebuild the modules again). Note I did not apply any of the fs specific changes. I only did the VFS part of your patch that was actually necessary. And I reverted my ncpfs fix so it is now again using the vfs supplied symlink helpers. Having booted the new kernel and trying nautilus it worked fine accessing the same symlink that failed before, so your patch fixes the ncpfs problem as one would expect but always good to be sure. (-: btw. It may be an idea to switch smbfs to use the fixed generic symlink functions, too, so it benefits from symlink caching... Best regards, Anton -- Anton Altaparmakov aia21 at cam.ac.uk (replace at with @) Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net WWW: http://linux-ntfs.sf.net/ http://www-stu.christs.cam.ac.uk/~aia21/ - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Kernel bug: Bad page state: related to generic symlink code and mmap
Hi,
There is a bug somewhere in 2.6.11.4 and I can't figure out where it is.
I assume it is present in older and newer kernels, too as the related
code hasn't changed much AFAICS and googling for Bad page state
returns rather a lot of hits relating to both older (up to 2.5.70!) and
newer kernels...
Note: PLEASE do not stop reading because you read ncpfs below as I am
pretty sure it is not ncpfs related! And looking at google a lot of
people have reported such similar problems since 2.5.70 or so and they
were all told to go away as they have bad ram. That is impossible
because this happens on well over 600 workstations and several servers
100% reproducible. Many different types of hardware, different makes,
difference age, all running smp kernels even if single cpu. You can't
tell me they all have bad ram. Windows works fine and Linux works fine
except for that one specific problem which is 100% reproducible...
The bug only appears, but it appears 100% reproducibly when a cross
volume symlink on ncpfs is accessed using nautilus under gnome. I.e.
double click on a cross volume symlink on ncpfs in nautilus and the
machine locks up solid. Here is what is logged to syslog over the
network (this is an example, after the second message the machine is
locked up solid, I can send you other traces if you want to see more of
them):
Bad page state at free_hot_cold_page (in process 'nautilus', page c10d33c0)
flags:0x200c mapping:cd83f5d0 mapcount:0 count:0
Backtrace:
[c01470ec] bad_page+0x7c/0xb0
[c014790c] free_hot_cold_page+0x7c/0x110
[c01482fc] __pagevec_free+0x1c/0x30
[c014d3e3] release_pages+0x163/0x180
[c014d572] __pagevec_lru_add+0xb2/0xc0
[c014d1a5] lru_add_drain+0x45/0x50
[c015640f] unmap_region+0x2f/0x130
[c015653a] detach_vmas_to_be_unmapped+0x2a/0x60
[c0156782] do_munmap+0x102/0x150
[c0156818] sys_munmap+0x48/0x70
[c0104079] sysenter_past_esp+0x52/0x79
Trying to fix it up, but a reboot is needed
Bad page state at free_hot_cold_page (in process 'nautilus', page c10d33c0)
flags:0x2004 mapping: mapcount:1 count:0
Backtrace:
[c01470ec] bad_page+0x7c/0xb0
[c014790c] free_hot_cold_page+0x7c/0x110
[c0171659] link_path_walk+0x929/0xe80
[c02b6882] ip_local_deliver+0x192/0x290
[c02b6d94] ip_rcv+0x414/0x540
[c0171e96] path_lookup+0xa6/0x1c0
[c01726f2] open_namei+0x82/0x680
[c011b297] recalc_task_prio+0xe7/0x200
[c0162588] filp_open+0x28/0x50
[c016284a] get_unused_fd+0x9a/0xc0
[c01705e8] getname+0x68/0xe0
[c016295c] sys_open+0x3c/0xa0
[c0104079] sysenter_past_esp+0x52/0x79
Trying to fix it up, but a reboot is needed
So somehow pages get into a bad state.
Note that using gnome-terminal or console to access the symlink works
fine. It is only nautilus and looking at the backtrace it has something
to do with mmap.
Note this is not an ncpfs bug AFAICS because ncpfs uses the generic
symlink code provided by VFS. From fs/ncpfs/inode.c:
static struct inode_operations ncp_symlink_inode_operations = {
.readlink = generic_readlink,
.follow_link= page_follow_link_light,
.put_link = page_put_link,
.setattr= ncp_notify_change,
};
And from fs/ncpfs/symlink.c:
struct address_space_operations ncp_symlink_aops = {
.readpage = ncp_symlink_readpage,
};
And ncp_symlink_readpage() is:
#define NCP_SYMLINK_MAGIC0 cpu_to_le32(0x6c6d7973) /* symlnk- */
#define NCP_SYMLINK_MAGIC1 cpu_to_le32(0x3e2d6b6e)
/* - read a symbolic link -- */
static int ncp_symlink_readpage(struct file *file, struct page *page)
{
struct inode *inode = page-mapping-host;
int error, length, len;
char *link, *rawlink;
char *buf = kmap(page);
error = -ENOMEM;
rawlink=(char *)kmalloc(NCP_MAX_SYMLINK_SIZE, GFP_KERNEL);
if (!rawlink)
goto fail;
if (ncp_make_open(inode,O_RDONLY))
goto failEIO;
error=ncp_read_kernel(NCP_SERVER(inode),NCP_FINFO(inode)-file_handle,
0,NCP_MAX_SYMLINK_SIZE,rawlink,length);
ncp_inode_close(inode);
/* Close file handle if no other users... */
ncp_make_closed(inode);
if (error)
goto failEIO;
if (NCP_FINFO(inode)-flags NCPI_KLUDGE_SYMLINK) {
if (lengthNCP_MIN_SYMLINK_SIZE ||
((__le32 *)rawlink)[0]!=NCP_SYMLINK_MAGIC0 ||
((__le32 *)rawlink)[1]!=NCP_SYMLINK_MAGIC1)
goto failEIO;
link = rawlink + 8;
length -= 8;
} else {
link = rawlink;
}
len = NCP_MAX_SYMLINK_SIZE;
error = ncp_vol2io(NCP_SERVER(inode), buf, len, link, length, 0);
kfree(rawlink);
if (error)
goto fail;
SetPageUptodate(page);
kunmap(page);
unlock_page(page);
return 0;
failEIO:
error =
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, Aug 19, 2005 at 12:14:48PM +0100, Anton Altaparmakov wrote: Hi, There is a bug somewhere in 2.6.11.4 and I can't figure out where it is. I assume it is present in older and newer kernels, too as the related code hasn't changed much AFAICS and googling for Bad page state returns rather a lot of hits relating to both older (up to 2.5.70!) and newer kernels... Note: PLEASE do not stop reading because you read ncpfs below as I am pretty sure it is not ncpfs related! And looking at google a lot of people have reported such similar problems since 2.5.70 or so and they were all told to go away as they have bad ram. That is impossible because this happens on well over 600 workstations and several servers 100% reproducible. Many different types of hardware, different makes, difference age, all running smp kernels even if single cpu. You can't tell me they all have bad ram. Windows works fine and Linux works fine except for that one specific problem which is 100% reproducible... The bug only appears, but it appears 100% reproducibly when a cross volume symlink on ncpfs is accessed using nautilus under gnome. I.e. double click on a cross volume symlink on ncpfs in nautilus and the machine locks up solid. Ugh... Could you at least tell what does nautilus attempt to do at that point? Something that wouldn't show up with simple ls -l symlink or cat symlink /dev/null, judging by the above, but what? - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, 2005-08-19 at 15:20 +0100, Al Viro wrote:
On Fri, Aug 19, 2005 at 12:14:48PM +0100, Anton Altaparmakov wrote:
There is a bug somewhere in 2.6.11.4 and I can't figure out where it is.
I assume it is present in older and newer kernels, too as the related
code hasn't changed much AFAICS and googling for Bad page state
returns rather a lot of hits relating to both older (up to 2.5.70!) and
newer kernels...
Note: PLEASE do not stop reading because you read ncpfs below as I am
pretty sure it is not ncpfs related! And looking at google a lot of
people have reported such similar problems since 2.5.70 or so and they
were all told to go away as they have bad ram. That is impossible
because this happens on well over 600 workstations and several servers
100% reproducible. Many different types of hardware, different makes,
difference age, all running smp kernels even if single cpu. You can't
tell me they all have bad ram. Windows works fine and Linux works fine
except for that one specific problem which is 100% reproducible...
The bug only appears, but it appears 100% reproducibly when a cross
volume symlink on ncpfs is accessed using nautilus under gnome. I.e.
double click on a cross volume symlink on ncpfs in nautilus and the
machine locks up solid.
Ugh... Could you at least tell what does nautilus attempt to do at that
point? Something that wouldn't show up with simple ls -l symlink or
cat symlink /dev/null, judging by the above, but what?
One important thing I forgot to add is that the symlink must point to a
directory not a file. If it points to a file all works fine.
I tried stracing nautilus to answer your question. And this time for
the first time instead of a Bad page state I got a BUG() triggered in
fs/namei.c, the arrow below marks the spot:
void page_put_link(struct dentry *dentry, struct nameidata *nd)
{
if (!IS_ERR(nd_get_link(nd))) {
struct page *page;
page = find_get_page(dentry-d_inode-i_mapping, 0);
if (!page)
BUG();
kunmap(page);
page_cache_release(page);
page_cache_release(page);
}
}
Here is the BUG output:
kernel BUG at fs/namei.c:2329!
invalid operand: [#1]
SMP
Modules linked in: autofs4 nls_cp850 nls_utf8 ncpfs subfs fuse snd
soundcore speedstep_lib freq_table thermal processor fan button battery
ac nvram ipt_REJECT iptable_filter ip_tables af_packet edd joydev evdev
st video1394 ohci1394 raw1394 ieee1394 capability sg sr_mod dm_mod e1000
uhci_hcd usbcore i2c_i801 i2c_core hw_random parport_pc lp parport
reiserfs ide_cd cdrom ide_disk aic7xxx aic79xx via82cxxx ata_piix libata
piix ide_core sd_mod scsi_mod
CPU:3
EIP:0060:[c0174e3e]Tainted: G U VLI
EFLAGS: 00010246 (2.6.11.4-21.8-smp)
EIP is at page_put_link+0x3e/0x50
eax: ebx: ecx: d9c75654 edx:
esi: edi: d9d3 ebp: ffac2000 esp: d9d31eac
ds: 007b es: 007b ss: 0068
Process nautilus (pid: 21910, threadinfo=d9d3 task=f5caf550)
Stack: d9c75654 c0171659 4000 c2025060 0001 d9d31f1c
dff3ec80
d9c75654 001d981f 0003 d9eb2014 d9d3 d9d31f1c
d9eb2000 c0171e96 d9eb2000 d9d31f1c 0001 d9eb2000 c017211f
08542878
Call Trace:
[c0171659] link_path_walk+0x929/0xe80
[c0171e96] path_lookup+0xa6/0x1c0
[c017211f] __user_walk+0x2f/0x70
[c016c6fd] vfs_stat+0x1d/0x60
[c016ce5f] sys_stat64+0xf/0x30
[c0109051] do_syscall_trace+0xb1/0x175
[c01040d7] syscall_call+0x7/0xb
Code: 31 d2 8b 80 a8 00 00 00 e8 80 e6 fc ff 85 c0 89 c3 74 18 89 d8 e8
93 5e fa ff 89 d8 e8 2c 80 fd ff 89 d8 5b e9 24 80 fd ff 5b c3 0f 0b
19 09 76 bb 32 c0 eb de 90 8d b4 26 00 00 00 00 83 ec 28
The compressed strace output is attached. It was tracing the main
nautilus process (strace -f -F -o /strace.log -p 21910) which is where
the bug was hit as well. FWIW I attached strace to the process just
before double-clicking on the symlink in the nautilus window...
Looking at the bottom of the strace.log file it seems like two possibly
concurrent stat64() calls of the symlink were running, one of which did
not complete... (note this was done on a dual-cpu machine with
hyperthreading enabled, i.e. it appears to have four cpus).
Does this help?
Let me know if I can provide any other details... (I can provide an ssh
connection to a machine for testing purposes if required.)
Best regards,
Anton
--
Anton Altaparmakov aia21 at cam.ac.uk (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ http://www-stu.christs.cam.ac.uk/~aia21/
strace.log.bz2
Description: application/bzip
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, 19 Aug 2005, Anton Altaparmakov wrote: struct page *page; page = find_get_page(dentry-d_inode-i_mapping, 0); if (!page) BUG(); Something has truncated the mapping. My guess is that you had a cache invalidate event that removed the page from the mapping while it was being used. That might also explain why this only happens for ncpfs. I bet that in the other cases, the mapping was also invalidated, but re-filled immediately, and your strace slowed the other process down enough that it didn't get to re-fill the cache it invalidated or something like that. The fact that it happens only for cross-volume things might also be explained that way - is there something ncpfs does when switching volumes that might trigger a cache invalidate for another volume (either on the client side or the server side - maybe the server tends to try to break the connection for the old volume when you start traversing a new one?) The generic page cache for symlinks code does _not_ support invalidating the cache while it's being used. A local filesystem will obviously never invalidate the cache at all. Hmm.. NFS _does_ use the page cache for symlinks, but uses it slightly differently: instead of relying on the page cache entry being the same when freeing the page, it just caches the page it looked up in the page cache (ie nfs_follow_link() does look up the page cache, but then hides the page pointer inside the page data itself (uglee), and thus does not depend on the mapping staying the same (nfs_put_link() just takes the page from the symlink data). Linus - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, Aug 19, 2005 at 09:07:35AM -0700, Linus Torvalds wrote: Hmm.. NFS _does_ use the page cache for symlinks, but uses it slightly differently: instead of relying on the page cache entry being the same when freeing the page, it just caches the page it looked up in the page cache (ie nfs_follow_link() does look up the page cache, but then hides the page pointer inside the page data itself (uglee), and thus does not depend on the mapping staying the same (nfs_put_link() just takes the page from the symlink data). For NFS that was done exactly to deal with cache invalidation. IIRC, I've convinced myself that it wasn't going to happen on ncpfs and happily abstained from duplicating the NFS variant. - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, 19 Aug 2005, Linus Torvalds wrote: The generic page cache for symlinks code does _not_ support invalidating the cache while it's being used. A local filesystem will obviously never invalidate the cache at all. Hmm.. NFS _does_ use the page cache for symlinks [..] Looking more and more at this, I'm convinced this is it. Basically, page_follow_link_light() and page_put_link() depend on the fact that the page in the page cache is the same one the whole time: page_follow_link_light() will increment the page count of the page it finds at offset 0, and page_put_link() will decrement it. If the page has changed, they increment/decrement different pages. There's two ways to fix this: - document this as a fundamental fact, and apply the ncpfs patch. Local filesystems can still continue to use the generic helper functions (all other users _are_ local filesystems). - make nameidata contain not just the virtual addresses of the names, but also have a struct page *pages[MAX_NESTED_LINKS + 1], and save away the page there. That will fix ncpfs, and we could then make NFS also use the generic routines. I suspect that #1 is the prudent one. We have a patch already, and we don't want to grow nameidata. I'll commit a comment at the head of page_follow_link_light() too. Linus - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, 19 Aug 2005, Linus Torvalds wrote:
- document this as a fundamental fact, and apply the ncpfs patch. Local
filesystems can still continue to use the generic helper functions
(all other users _are_ local filesystems).
Actually, looking at the ncpfs patch, I'd rather not apply that patch
as-is. It looks like it will totally disable symlink caching, which would
be kind of sad. Somebody willing to do the same thing NFS does?
NFS hides away the struct page * pointer at the end of the page data, so
that when we pass the pointer to the virtual address around, we can
trivially look up the struct page.
An alternative is to make the symlink address-space use a gfp_mask of
GFP_KERNEL (no highmem), at which point ncpfs_put_link() just becomes
something like
void ncpfs_put_link(struct dentry *dentry, struct nameidata *nd)
{
char *addr = nd_get_link(nd);
if (!IS_ERR(addr))
page_cache_release(virt_to_page(addr));
}
which is pretty ugly, but even simpler than the NFS trick.
Anybody?
Linus
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, Aug 19, 2005 at 09:43:17AM -0700, Linus Torvalds wrote:
Actually, looking at the ncpfs patch, I'd rather not apply that patch
as-is. It looks like it will totally disable symlink caching, which would
be kind of sad. Somebody willing to do the same thing NFS does?
NFS hides away the struct page * pointer at the end of the page data, so
that when we pass the pointer to the virtual address around, we can
trivially look up the struct page.
An alternative is to make the symlink address-space use a gfp_mask of
GFP_KERNEL (no highmem), at which point ncpfs_put_link() just becomes
something like
void ncpfs_put_link(struct dentry *dentry, struct nameidata *nd)
{
char *addr = nd_get_link(nd);
if (!IS_ERR(addr))
page_cache_release(virt_to_page(addr));
}
which is pretty ugly, but even simpler than the NFS trick.
Anybody?
I'm taking NFS helpers to libfs.c and switching ncpfs to them. IMO that's
better than copying the damn thing and other network filesystems might have
the same needs eventually...
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, Aug 19, 2005 at 05:53:32PM +0100, Al Viro wrote:
I'm taking NFS helpers to libfs.c and switching ncpfs to them. IMO that's
better than copying the damn thing and other network filesystems might have
the same needs eventually...
[something like this - completely untested]
* stray_page_get_link(inode, filler) - returns ERR_PTR(error) or pointer
to symlink body. Said symlink body sits in a page at offset equal to
offsetof(page, struct stray_page_link). filler() is expected to put it
at such offset. Page is cached.
* stray_page_put_link() - -put_link() suitable for links obtained from
stray_page_get_link(). Unlike the usual pagecache-based variants, this
sucker does _not_ rely on page staying cached.
* nfs and ncpfs switched to the helpers above.
Signed-off-by: Al Viro [EMAIL PROTECTED]
diff -urN RC14-rc6-git10-base/fs/libfs.c current/fs/libfs.c
--- RC13-rc6-git10-base/fs/libfs.c 2005-08-10 10:37:52.0 -0400
+++ current/fs/libfs.c 2005-08-19 13:29:39.0 -0400
@@ -7,6 +7,7 @@
#include linux/pagemap.h
#include linux/mount.h
#include linux/vfs.h
+#include linux/namei.h
#include asm/uaccess.h
int simple_getattr(struct vfsmount *mnt, struct dentry *dentry,
@@ -616,6 +617,42 @@
return ret;
}
+char *stray_page_get_link(struct inode *inode, int (*filler)(struct inode *,
struct page *))
+{
+ struct stray_page_symlink *p;
+ struct page *page;
+
+ page = read_cache_page(inode-i_data, 0, (filler_t *)filler, inode);
+ if (IS_ERR(page))
+ goto read_failed;
+ if (!PageUptodate(page))
+ goto getlink_read_error;
+ p = kmap(page);
+ p-page = page;
+ return p-body;
+
+getlink_read_error:
+ page_cache_release(page);
+ page = ERR_PTR(-EIO);
+read_failed:
+ return (char *)page;/* error */
+}
+
+void stray_page_put_link(struct dentry *dentry, struct nameidata *nd)
+{
+ char *s = nd_get_link(nd);
+ if (!IS_ERR(s)) {
+ struct stray_page_symlink *p;
+ struct page *page;
+
+ p = container_of(s, struct stray_page_symlink, body[0]);
+ page = p-page;
+
+ kunmap(page);
+ page_cache_release(page);
+ }
+}
+
EXPORT_SYMBOL(dcache_dir_close);
EXPORT_SYMBOL(dcache_dir_lseek);
EXPORT_SYMBOL(dcache_dir_open);
@@ -648,3 +685,5 @@
EXPORT_SYMBOL_GPL(simple_attr_close);
EXPORT_SYMBOL_GPL(simple_attr_read);
EXPORT_SYMBOL_GPL(simple_attr_write);
+EXPORT_SYMBOL(stray_page_get_link);
+EXPORT_SYMBOL(stray_page_put_link);
diff -urN RC13-rc6-git10-base/fs/ncpfs/inode.c current/fs/ncpfs/inode.c
--- RC13-rc6-git10-base/fs/ncpfs/inode.c2005-06-17 15:48:29.0
-0400
+++ current/fs/ncpfs/inode.c2005-08-19 13:12:01.0 -0400
@@ -104,7 +104,7 @@
extern struct dentry_operations ncp_root_dentry_operations;
#if defined(CONFIG_NCPFS_EXTRAS) || defined(CONFIG_NCPFS_NFS_NS)
-extern struct address_space_operations ncp_symlink_aops;
+extern int ncp_follow_link(struct dentry *dentry, struct nameidata *nd);
extern int ncp_symlink(struct inode*, struct dentry*, const char*);
#endif
@@ -233,8 +233,8 @@
#if defined(CONFIG_NCPFS_EXTRAS) || defined(CONFIG_NCPFS_NFS_NS)
static struct inode_operations ncp_symlink_inode_operations = {
.readlink = generic_readlink,
- .follow_link= page_follow_link_light,
- .put_link = page_put_link,
+ .follow_link= ncp_follow_link,
+ .put_link = stray_page_put_link,
.setattr= ncp_notify_change,
};
#endif
@@ -272,7 +272,6 @@
#if defined(CONFIG_NCPFS_EXTRAS) || defined(CONFIG_NCPFS_NFS_NS)
} else if (S_ISLNK(inode-i_mode)) {
inode-i_op = ncp_symlink_inode_operations;
- inode-i_data.a_ops = ncp_symlink_aops;
#endif
} else {
make_bad_inode(inode);
diff -urN RC13-rc6-git10-base/fs/ncpfs/symlink.c current/fs/ncpfs/symlink.c
--- RC13-rc6-git10-base/fs/ncpfs/symlink.c 2005-06-17 15:48:29.0
-0400
+++ current/fs/ncpfs/symlink.c 2005-08-19 13:26:26.0 -0400
@@ -30,6 +30,7 @@
#include linux/time.h
#include linux/mm.h
#include linux/stat.h
+#include linux/namei.h
#include ncplib_kernel.h
@@ -41,9 +42,8 @@
/* - read a symbolic link -- */
-static int ncp_symlink_readpage(struct file *file, struct page *page)
+static int symlink_filler(struct inode *inode, struct page *page)
{
- struct inode *inode = page-mapping-host;
int error, length, len;
char *link, *rawlink;
char *buf = kmap(page);
@@ -77,6 +77,7 @@
}
len = NCP_MAX_SYMLINK_SIZE;
+ buf += offsetof(struct stray_page_symlink, body);
error = ncp_vol2io(NCP_SERVER(inode), buf, len, link, length, 0);
kfree(rawlink);
if (error)
@@ -96,13 +97,12 @@
return error;
}
-/*
- *
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, Aug 19, 2005 at 07:02:18PM +0100, Al Viro wrote: On Fri, Aug 19, 2005 at 05:53:32PM +0100, Al Viro wrote: I'm taking NFS helpers to libfs.c and switching ncpfs to them. IMO that's better than copying the damn thing and other network filesystems might have the same needs eventually... [something like this - completely untested] * stray_page_get_link(inode, filler) - returns ERR_PTR(error) or pointer to symlink body. Said symlink body sits in a page at offset equal to offsetof(page, struct stray_page_link). filler() is expected to put it at such offset. Page is cached. * stray_page_put_link() - -put_link() suitable for links obtained from stray_page_get_link(). Unlike the usual pagecache-based variants, this sucker does _not_ rely on page staying cached. * nfs and ncpfs switched to the helpers above. Can you add some kerneldoc comments to describe them? Especially as the name is not very descriptive. - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
Al Viro wrote: On Fri, Aug 19, 2005 at 05:53:32PM +0100, Al Viro wrote: I'm taking NFS helpers to libfs.c and switching ncpfs to them. IMO that's better than copying the damn thing and other network filesystems might have the same needs eventually... [something like this - completely untested] * stray_page_get_link(inode, filler) - returns ERR_PTR(error) or pointer to symlink body. Said symlink body sits in a page at offset equal to offsetof(page, struct stray_page_link). filler() is expected to put it at such offset. Page is cached. * stray_page_put_link() - -put_link() suitable for links obtained from stray_page_get_link(). Unlike the usual pagecache-based variants, this sucker does _not_ rely on page staying cached. * nfs and ncpfs switched to the helpers above. Signed-off-by: Al Viro [EMAIL PROTECTED] Just out of curiosity - what protects even local filesystems against concurrent truncate and symlink resolving when using the page cache helpers? --Mika - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, Aug 19, 2005 at 07:00:38PM +0100, Christoph Hellwig wrote: On Fri, Aug 19, 2005 at 07:02:18PM +0100, Al Viro wrote: On Fri, Aug 19, 2005 at 05:53:32PM +0100, Al Viro wrote: I'm taking NFS helpers to libfs.c and switching ncpfs to them. IMO that's better than copying the damn thing and other network filesystems might have the same needs eventually... [something like this - completely untested] * stray_page_get_link(inode, filler) - returns ERR_PTR(error) or pointer to symlink body. Said symlink body sits in a page at offset equal to offsetof(page, struct stray_page_link). filler() is expected to put it at such offset. Page is cached. * stray_page_put_link() - -put_link() suitable for links obtained from stray_page_get_link(). Unlike the usual pagecache-based variants, this sucker does _not_ rely on page staying cached. * nfs and ncpfs switched to the helpers above. Can you add some kerneldoc comments to describe them? Especially as the name is not very descriptive. Hey, if anybody has suggestions on names - they are very welcome ;-) FWIW, I'd rather take page_symlink(), page_symlink_inode_operations, page_put_link(), page_follow_link_light(), page_readlink(), page_getlink(), generic_readlink() and vfs_readlink() to the same place where these guys would live. They all belong together and none of them has any business in fs/namei.c. Options: fs/libfs.c or separate library since fs/libfs.c is getting crowded. Linus, do you have any objections to that or suggestions on filename here? - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, Aug 19, 2005 at 08:38:34PM +0100, Al Viro wrote: FWIW, I'd rather take page_symlink(), page_symlink_inode_operations, page_put_link(), page_follow_link_light(), page_readlink(), page_getlink(), generic_readlink() and vfs_readlink() to the same place where these guys would live. They all belong together and none of them has any business in fs/namei.c. Options: fs/libfs.c or separate library since fs/libfs.c is getting crowded. Linus, do you have any objections to that or suggestions on filename here? fs/symlink.c? -- Next the statesmen will invent cheap lies, putting the blame upon the nation that is attacked, and every man will be glad of those conscience-soothing falsities, and will diligently study them, and refuse to examine any refutations of them; and thus he will by and by convince himself that the war is just, and will thank God for the better sleep he enjoys after this process of grotesque self-deception. -- Mark Twain - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
Al Viro wrote: On Fri, Aug 19, 2005 at 10:16:47PM +0300, Mika Penttilä wrote: Just out of curiosity - what protects even local filesystems against concurrent truncate and symlink resolving when using the page cache helpers? How do you get truncate(2) or ftruncate(2) to do something with a symlink? The former follows links, the latter takes an open file... - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ Yes that is right, there is no way to invalidate the symlink inode mapping page(s) from user space. --Mika - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, 19 Aug 2005, Al Viro wrote:
FWIW, I'd rather take page_symlink(), page_symlink_inode_operations,
page_put_link(), page_follow_link_light(), page_readlink(), page_getlink(),
generic_readlink() and vfs_readlink() to the same place where these guys
would live. They all belong together and none of them has any business
in fs/namei.c. Options: fs/libfs.c or separate library since fs/libfs.c
is getting crowded. Linus, do you have any objections to that or suggestions
on filename here?
I'm not sure if this merits a new file or new organization (hey,
fs/lib/xxx might be good in theory). In particular, I had actually been
hoping to release 2.6.13 today, but this seems like a valid thing to hold
things up for - but not if we're going to re-organize things.
The one thing that strikes me is that we might actually have less pain if
we just changed the calling convention for follow_link/put_link slightly
instead of creating a new library function. The existing page cache
thing really _does_ work very well, and would work fine for NFS and ncpfs
too, if we just allowed an extra cookie to be passed along from
follow_link() to put_link().
A patch like this (totally untested, and you'd need to update any
filesystems that don't use the regular page_follow_link interface) would
seem to clean up the mess nicely.. The basic change is that follow_link()
returns a error-pointer cookie instead of just zero or error, and that is
passed into put_link().
That simple calling convention change solves all problems. It so _happens_
that any old binary code also continues to work (the cookie will be zero,
and put_link will ignore it), so it shouldn't even break any unconverted
stuff (unless they mix using their own functions _and_ using the helpher
functions, which is of course possible).
The shouldn't break nonconverted filesystems makes me think this is a
safe change. Comments?
NOTE NOTE NOTE! Let me say again that it's untested. It might not break
nonconverted filesystems, but it equally well migth break even the
converted ones ;)
Linus
diff --git a/fs/namei.c b/fs/namei.c
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -501,6 +501,7 @@ struct path {
static inline int __do_follow_link(struct path *path, struct nameidata *nd)
{
int error;
+ void *cookie;
struct dentry *dentry = path-dentry;
touch_atime(path-mnt, dentry);
@@ -508,13 +509,15 @@ static inline int __do_follow_link(struc
if (path-mnt == nd-mnt)
mntget(path-mnt);
- error = dentry-d_inode-i_op-follow_link(dentry, nd);
- if (!error) {
+ cookie = dentry-d_inode-i_op-follow_link(dentry, nd);
+ error = PTR_ERR(cookie);
+ if (!IS_ERR(cookie)) {
char *s = nd_get_link(nd);
+ error = 0;
if (s)
error = __vfs_follow_link(nd, s);
if (dentry-d_inode-i_op-put_link)
- dentry-d_inode-i_op-put_link(dentry, nd);
+ dentry-d_inode-i_op-put_link(dentry, nd, cookie);
}
dput(dentry);
mntput(path-mnt);
@@ -2345,14 +2348,17 @@ int generic_readlink(struct dentry *dent
{
struct nameidata nd;
int res;
+ void *cookie;
+
nd.depth = 0;
- res = dentry-d_inode-i_op-follow_link(dentry, nd);
- if (!res) {
+ cookie = dentry-d_inode-i_op-follow_link(dentry, nd);
+ if (!IS_ERR(cookie)) {
res = vfs_readlink(dentry, buffer, buflen, nd_get_link(nd));
if (dentry-d_inode-i_op-put_link)
- dentry-d_inode-i_op-put_link(dentry, nd);
+ dentry-d_inode-i_op-put_link(dentry, nd, cookie);
+ cookie = ERR_PTR(0);
}
- return res;
+ return PTR_ERR(cookie);
}
int vfs_follow_link(struct nameidata *nd, const char *link)
@@ -2395,23 +2401,19 @@ int page_readlink(struct dentry *dentry,
return res;
}
-int page_follow_link_light(struct dentry *dentry, struct nameidata *nd)
+void *page_follow_link_light(struct dentry *dentry, struct nameidata *nd)
{
struct page *page;
nd_set_link(nd, page_getlink(dentry, page));
- return 0;
+ return page;
}
-void page_put_link(struct dentry *dentry, struct nameidata *nd)
+void page_put_link(struct dentry *dentry, struct nameidata *nd, void *cookie)
{
if (!IS_ERR(nd_get_link(nd))) {
- struct page *page;
- page = find_get_page(dentry-d_inode-i_mapping, 0);
- if (!page)
- BUG();
+ struct page *page = cookie;
kunmap(page);
page_cache_release(page);
- page_cache_release(page);
}
}
diff --git a/include/linux/fs.h b/include/linux/fs.h
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -993,8 +993,8 @@ struct inode_operations {
int (*rename) (struct inode *,
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, 19 Aug 2005, Linus Torvalds wrote: On Fri, 19 Aug 2005, Linus Torvalds wrote: - document this as a fundamental fact, and apply the ncpfs patch. Local filesystems can still continue to use the generic helper functions (all other users _are_ local filesystems). Actually, looking at the ncpfs patch, I'd rather not apply that patch as-is. It looks like it will totally disable symlink caching, which would be kind of sad. Somebody willing to do the same thing NFS does? It does disable link caching. But I didn't make this up. This is exactly what smbfs uses. I just copied smbfs given ncpfs copies almost everything smbfs does anyway... Best regards, Anton -- Anton Altaparmakov aia21 at cam.ac.uk (replace at with @) Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net WWW: http://linux-ntfs.sf.net/ http://www-stu.christs.cam.ac.uk/~aia21/ - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, 19 Aug 2005, Anton Altaparmakov wrote: It does disable link caching. But I didn't make this up. This is exactly what smbfs uses. I just copied smbfs given ncpfs copies almost everything smbfs does anyway... Can you test whether the untested test-patch I sent out seems to work for your case that bugs out? You'll probably get a few new initialization from incompatible pointer type warnings, but I do believe that they should be harmless at least on normal architectures. Linus - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, Aug 19, 2005 at 08:43:06PM +0100, Al Viro wrote: On Fri, Aug 19, 2005 at 08:41:29PM +0100, Matthew Wilcox wrote: is getting crowded. Linus, do you have any objections to that or suggestions on filename here? fs/symlink.c? Or fs/lib/symlink.c... That's a very good idea. We were in need of fs/lib/ for a long time to unwind all the generic routines of the VFS logic. - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, 19 Aug 2005, Linus Torvalds wrote:
The one thing that strikes me is that we might actually have less pain if
we just changed the calling convention for follow_link/put_link slightly
instead of creating a new library function. The existing page cache
thing really _does_ work very well, and would work fine for NFS and ncpfs
too, if we just allowed an extra cookie to be passed along from
follow_link() to put_link().
A patch like this (totally untested, and you'd need to update any
filesystems that don't use the regular page_follow_link interface) would
seem to clean up the mess nicely.. The basic change is that follow_link()
returns a error-pointer cookie instead of just zero or error, and that is
passed into put_link().
That simple calling convention change solves all problems. It so _happens_
that any old binary code also continues to work (the cookie will be zero,
and put_link will ignore it), so it shouldn't even break any unconverted
stuff (unless they mix using their own functions _and_ using the helpher
functions, which is of course possible).
The shouldn't break nonconverted filesystems makes me think this is a
safe change. Comments?
NOTE NOTE NOTE! Let me say again that it's untested. It might not break
nonconverted filesystems, but it equally well migth break even the
converted ones ;)
Linus
diff --git a/fs/namei.c b/fs/namei.c
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -501,6 +501,7 @@ struct path {
static inline int __do_follow_link(struct path *path, struct nameidata *nd)
{
int error;
+ void *cookie;
struct dentry *dentry = path-dentry;
touch_atime(path-mnt, dentry);
@@ -508,13 +509,15 @@ static inline int __do_follow_link(struc
if (path-mnt == nd-mnt)
mntget(path-mnt);
- error = dentry-d_inode-i_op-follow_link(dentry, nd);
- if (!error) {
+ cookie = dentry-d_inode-i_op-follow_link(dentry, nd);
+ error = PTR_ERR(cookie);
+ if (!IS_ERR(cookie)) {
char *s = nd_get_link(nd);
+ error = 0;
if (s)
error = __vfs_follow_link(nd, s);
if (dentry-d_inode-i_op-put_link)
- dentry-d_inode-i_op-put_link(dentry, nd);
+ dentry-d_inode-i_op-put_link(dentry, nd, cookie);
}
dput(dentry);
mntput(path-mnt);
@@ -2345,14 +2348,17 @@ int generic_readlink(struct dentry *dent
{
struct nameidata nd;
int res;
+ void *cookie;
+
nd.depth = 0;
- res = dentry-d_inode-i_op-follow_link(dentry, nd);
- if (!res) {
+ cookie = dentry-d_inode-i_op-follow_link(dentry, nd);
+ if (!IS_ERR(cookie)) {
res = vfs_readlink(dentry, buffer, buflen, nd_get_link(nd));
if (dentry-d_inode-i_op-put_link)
- dentry-d_inode-i_op-put_link(dentry, nd);
+ dentry-d_inode-i_op-put_link(dentry, nd, cookie);
+ cookie = ERR_PTR(0);
You are throwing away the error return from vfs_readlink(). I suspect you
wanted:
+ cookie = ERR_PTR(res);
}
- return res;
+ return PTR_ERR(cookie);
}
int vfs_follow_link(struct nameidata *nd, const char *link)
@@ -2395,23 +2401,19 @@ int page_readlink(struct dentry *dentry,
return res;
}
-int page_follow_link_light(struct dentry *dentry, struct nameidata *nd)
+void *page_follow_link_light(struct dentry *dentry, struct nameidata *nd)
{
struct page *page;
nd_set_link(nd, page_getlink(dentry, page));
- return 0;
+ return page;
}
-void page_put_link(struct dentry *dentry, struct nameidata *nd)
+void page_put_link(struct dentry *dentry, struct nameidata *nd, void *cookie)
{
if (!IS_ERR(nd_get_link(nd))) {
- struct page *page;
- page = find_get_page(dentry-d_inode-i_mapping, 0);
- if (!page)
- BUG();
+ struct page *page = cookie;
kunmap(page);
page_cache_release(page);
- page_cache_release(page);
}
}
diff --git a/include/linux/fs.h b/include/linux/fs.h
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -993,8 +993,8 @@ struct inode_operations {
int (*rename) (struct inode *, struct dentry *,
struct inode *, struct dentry *);
int (*readlink) (struct dentry *, char __user *,int);
- int (*follow_link) (struct dentry *, struct nameidata *);
- void (*put_link) (struct dentry *, struct nameidata *);
+ void * (*follow_link) (struct dentry *, struct nameidata *);
+ void (*put_link) (struct dentry *, struct nameidata *, void *);
void (*truncate) (struct inode *);
int (*permission) (struct inode *, int, struct nameidata *);
int (*setattr) (struct dentry *, struct iattr *);
@@ -1602,8 +1602,8 @@ extern struct
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, 19 Aug 2005, Anton Altaparmakov wrote:
Yes, sure. I have applied your patch to our 2.6.11.4 tree (with the one
liner change I emailed you just now) and have kicked off a compile.
Actually, hold on. The original patch had another problem: it returned an
uninitialized page pointer when page_getlink() failed.
This one should have that fixed, and has converted a few other
filesystems. Most of them trivially, but I took the opportunity to just
simplify NFS while I was at it, since it now has no reason to need to save
off the struct page * any more.
It's still not tested, but at least I've looked at it a bit more ;)
Linus
---
diff --git a/fs/autofs/symlink.c b/fs/autofs/symlink.c
--- a/fs/autofs/symlink.c
+++ b/fs/autofs/symlink.c
@@ -12,11 +12,12 @@
#include autofs_i.h
-static int autofs_follow_link(struct dentry *dentry, struct nameidata *nd)
+/* Nothing to release.. */
+static void *autofs_follow_link(struct dentry *dentry, struct nameidata *nd)
{
char *s=((struct autofs_symlink *)dentry-d_inode-u.generic_ip)-data;
nd_set_link(nd, s);
- return 0;
+ return NULL;
}
struct inode_operations autofs_symlink_inode_operations = {
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -83,8 +83,8 @@ extern int cifs_dir_notify(struct file *
extern struct dentry_operations cifs_dentry_ops;
/* Functions related to symlinks */
-extern int cifs_follow_link(struct dentry *direntry, struct nameidata *nd);
-extern void cifs_put_link(struct dentry *direntry, struct nameidata *nd);
+extern void *cifs_follow_link(struct dentry *direntry, struct nameidata *nd);
+extern void cifs_put_link(struct dentry *direntry, struct nameidata *nd, void
*);
extern int cifs_readlink(struct dentry *direntry, char __user *buffer,
int buflen);
extern int cifs_symlink(struct inode *inode, struct dentry *direntry,
diff --git a/fs/cifs/link.c b/fs/cifs/link.c
--- a/fs/cifs/link.c
+++ b/fs/cifs/link.c
@@ -92,7 +92,7 @@ cifs_hl_exit:
return rc;
}
-int
+void *
cifs_follow_link(struct dentry *direntry, struct nameidata *nd)
{
struct inode *inode = direntry-d_inode;
@@ -148,7 +148,7 @@ out:
out_no_free:
FreeXid(xid);
nd_set_link(nd, target_path);
- return 0;
+ return NULL;/* No cookie */
}
int
@@ -330,7 +330,7 @@ cifs_readlink(struct dentry *direntry, c
return rc;
}
-void cifs_put_link(struct dentry *direntry, struct nameidata *nd)
+void cifs_put_link(struct dentry *direntry, struct nameidata *nd, void *cookie)
{
char *p = nd_get_link(nd);
if (!IS_ERR(p))
diff --git a/fs/ext2/symlink.c b/fs/ext2/symlink.c
--- a/fs/ext2/symlink.c
+++ b/fs/ext2/symlink.c
@@ -21,11 +21,11 @@
#include xattr.h
#include linux/namei.h
-static int ext2_follow_link(struct dentry *dentry, struct nameidata *nd)
+static void *ext2_follow_link(struct dentry *dentry, struct nameidata *nd)
{
struct ext2_inode_info *ei = EXT2_I(dentry-d_inode);
nd_set_link(nd, (char *)ei-i_data);
- return 0;
+ return NULL;
}
struct inode_operations ext2_symlink_inode_operations = {
diff --git a/fs/ext3/symlink.c b/fs/ext3/symlink.c
--- a/fs/ext3/symlink.c
+++ b/fs/ext3/symlink.c
@@ -23,11 +23,11 @@
#include linux/namei.h
#include xattr.h
-static int ext3_follow_link(struct dentry *dentry, struct nameidata *nd)
+static void * ext3_follow_link(struct dentry *dentry, struct nameidata *nd)
{
struct ext3_inode_info *ei = EXT3_I(dentry-d_inode);
nd_set_link(nd, (char*)ei-i_data);
- return 0;
+ return NULL;
}
struct inode_operations ext3_symlink_inode_operations = {
diff --git a/fs/namei.c b/fs/namei.c
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -501,6 +501,7 @@ struct path {
static inline int __do_follow_link(struct path *path, struct nameidata *nd)
{
int error;
+ void *cookie;
struct dentry *dentry = path-dentry;
touch_atime(path-mnt, dentry);
@@ -508,13 +509,15 @@ static inline int __do_follow_link(struc
if (path-mnt == nd-mnt)
mntget(path-mnt);
- error = dentry-d_inode-i_op-follow_link(dentry, nd);
- if (!error) {
+ cookie = dentry-d_inode-i_op-follow_link(dentry, nd);
+ error = PTR_ERR(cookie);
+ if (!IS_ERR(cookie)) {
char *s = nd_get_link(nd);
+ error = 0;
if (s)
error = __vfs_follow_link(nd, s);
if (dentry-d_inode-i_op-put_link)
- dentry-d_inode-i_op-put_link(dentry, nd);
+ dentry-d_inode-i_op-put_link(dentry, nd, cookie);
}
dput(dentry);
mntput(path-mnt);
@@ -2344,15 +2347,17 @@ out:
int generic_readlink(struct dentry *dentry, char __user *buffer, int buflen)
{
struct nameidata nd;
- int res;
+ void *cookie;
+
nd.depth = 0;
-
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, Aug 19, 2005 at 03:04:52PM -0700, Linus Torvalds wrote: On Fri, 19 Aug 2005, Anton Altaparmakov wrote: Yes, sure. I have applied your patch to our 2.6.11.4 tree (with the one liner change I emailed you just now) and have kicked off a compile. Actually, hold on. The original patch had another problem: it returned an uninitialized page pointer when page_getlink() failed. This one should have that fixed, and has converted a few other filesystems. Most of them trivially, but I took the opportunity to just simplify NFS while I was at it, since it now has no reason to need to save off the struct page * any more. It's still not tested, but at least I've looked at it a bit more ;) That looks OK except for * jffs2 is b0rken (see patch in another mail) * afs, autofs4, befs, devfs, freevxfs, jffs2, jfs, ncpfs, procfs, smbfs, sysvfs, ufs, xfs - prototype change for -follow_link() * befs, smbfs, xfs - same for -put_link() * ncpfs fix is actually missing here Prototype changes are covered by patch below (incremental on top of your + jffs2 fix upthread). No ncpfs changes - these will go separately, assuming you haven't done them yet; just a plain janitor stuff. - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Sat, Aug 20, 2005 at 12:15:42AM +0100, Al Viro wrote:
That looks OK except for
* jffs2 is b0rken (see patch in another mail)
* afs, autofs4, befs, devfs, freevxfs, jffs2, jfs, ncpfs, procfs,
smbfs, sysvfs, ufs, xfs - prototype change for -follow_link()
* befs, smbfs, xfs - same for -put_link()
* ncpfs fix is actually missing here
Prototype changes are covered by patch below (incremental on top of your +
jffs2 fix upthread). No ncpfs changes - these will go separately, assuming
you haven't done them yet; just a plain janitor stuff.
Gaack... And here's the patch itself - sorry.
diff -urN RC13-rc6-git10-base/fs/afs/mntpt.c current/fs/afs/mntpt.c
--- RC13-rc6-git10-base/fs/afs/mntpt.c 2005-06-17 15:48:29.0 -0400
+++ current/fs/afs/mntpt.c 2005-08-19 19:02:48.0 -0400
@@ -30,7 +30,7 @@
struct dentry *dentry,
struct nameidata *nd);
static int afs_mntpt_open(struct inode *inode, struct file *file);
-static int afs_mntpt_follow_link(struct dentry *dentry, struct nameidata *nd);
+static void *afs_mntpt_follow_link(struct dentry *dentry, struct nameidata
*nd);
struct file_operations afs_mntpt_file_operations = {
.open = afs_mntpt_open,
@@ -233,7 +233,7 @@
/*
* follow a link from a mountpoint directory, thus causing it to be mounted
*/
-static int afs_mntpt_follow_link(struct dentry *dentry, struct nameidata *nd)
+static void *afs_mntpt_follow_link(struct dentry *dentry, struct nameidata *nd)
{
struct vfsmount *newmnt;
struct dentry *old_dentry;
@@ -249,7 +249,7 @@
newmnt = afs_mntpt_do_automount(dentry);
if (IS_ERR(newmnt)) {
path_release(nd);
- return PTR_ERR(newmnt);
+ return (void *)newmnt;
}
old_dentry = nd-dentry;
@@ -267,7 +267,7 @@
}
kleave( = %d, err);
- return err;
+ return ERR_PTR(err);
} /* end afs_mntpt_follow_link() */
/*/
diff -urN RC13-rc6-git10-base/fs/autofs4/symlink.c current/fs/autofs4/symlink.c
--- RC13-rc6-git10-base/fs/autofs4/symlink.c2005-06-17 15:48:29.0
-0400
+++ current/fs/autofs4/symlink.c2005-08-19 19:03:11.0 -0400
@@ -12,11 +12,11 @@
#include autofs_i.h
-static int autofs4_follow_link(struct dentry *dentry, struct nameidata *nd)
+static void *autofs4_follow_link(struct dentry *dentry, struct nameidata *nd)
{
struct autofs_info *ino = autofs4_dentry_ino(dentry);
nd_set_link(nd, (char *)ino-u.symlink);
- return 0;
+ return NULL;
}
struct inode_operations autofs4_symlink_inode_operations = {
diff -urN RC13-rc6-git10-base/fs/befs/linuxvfs.c current/fs/befs/linuxvfs.c
--- RC13-rc6-git10-base/fs/befs/linuxvfs.c 2005-06-17 15:48:29.0
-0400
+++ current/fs/befs/linuxvfs.c 2005-08-19 19:03:52.0 -0400
@@ -41,8 +41,8 @@
static void befs_destroy_inode(struct inode *inode);
static int befs_init_inodecache(void);
static void befs_destroy_inodecache(void);
-static int befs_follow_link(struct dentry *, struct nameidata *);
-static void befs_put_link(struct dentry *, struct nameidata *);
+static void *befs_follow_link(struct dentry *, struct nameidata *);
+static void befs_put_link(struct dentry *, struct nameidata *, void *);
static int befs_utf2nls(struct super_block *sb, const char *in, int in_len,
char **out, int *out_len);
static int befs_nls2utf(struct super_block *sb, const char *in, int in_len,
@@ -487,10 +487,10 @@
}
nd_set_link(nd, link);
- return 0;
+ return NULL;
}
-static void befs_put_link(struct dentry *dentry, struct nameidata *nd)
+static void befs_put_link(struct dentry *dentry, struct nameidata *nd, void *p)
{
befs_inode_info *befs_ino = BEFS_I(dentry-d_inode);
if (befs_ino-i_flags BEFS_LONG_SYMLINK) {
diff -urN RC13-rc6-git10-base/fs/devfs/base.c current/fs/devfs/base.c
--- RC13-rc6-git10-base/fs/devfs/base.c 2005-06-17 15:48:29.0 -0400
+++ current/fs/devfs/base.c 2005-08-19 19:04:17.0 -0400
@@ -2491,11 +2491,11 @@
return 0;
} /* End Function devfs_mknod */
-static int devfs_follow_link(struct dentry *dentry, struct nameidata *nd)
+static void *devfs_follow_link(struct dentry *dentry, struct nameidata *nd)
{
struct devfs_entry *p = get_devfs_entry_from_vfs_inode(dentry-d_inode);
nd_set_link(nd, p ? p-u.symlink.linkname : ERR_PTR(-ENODEV));
- return 0;
+ return NULL;
} /* End Function devfs_follow_link */
static struct inode_operations devfs_iops = {
diff -urN RC13-rc6-git10-base/fs/freevxfs/vxfs_immed.c
current/fs/freevxfs/vxfs_immed.c
--- RC13-rc6-git10-base/fs/freevxfs/vxfs_immed.c2005-06-17
15:48:29.0
Re: Kernel bug: Bad page state: related to generic symlink code and mmap
On Fri, Aug 19, 2005 at 06:08:12PM -0700, Linus Torvalds wrote: On Sat, 20 Aug 2005, Al Viro wrote: That looks OK except for * ncpfs fix is actually missing here Well, the thing is, with the change to page_follow_link() and page_put_link(), ncpfs should now work fine - it doesn't need any fixing any more. Ah - right, it's using normal methods... - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
