Skip to contentSkip to conversation

d_path errors

PATCH Andrea Arcangeli 2 participants 3 messages Aug 6, 2004

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

there's some minor bug in the d_path handling (the nfsd one may not the the correct fix, there's no failure path for it, so I just terminate the string, and the last one in the audit subsystem is just a robustness cleanup if somebody will extend d_path in the future, right now it's a noop).

on a slightly different topic, Al, could you suggest how to hack d_path so that it provides an absolute path with respect to the init_task root directory?

d_path currently returns /meminfo for dentries from /proc, instead of /proc/meminfo. I need to build '/proc/meminfo' instead. I'm unsure if to hack d_path to do it internally, or if to work around d_path and to walk the mountpoints externally to it. I think the internal one is simpler though.

Index: linux-2.5/fs/compat.c
===================================================================
RCS file: /home/andrea/crypto/cvs/linux-2.5/fs/compat.c,v retrieving revision 1.35

fs/compat.c +2 -0
diff -u -p -r1.35 compat.c
@@ -429,6 +429,8 @@ asmlinkage long compat_sys_ioctl(unsigne
 			       		fn = d_path(filp->f_dentry,
 						filp->f_vfsmnt, path,
 						PAGE_SIZE);
+					if (IS_ERR(fn))
+						fn = "?";
 				}
 
 				sprintf(buf,"'%c'", (cmd>>24) & 0x3f);

Index: linux-2.5/fs/nfsd/export.c
===================================================================
RCS file: /home/andrea/crypto/cvs/linux-2.5/fs/nfsd/export.c,v retrieving revision 1.92

fs/nfsd/export.c +5 -0
diff -u -p -r1.92 export.c
@@ -294,6 +294,11 @@ void svc_export_request(struct cache_det
 
 	qword_add(bpp, blen, exp->ex_client->name);
 	pth = d_path(exp->ex_dentry, exp->ex_mnt, *bpp, *blen);
+	if (IS_ERR(pth)) {
+		/* is this correct? */
+		(*bpp)[0] = '\n';
+		return;
+	}
 	qword_add(bpp, blen, pth);
 	(*bpp)[-1] = '\n';
 }

Index: linux-2.5/kernel/audit.c
===================================================================
RCS file: /home/andrea/crypto/cvs/linux-2.5/kernel/audit.c,v retrieving revision 1.2

kernel/audit.c +1 -1
diff -u -p -r1.2 audit.c
@@ -708,7 +708,7 @@ void audit_log_d_path(struct audit_buffe
 		audit_log_move(ab);
 	avail = sizeof(ab->tmp) - ab->len;
 	p = d_path(dentry, vfsmnt, ab->tmp + ab->len, avail);
-	if (p == ERR_PTR(-ENAMETOOLONG)) {
+	if (IS_ERR(p)) {
 		/* FIXME: can we save some information here? */
 		audit_log_format(ab, "<toolong>");
 	} else {
#1 lore raw
Quoted 3 lines · Andrea Arcangeli

on a slightly different topic, Al, could you suggest how to hack d_path so that it provides an absolute path with respect to the init_task root directory?

Simple: you don't.
a) filesystem may be mounted more than once in init_task's namespace
b) filesystem may be not mounted there at all.
c) different subtrees of filessytem might be mounted there and
full tree might be not among them; your file might be covered by some of them.

What you are asking for is about as feasible as "I have an unlinked file. Show me a pathname of existing link to it". The best answer will be along the lines of "use find(1) if you are that desperate". And if vfsmount/dentry did *not* give you an absolute pathname, you are in situation equivalent to that.

No way in hell it's getting shoved in d_path() and I'm very sceptical about any code that really needs that.

#2 lore raw
Quoted 13 lines · On Fri, Aug 06, 2004 at 06:23:23PM +0100…
Quoted 3 lines · Andrea Arcangeli

on a slightly different topic, Al, could you suggest how to hack d_path so that it provides an absolute path with respect to the init_task root directory?

Simple: you don't.
a) filesystem may be mounted more than once in init_task's namespace
b) filesystem may be not mounted there at all.
c) different subtrees of filessytem might be mounted there and
full tree might be not among them; your file might be covered by some of them.

Quoted 8 lines

What you are asking for is about as feasible as "I have an unlinked file. Show me a pathname of existing link to it". The best answer will be along the lines of "use find(1) if you are that desperate". And if vfsmount/dentry did *not* give you an absolute pathname, you are in situation equivalent to that.

No way in hell it's getting shoved in d_path() and I'm very sceptical about any code that really needs that.

This is an heuristic. I doesn't need to be correct. But if a file is placed in /proc/meminfo I want to generate the string "/proc/meminfo" _not_ "/meminfo" like it happens right now. I'm fine to workaround the d_path walking the mountpoints.

See the current patch how it looks and what it does. This dumps into userspace all dcache accesses so I can preload all getdents and all read(2) into pagecache and buffercache during boot. The preload is actually much harder than the logging, not clever preload (i.e. just a loop that read everything from disk) will make the bootcache useless or it may even make the boot slower. But the preload is not covered in this patch, this patch is only about logging the fs metdata accesses into userspace. Hooking into the pagecache is a minor issue, it's worthless to preload pagecache offsets (especially if you use lseek and destroy readhaead) if we don't preload all dcache and buffercache first. I already use this (without any granularity on the pagecache accesses) and it reduces my boot time from grub to X of 10% (so I'm already benefiting from this feature on my own laptop in daily usage), I didn't yet try to cache kde as well, but before trying any further thing I'd like to polish the bugs like the fact I see "/meminfo" instead of "/proc/meminfo" or similar. I obviously need to filter out everything not filebacked (and I'm going to do that in userspace). And for the filebacked I need a path that the init task can load. Note, I wrote init_task below but really this should be current->fs, it's not going to make a difference in practice.

I'll be offline for the next two weeks, but I'll answer any email after I'm back.

thanks!

(this is very alpha code, just for measurements)

fs/proc/Makefile renamed from fs/proc/Makefile.~1~ +1 -1
@@ -8,7 +8,7 @@ proc-y			:= task_nommu.o
 proc-$(CONFIG_MMU)	:= task_mmu.o
 
 proc-y       += inode.o root.o base.o generic.o array.o \
-		kmsg.o proc_tty.o proc_misc.o
+		kmsg.o proc_tty.o proc_misc.o bootcache.o
 
 proc-$(CONFIG_PROC_KCORE)	+= kcore.o
 proc-$(CONFIG_PROC_DEVICETREE)	+= proc_devtree.o
fs/proc/proc_misc.c renamed from fs/proc/proc_misc.c.~1~ +3 -0
@@ -692,6 +692,9 @@ void __init proc_misc_init(void)
 	entry = create_proc_entry("kmsg", S_IRUSR, &proc_root);
 	if (entry)
 		entry->proc_fops = &proc_kmsg_operations;
+	entry = create_proc_entry("bootcache", S_IRUSR, &proc_root);
+	if (entry)
+		entry->proc_fops = &proc_bootcache_operations;
 	create_seq_entry("cpuinfo", 0, &proc_cpuinfo_operations);
 	create_seq_entry("partitions", 0, &proc_partitions_operations);
 	create_seq_entry("stat", 0, &proc_stat_operations);
fs/namei.c renamed from fs/namei.c.~1~ +9 -3
@@ -27,6 +27,7 @@
 #include <linux/personality.h>
 #include <linux/security.h>
 #include <linux/mount.h>
+#include <linux/bootcache.h>
 #include <asm/namei.h>
 #include <asm/uaccess.h>
 
@@ -398,8 +399,11 @@ again:
 			result = dir->i_op->lookup(dir, dentry, nd);
 			if (result)
 				dput(dentry);
-			else
+			else {
 				result = dentry;
+				if (unlikely(is_bootcache_enabled()))
+					bootcache_new_entry(dentry);
+			}
 		}
 		up(&dir->i_sem);
 		return result;
@@ -989,9 +993,11 @@ static struct dentry * __lookup_hash(str
 		if (!new)
 			goto out;
 		dentry = inode->i_op->lookup(inode, new, nd);
-		if (!dentry)
+		if (!dentry) {
 			dentry = new;
-		else
+			if (unlikely(is_bootcache_enabled()))
+				bootcache_new_entry(dentry);
+		} else
 			dput(new);
 	}
 out:
include/linux/proc_fs.h renamed from include/linux/proc_fs.h.~1~ +1 -0
@@ -115,6 +115,7 @@ extern struct file_operations proc_file_
 extern struct file_operations proc_kcore_operations;
 extern struct file_operations proc_kmsg_operations;
 extern struct file_operations ppc_htab_operations;
+extern struct file_operations proc_bootcache_operations;
 
 /*
  * proc_tty.c
fs/proc/bootcache.c new file +210 -0

210 lines not on this page · raw message

include/linux/bootcache.h new file +24 -0
@@ -0,0 +1,24 @@
+#ifndef _BOOTCACHE_H
+#define _BOOTCACHE_H
+
+#include <linux/config.h>
+#include <linux/dcache.h>
+#include <linux/kernel.h>
+
+#ifdef CONFIG_PROC_FS
+extern unsigned int bootcache_nr;
+
+static inline int is_bootcache_enabled(void)
+{
+	return bootcache_nr;
+}
+#else
+static inline int is_bootcache_enabled(void)
+{
+	return 0;
+}
+#endif
+
+extern void FASTCALL(bootcache_new_entry(struct dentry * dentry));
+
+#endif /* _BOOTCACHE_H */