* [PATCH 1/7] FS-Cache: Provide a filesystem-specific sync'able page bit
@ 2006-04-20 16:59 David Howells
2006-04-20 16:59 ` [PATCH 2/7] FS-Cache: Add notification of page becoming writable to VMA ops David Howells
` (6 more replies)
0 siblings, 7 replies; 29+ messages in thread
From: David Howells @ 2006-04-20 16:59 UTC (permalink / raw)
To: torvalds, akpm, steved, sct, aviro
Cc: linux-fsdevel, linux-cachefs, nfsv4, linux-kernel
The attached patch provides a filesystem-specific page bit that a filesystem
can synchronise upon. This can be used, for example, by a netfs to synchronise
with CacheFS writing its pages to disk.
Signed-Off-By: David Howells <dhowells@redhat.com>
---
include/linux/page-flags.h | 10 ++++++++++
include/linux/pagemap.h | 11 +++++++++++
mm/filemap.c | 17 +++++++++++++++++
3 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index d276a4e..5486874 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -74,6 +74,7 @@
#define PG_slab 7 /* slab debug (Suparna wants this) */
#define PG_checked 8 /* kill me in 2.5.<early>. */
+#define PG_fs_misc 8
#define PG_arch_1 9
#define PG_reserved 10
#define PG_private 11 /* Has something at ->private */
@@ -376,4 +377,13 @@ static inline void set_page_writeback(st
test_set_page_writeback(page);
}
+/*
+ * Filesystem-specific page bit testing
+ */
+#define PageFsMisc(page) test_bit(PG_fs_misc, &(page)->flags)
+#define SetPageFsMisc(page) set_bit(PG_fs_misc, &(page)->flags)
+#define TestSetPageFsMisc(page) test_and_set_bit(PG_fs_misc, &(page)->flags)
+#define ClearPageFsMisc(page) clear_bit(PG_fs_misc, &(page)->flags)
+#define TestClearPageFsMisc(page) test_and_clear_bit(PG_fs_misc, &(page)->flags)
+
#endif /* PAGE_FLAGS_H */
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 9539efd..02e7d8b 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -206,6 +206,17 @@ static inline void wait_on_page_writebac
extern void end_page_writeback(struct page *page);
/*
+ * Wait for filesystem-specific page synchronisation to complete
+ */
+static inline void wait_on_page_fs_misc(struct page *page)
+{
+ if (PageFsMisc(page))
+ wait_on_page_bit(page, PG_fs_misc);
+}
+
+extern void fastcall end_page_fs_misc(struct page *page);
+
+/*
* Fault a userspace page into pagetables. Return non-zero on a fault.
*
* This assumes that two userspace pages are always sufficient. That's
diff --git a/mm/filemap.c b/mm/filemap.c
index 3ef2073..d4a598f 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -545,6 +545,23 @@ void fastcall __lock_page(struct page *p
EXPORT_SYMBOL(__lock_page);
/*
+ * Note completion of filesystem specific page synchronisation
+ *
+ * This is used to allow a page to be written to a filesystem cache in the
+ * background without holding up the completion of readpage
+ */
+void fastcall end_page_fs_misc(struct page *page)
+{
+ smp_mb__before_clear_bit();
+ if (!TestClearPageFsMisc(page))
+ BUG();
+ smp_mb__after_clear_bit();
+ __wake_up_bit(page_waitqueue(page), &page->flags, PG_fs_misc);
+}
+
+EXPORT_SYMBOL(end_page_fs_misc);
+
+/*
* a rather lightweight function, finding and getting a reference to a
* hashed page atomically.
*/
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 2/7] FS-Cache: Add notification of page becoming writable to VMA ops
2006-04-20 16:59 [PATCH 1/7] FS-Cache: Provide a filesystem-specific sync'able page bit David Howells
@ 2006-04-20 16:59 ` David Howells
2006-04-20 17:40 ` Zach Brown
2006-04-20 16:59 ` [PATCH 3/7] FS-Cache: Avoid ENFILE checking for kernel-specific open files David Howells
` (5 subsequent siblings)
6 siblings, 1 reply; 29+ messages in thread
From: David Howells @ 2006-04-20 16:59 UTC (permalink / raw)
To: torvalds, akpm, steved, sct, aviro
Cc: linux-fsdevel, linux-cachefs, nfsv4, linux-kernel
The attached patch adds a new VMA operation to notify a filesystem or other
driver about the MMU generating a fault because userspace attempted to write
to a page mapped through a read-only PTE.
This facility permits the filesystem or driver to:
(*) Implement storage allocation/reservation on attempted write, and so to
deal with problems such as ENOSPC more gracefully (perhaps by generating
SIGBUS).
(*) Delay making the page writable until the contents have been written to a
backing cache. This is useful for NFS/AFS when using FS-Cache/CacheFS.
It permits the filesystem to have some guarantee about the state of the
cache.
(*) Account and limit number of dirty pages. This is one piece of the puzzle
needed to make shared writable mapping work safely in FUSE.
Signed-Off-By: David Howells <dhowells@redhat.com>
---
include/linux/mm.h | 4 ++
mm/memory.c | 99 +++++++++++++++++++++++++++++++++++++++-------------
mm/mmap.c | 12 +++++-
mm/mprotect.c | 11 +++++-
4 files changed, 98 insertions(+), 28 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 1154684..cd3c2cf 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -200,6 +200,10 @@ struct vm_operations_struct {
void (*close)(struct vm_area_struct * area);
struct page * (*nopage)(struct vm_area_struct * area, unsigned long address, int *type);
int (*populate)(struct vm_area_struct * area, unsigned long address, unsigned long len, pgprot_t prot, unsigned long pgoff, int nonblock);
+
+ /* notification that a previously read-only page is about to become
+ * writable, if an error is returned it will cause a SIGBUS */
+ int (*page_mkwrite)(struct vm_area_struct *vma, struct page *page);
#ifdef CONFIG_NUMA
int (*set_policy)(struct vm_area_struct *vma, struct mempolicy *new);
struct mempolicy *(*get_policy)(struct vm_area_struct *vma,
diff --git a/mm/memory.c b/mm/memory.c
index 0ec7bc6..6c6891e 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1445,25 +1445,59 @@ static int do_wp_page(struct mm_struct *
{
struct page *old_page, *new_page;
pte_t entry;
- int ret = VM_FAULT_MINOR;
+ int reuse, ret = VM_FAULT_MINOR;
old_page = vm_normal_page(vma, address, orig_pte);
if (!old_page)
goto gotten;
- if (PageAnon(old_page) && !TestSetPageLocked(old_page)) {
- int reuse = can_share_swap_page(old_page);
- unlock_page(old_page);
- if (reuse) {
- flush_cache_page(vma, address, pte_pfn(orig_pte));
- entry = pte_mkyoung(orig_pte);
- entry = maybe_mkwrite(pte_mkdirty(entry), vma);
- ptep_set_access_flags(vma, address, page_table, entry, 1);
- update_mmu_cache(vma, address, entry);
- lazy_mmu_prot_update(entry);
- ret |= VM_FAULT_WRITE;
- goto unlock;
+ if (unlikely(vma->vm_flags & VM_SHARED)) {
+ if (vma->vm_ops && vma->vm_ops->page_mkwrite) {
+ /*
+ * Notify the address space that the page is about to
+ * become writable so that it can prohibit this or wait
+ * for the page to get into an appropriate state.
+ *
+ * We do this without the lock held, so that it can
+ * sleep if it needs to.
+ */
+ page_cache_get(old_page);
+ pte_unmap_unlock(page_table, ptl);
+
+ if (vma->vm_ops->page_mkwrite(vma, old_page) < 0)
+ goto unwritable_page;
+
+ page_cache_release(old_page);
+
+ /*
+ * Since we dropped the lock we need to revalidate
+ * the PTE as someone else may have changed it. If
+ * they did, we just return, as we can count on the
+ * MMU to tell us if they didn't also make it writable.
+ */
+ page_table = pte_offset_map_lock(mm, pmd, address,
+ &ptl);
+ if (!pte_same(*page_table, orig_pte))
+ goto unlock;
}
+
+ reuse = 1;
+ } else if (PageAnon(old_page) && !TestSetPageLocked(old_page)) {
+ reuse = can_share_swap_page(old_page);
+ unlock_page(old_page);
+ } else {
+ reuse = 0;
+ }
+
+ if (reuse) {
+ flush_cache_page(vma, address, pte_pfn(orig_pte));
+ entry = pte_mkyoung(orig_pte);
+ entry = maybe_mkwrite(pte_mkdirty(entry), vma);
+ ptep_set_access_flags(vma, address, page_table, entry, 1);
+ update_mmu_cache(vma, address, entry);
+ lazy_mmu_prot_update(entry);
+ ret |= VM_FAULT_WRITE;
+ goto unlock;
}
/*
@@ -1523,6 +1557,10 @@ oom:
if (old_page)
page_cache_release(old_page);
return VM_FAULT_OOM;
+
+unwritable_page:
+ page_cache_release(old_page);
+ return VM_FAULT_SIGBUS;
}
/*
@@ -2074,18 +2112,31 @@ retry:
/*
* Should we do an early C-O-W break?
*/
- if (write_access && !(vma->vm_flags & VM_SHARED)) {
- struct page *page;
+ if (write_access) {
+ if (!(vma->vm_flags & VM_SHARED)) {
+ struct page *page;
- if (unlikely(anon_vma_prepare(vma)))
- goto oom;
- page = alloc_page_vma(GFP_HIGHUSER, vma, address);
- if (!page)
- goto oom;
- copy_user_highpage(page, new_page, address);
- page_cache_release(new_page);
- new_page = page;
- anon = 1;
+ if (unlikely(anon_vma_prepare(vma)))
+ goto oom;
+ page = alloc_page_vma(GFP_HIGHUSER, vma, address);
+ if (!page)
+ goto oom;
+ copy_user_highpage(page, new_page, address);
+ page_cache_release(new_page);
+ new_page = page;
+ anon = 1;
+
+ } else {
+ /* if the page will be shareable, see if the backing
+ * address space wants to know that the page is about
+ * to become writable */
+ if (vma->vm_ops->page_mkwrite &&
+ vma->vm_ops->page_mkwrite(vma, new_page) < 0
+ ) {
+ page_cache_release(new_page);
+ return VM_FAULT_SIGBUS;
+ }
+ }
}
page_table = pte_offset_map_lock(mm, pmd, address, &ptl);
diff --git a/mm/mmap.c b/mm/mmap.c
index e6ee123..6446c61 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1065,7 +1065,8 @@ munmap_back:
vma->vm_start = addr;
vma->vm_end = addr + len;
vma->vm_flags = vm_flags;
- vma->vm_page_prot = protection_map[vm_flags & 0x0f];
+ vma->vm_page_prot = protection_map[vm_flags &
+ (VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
vma->vm_pgoff = pgoff;
if (file) {
@@ -1089,6 +1090,12 @@ munmap_back:
goto free_vma;
}
+ /* Don't make the VMA automatically writable if it's shared, but the
+ * backer wishes to know when pages are first written to */
+ if (vma->vm_ops && vma->vm_ops->page_mkwrite)
+ vma->vm_page_prot =
+ protection_map[vm_flags & (VM_READ|VM_WRITE|VM_EXEC)];
+
/* We set VM_ACCOUNT in a shared mapping's vm_flags, to inform
* shmem_zero_setup (perhaps called through /dev/zero's ->mmap)
* that memory reservation must be checked; but that reservation
@@ -1921,7 +1928,8 @@ unsigned long do_brk(unsigned long addr,
vma->vm_end = addr + len;
vma->vm_pgoff = pgoff;
vma->vm_flags = flags;
- vma->vm_page_prot = protection_map[flags & 0x0f];
+ vma->vm_page_prot = protection_map[flags &
+ (VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
vma_link(mm, vma, prev, rb_link, rb_parent);
out:
mm->total_vm += len >> PAGE_SHIFT;
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 4c14d42..2697abd 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -106,6 +106,7 @@ mprotect_fixup(struct vm_area_struct *vm
unsigned long oldflags = vma->vm_flags;
long nrpages = (end - start) >> PAGE_SHIFT;
unsigned long charged = 0;
+ unsigned int mask;
pgprot_t newprot;
pgoff_t pgoff;
int error;
@@ -132,8 +133,6 @@ mprotect_fixup(struct vm_area_struct *vm
}
}
- newprot = protection_map[newflags & 0xf];
-
/*
* First try to merge with previous and/or next vma.
*/
@@ -160,6 +159,14 @@ mprotect_fixup(struct vm_area_struct *vm
}
success:
+ /* Don't make the VMA automatically writable if it's shared, but the
+ * backer wishes to know when pages are first written to */
+ mask = VM_READ|VM_WRITE|VM_EXEC|VM_SHARED;
+ if (vma->vm_ops && vma->vm_ops->page_mkwrite)
+ mask &= ~VM_SHARED;
+
+ newprot = protection_map[newflags & mask];
+
/*
* vm_flags and vm_page_prot are protected by the mmap_sem
* held in write mode.
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 3/7] FS-Cache: Avoid ENFILE checking for kernel-specific open files
2006-04-20 16:59 [PATCH 1/7] FS-Cache: Provide a filesystem-specific sync'able page bit David Howells
2006-04-20 16:59 ` [PATCH 2/7] FS-Cache: Add notification of page becoming writable to VMA ops David Howells
@ 2006-04-20 16:59 ` David Howells
2006-04-20 17:18 ` Christoph Hellwig
2006-04-21 0:07 ` Andrew Morton
2006-04-20 16:59 ` [PATCH 4/7] FS-Cache: Export find_get_pages() David Howells
` (4 subsequent siblings)
6 siblings, 2 replies; 29+ messages in thread
From: David Howells @ 2006-04-20 16:59 UTC (permalink / raw)
To: torvalds, akpm, steved, sct, aviro
Cc: linux-fsdevel, linux-cachefs, nfsv4, linux-kernel
Make it possible to avoid ENFILE checking for kernel specific open files, such
as are used by the CacheFiles module.
After, for example, tarring up a kernel source tree over the network, the
CacheFiles module may easily have 20000+ files open in the backing filesystem,
thus causing all non-root processes to be given error ENFILE when they try to
open a file, socket, pipe, etc..
Signed-Off-By: David Howells <dhowells@redhat.com>
---
arch/ia64/kernel/perfmon.c | 2 +-
drivers/infiniband/core/uverbs_main.c | 2 +-
fs/eventpoll.c | 2 +-
fs/file_table.c | 36 ++++++++++++++++++++++++---------
fs/hugetlbfs/inode.c | 2 +-
fs/inotify.c | 2 +-
fs/namei.c | 2 +-
fs/open.c | 22 +++++++++++++++++++-
fs/pipe.c | 4 ++--
include/linux/file.h | 1 -
include/linux/fs.h | 8 ++++++-
kernel/futex.c | 2 +-
kernel/sysctl.c | 2 +-
mm/shmem.c | 2 +-
mm/tiny-shmem.c | 2 +-
net/socket.c | 2 +-
16 files changed, 67 insertions(+), 26 deletions(-)
diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
index 077f212..f23ab3a 100644
--- a/arch/ia64/kernel/perfmon.c
+++ b/arch/ia64/kernel/perfmon.c
@@ -2162,7 +2162,7 @@ pfm_alloc_fd(struct file **cfile)
ret = -ENFILE;
- file = get_empty_filp();
+ file = get_empty_filp(0);
if (!file) goto out;
/*
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index ff092a0..4f7137c 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -525,7 +525,7 @@ struct file *ib_uverbs_alloc_event_file(
goto err;
}
- filp = get_empty_filp();
+ filp = get_empty_filp(0);
if (!filp) {
ret = -ENFILE;
goto err_fd;
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 1b4491c..f774038 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -714,7 +714,7 @@ static int ep_getfd(int *efd, struct ino
/* Get an ready to use file */
error = -ENFILE;
- file = get_empty_filp();
+ file = get_empty_filp(0);
if (!file)
goto eexit_1;
diff --git a/fs/file_table.c b/fs/file_table.c
index bcea199..300e7c2 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -34,6 +34,7 @@ struct files_stat_struct files_stat = {
__cacheline_aligned_in_smp DEFINE_SPINLOCK(files_lock);
static struct percpu_counter nr_files __cacheline_aligned_in_smp;
+static atomic_t nr_kernel_files;
static inline void file_free_rcu(struct rcu_head *head)
{
@@ -43,7 +44,10 @@ static inline void file_free_rcu(struct
static inline void file_free(struct file *f)
{
- percpu_counter_dec(&nr_files);
+ if (!(f->f_kernel_flags & FKFLAGS_KERNEL))
+ percpu_counter_dec(&nr_files);
+ else
+ atomic_dec(&nr_kernel_files);
call_rcu(&f->f_u.fu_rcuhead, file_free_rcu);
}
@@ -72,6 +76,7 @@ int proc_nr_files(ctl_table *table, int
void __user *buffer, size_t *lenp, loff_t *ppos)
{
files_stat.nr_files = get_nr_files();
+ files_stat.nr_kernel_files = atomic_read(&nr_kernel_files);
return proc_dointvec(table, write, filp, buffer, lenp, ppos);
}
#else
@@ -86,7 +91,7 @@ int proc_nr_files(ctl_table *table, int
* Returns NULL, if there are no more free file structures or
* we run out of memory.
*/
-struct file *get_empty_filp(void)
+struct file *get_empty_filp(int kernel)
{
struct task_struct *tsk;
static int old_max;
@@ -95,20 +100,29 @@ struct file *get_empty_filp(void)
/*
* Privileged users can go above max_files
*/
- if (get_nr_files() >= files_stat.max_files && !capable(CAP_SYS_ADMIN)) {
- /*
- * percpu_counters are inaccurate. Do an expensive check before
- * we go and fail.
- */
- if (percpu_counter_sum(&nr_files) >= files_stat.max_files)
- goto over;
+ if (!kernel) {
+ if (get_nr_files() >= files_stat.max_files &&
+ !capable(CAP_SYS_ADMIN)
+ ) {
+ /*
+ * percpu_counters are inaccurate. Do an expensive
+ * check before we go and fail.
+ */
+ if (percpu_counter_sum(&nr_files) >=
+ files_stat.max_files)
+ goto over;
+ }
}
f = kmem_cache_alloc(filp_cachep, GFP_KERNEL);
if (f == NULL)
goto fail;
- percpu_counter_inc(&nr_files);
+ if (!kernel)
+ percpu_counter_inc(&nr_files);
+ else
+ atomic_inc(&nr_kernel_files);
+
memset(f, 0, sizeof(*f));
if (security_file_alloc(f))
goto fail_sec;
@@ -117,6 +131,7 @@ struct file *get_empty_filp(void)
INIT_LIST_HEAD(&f->f_u.fu_list);
atomic_set(&f->f_count, 1);
rwlock_init(&f->f_owner.lock);
+ f->f_kernel_flags = kernel ? FKFLAGS_KERNEL : 0;
f->f_uid = tsk->fsuid;
f->f_gid = tsk->fsgid;
eventpoll_init_file(f);
@@ -235,6 +250,7 @@ struct file fastcall *fget_light(unsigne
return file;
}
+EXPORT_SYMBOL(fget_light);
void put_filp(struct file *file)
{
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 3a5b4e9..cc27ee8 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -770,7 +770,7 @@ struct file *hugetlb_zero_setup(size_t s
goto out_shm_unlock;
error = -ENFILE;
- file = get_empty_filp();
+ file = get_empty_filp(0);
if (!file)
goto out_dentry;
diff --git a/fs/inotify.c b/fs/inotify.c
index 1f50302..2e66e05 100644
--- a/fs/inotify.c
+++ b/fs/inotify.c
@@ -939,7 +939,7 @@ asmlinkage long sys_inotify_init(void)
if (fd < 0)
return fd;
- filp = get_empty_filp();
+ filp = get_empty_filp(0);
if (!filp) {
ret = -ENFILE;
goto out_put_fd;
diff --git a/fs/namei.c b/fs/namei.c
index 96723ae..6713213 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1146,7 +1146,7 @@ static int __path_lookup_intent_open(int
unsigned int lookup_flags, struct nameidata *nd,
int open_flags, int create_mode)
{
- struct file *filp = get_empty_filp();
+ struct file *filp = get_empty_filp(0);
int err;
if (filp == NULL)
diff --git a/fs/open.c b/fs/open.c
index 53ec28c..cea1538 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -962,7 +962,7 @@ struct file *dentry_open(struct dentry *
struct file *f;
error = -ENFILE;
- f = get_empty_filp();
+ f = get_empty_filp(0);
if (f == NULL) {
dput(dentry);
mntput(mnt);
@@ -974,6 +974,26 @@ struct file *dentry_open(struct dentry *
EXPORT_SYMBOL(dentry_open);
/*
+ * open a specifically in-kernel file
+ */
+struct file *dentry_open_kernel(struct dentry *dentry, struct vfsmount *mnt, int flags)
+{
+ int error;
+ struct file *f;
+
+ error = -ENFILE;
+ f = get_empty_filp(1);
+ if (f == NULL) {
+ dput(dentry);
+ mntput(mnt);
+ return ERR_PTR(error);
+ }
+
+ return __dentry_open(dentry, mnt, flags, f, NULL);
+}
+EXPORT_SYMBOL(dentry_open_kernel);
+
+/*
* Find an empty file descriptor entry, and mark it busy.
*/
int get_unused_fd(void)
diff --git a/fs/pipe.c b/fs/pipe.c
index 7fefb10..6081367 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -795,11 +795,11 @@ int do_pipe(int *fd)
int i, j;
error = -ENFILE;
- f1 = get_empty_filp();
+ f1 = get_empty_filp(0);
if (!f1)
goto no_files;
- f2 = get_empty_filp();
+ f2 = get_empty_filp(0);
if (!f2)
goto close_f1;
diff --git a/include/linux/file.h b/include/linux/file.h
index 9f7c251..da7be8f 100644
--- a/include/linux/file.h
+++ b/include/linux/file.h
@@ -79,7 +79,6 @@ extern void FASTCALL(set_close_on_exec(u
extern void put_filp(struct file *);
extern int get_unused_fd(void);
extern void FASTCALL(put_unused_fd(unsigned int fd));
-struct kmem_cache;
extern struct file ** alloc_fd_array(int);
extern void free_fd_array(struct file **, int);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3de2bfb..979b1d3 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -33,6 +33,7 @@ struct files_stat_struct {
int nr_files; /* read only */
int nr_free_files; /* read only */
int max_files; /* tunable */
+ int nr_kernel_files; /* read only */
};
extern struct files_stat_struct files_stat;
extern int get_max_files(void);
@@ -70,6 +71,8 @@ extern int dir_notify_enable;
behavior for cross-node execution/opening_for_writing of files */
#define FMODE_EXEC 16
+#define FKFLAGS_KERNEL 1 /* kernel internal file (not accounted) */
+
#define RW_MASK 1
#define RWA_MASK 2
#define READ 0
@@ -640,6 +643,7 @@ struct file {
atomic_t f_count;
unsigned int f_flags;
mode_t f_mode;
+ unsigned short f_kernel_flags;
loff_t f_pos;
struct fown_struct f_owner;
unsigned int f_uid, f_gid;
@@ -1377,6 +1381,7 @@ extern long do_sys_open(int fdf, const c
int mode);
extern struct file *filp_open(const char *, int, int);
extern struct file * dentry_open(struct dentry *, struct vfsmount *, int);
+extern struct file * dentry_open_kernel(struct dentry *, struct vfsmount *, int);
extern int filp_close(struct file *, fl_owner_t id);
extern char * getname(const char __user *);
@@ -1577,7 +1582,7 @@ static inline void insert_inode_hash(str
__insert_inode_hash(inode, inode->i_ino);
}
-extern struct file * get_empty_filp(void);
+extern struct file * get_empty_filp(int kernel);
extern void file_move(struct file *f, struct list_head *list);
extern void file_kill(struct file *f);
struct bio;
@@ -1603,6 +1608,7 @@ extern ssize_t generic_file_direct_write
unsigned long *, loff_t, loff_t *, size_t, size_t);
extern ssize_t generic_file_buffered_write(struct kiocb *, const struct iovec *,
unsigned long, loff_t, loff_t *, size_t, ssize_t);
+extern int generic_file_buffered_write_one_kernel_page(struct file *, pgoff_t, struct page *);
extern ssize_t do_sync_read(struct file *filp, char __user *buf, size_t len, loff_t *ppos);
extern ssize_t do_sync_write(struct file *filp, const char __user *buf, size_t len, loff_t *ppos);
ssize_t generic_file_write_nolock(struct file *file, const struct iovec *iov,
diff --git a/kernel/futex.c b/kernel/futex.c
index 5699c51..7c334f3 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -779,7 +779,7 @@ static int futex_fd(unsigned long uaddr,
ret = get_unused_fd();
if (ret < 0)
goto out;
- filp = get_empty_filp();
+ filp = get_empty_filp(0);
if (!filp) {
put_unused_fd(ret);
ret = -ENFILE;
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index e82726f..e8f9b5f 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -943,7 +943,7 @@ static ctl_table fs_table[] = {
.ctl_name = FS_NRFILE,
.procname = "file-nr",
.data = &files_stat,
- .maxlen = 3*sizeof(int),
+ .maxlen = 4*sizeof(int),
.mode = 0444,
.proc_handler = &proc_nr_files,
},
diff --git a/mm/shmem.c b/mm/shmem.c
index 37eaf42..83bbbe8 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2311,7 +2311,7 @@ struct file *shmem_file_setup(char *name
goto put_memory;
error = -ENFILE;
- file = get_empty_filp();
+ file = get_empty_filp(0);
if (!file)
goto put_dentry;
diff --git a/mm/tiny-shmem.c b/mm/tiny-shmem.c
index f9d6a9c..b014dd5 100644
--- a/mm/tiny-shmem.c
+++ b/mm/tiny-shmem.c
@@ -71,7 +71,7 @@ struct file *shmem_file_setup(char *name
goto put_memory;
error = -ENFILE;
- file = get_empty_filp();
+ file = get_empty_filp(0);
if (!file)
goto put_dentry;
diff --git a/net/socket.c b/net/socket.c
index 23898f4..9743df2 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -377,7 +377,7 @@ static int sock_alloc_fd(struct file **f
fd = get_unused_fd();
if (likely(fd >= 0)) {
- struct file *file = get_empty_filp();
+ struct file *file = get_empty_filp(0);
*filep = file;
if (unlikely(!file)) {
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 4/7] FS-Cache: Export find_get_pages()
2006-04-20 16:59 [PATCH 1/7] FS-Cache: Provide a filesystem-specific sync'able page bit David Howells
2006-04-20 16:59 ` [PATCH 2/7] FS-Cache: Add notification of page becoming writable to VMA ops David Howells
2006-04-20 16:59 ` [PATCH 3/7] FS-Cache: Avoid ENFILE checking for kernel-specific open files David Howells
@ 2006-04-20 16:59 ` David Howells
2006-04-20 17:19 ` Christoph Hellwig
2006-04-20 16:59 ` [PATCH 6/7] FS-Cache: Make kAFS use FS-Cache David Howells
` (3 subsequent siblings)
6 siblings, 1 reply; 29+ messages in thread
From: David Howells @ 2006-04-20 16:59 UTC (permalink / raw)
To: torvalds, akpm, steved, sct, aviro
Cc: linux-fsdevel, linux-cachefs, nfsv4, linux-kernel
The attached patch exports find_get_pages() for use by the kAFS filesystem in
conjunction with it caching patch.
Signed-Off-By: David Howells <dhowells@redhat.com>
---
mm/filemap.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/mm/filemap.c b/mm/filemap.c
index d4a598f..d6f7ab4 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -714,6 +714,8 @@ unsigned find_get_pages(struct address_s
return ret;
}
+EXPORT_SYMBOL(find_get_pages);
+
/*
* Like find_get_pages, except we only return pages which are tagged with
* `tag'. We update *index to index the next page for the traversal.
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 6/7] FS-Cache: Make kAFS use FS-Cache
2006-04-20 16:59 [PATCH 1/7] FS-Cache: Provide a filesystem-specific sync'able page bit David Howells
` (2 preceding siblings ...)
2006-04-20 16:59 ` [PATCH 4/7] FS-Cache: Export find_get_pages() David Howells
@ 2006-04-20 16:59 ` David Howells
2006-04-21 0:12 ` [PATCH 1/7] FS-Cache: Provide a filesystem-specific sync'able page bit Andrew Morton
` (2 subsequent siblings)
6 siblings, 0 replies; 29+ messages in thread
From: David Howells @ 2006-04-20 16:59 UTC (permalink / raw)
To: torvalds, akpm, steved, sct, aviro
Cc: linux-fsdevel, linux-cachefs, nfsv4, linux-kernel
The attached patch makes the kAFS filesystem in fs/afs/ use FS-Cache, and
through it any attached caches. The kAFS filesystem will use caching
automatically if it's available.
Signed-Off-By: David Howells <dhowells@redhat.com>
---
fs/Kconfig | 7 +
fs/afs/cache.h | 27 ------
fs/afs/cell.c | 109 ++++++++++++++---------
fs/afs/cell.h | 16 +--
fs/afs/cmservice.c | 2
fs/afs/dir.c | 15 +--
fs/afs/file.c | 240 +++++++++++++++++++++++++++++++++-----------------
fs/afs/fsclient.c | 4 +
fs/afs/inode.c | 43 ++++++---
fs/afs/internal.h | 24 ++---
fs/afs/main.c | 24 ++---
fs/afs/mntpt.c | 12 +--
fs/afs/proc.c | 1
fs/afs/server.c | 3 -
fs/afs/vlocation.c | 185 ++++++++++++++++++++++++---------------
fs/afs/vnode.c | 249 +++++++++++++++++++++++++++++++++++++++++++---------
fs/afs/vnode.h | 10 +-
fs/afs/volume.c | 78 ++++++----------
fs/afs/volume.h | 28 +-----
19 files changed, 660 insertions(+), 417 deletions(-)
diff --git a/fs/Kconfig b/fs/Kconfig
index 66acf29..6c95e58 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -1816,6 +1816,13 @@ config AFS_FS
If unsure, say N.
+config AFS_FSCACHE
+ bool "Provide AFS client caching support"
+ depends on AFS_FS && FSCACHE && EXPERIMENTAL
+ help
+ Say Y here if you want AFS data to be cached locally on through the
+ generic filesystem cache manager
+
config RXRPC
tristate
diff --git a/fs/afs/cache.h b/fs/afs/cache.h
deleted file mode 100644
index 9eb7722..0000000
--- a/fs/afs/cache.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/* cache.h: AFS local cache management interface
- *
- * Copyright (C) 2002 Red Hat, Inc. All Rights Reserved.
- * Written by David Howells (dhowells@redhat.com)
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#ifndef _LINUX_AFS_CACHE_H
-#define _LINUX_AFS_CACHE_H
-
-#undef AFS_CACHING_SUPPORT
-
-#include <linux/mm.h>
-#ifdef AFS_CACHING_SUPPORT
-#include <linux/cachefs.h>
-#endif
-#include "types.h"
-
-#ifdef __KERNEL__
-
-#endif /* __KERNEL__ */
-
-#endif /* _LINUX_AFS_CACHE_H */
diff --git a/fs/afs/cell.c b/fs/afs/cell.c
index 009a9ae..93a0846 100644
--- a/fs/afs/cell.c
+++ b/fs/afs/cell.c
@@ -31,17 +31,21 @@ static DEFINE_RWLOCK(afs_cells_lock);
static DECLARE_RWSEM(afs_cells_sem); /* add/remove serialisation */
static struct afs_cell *afs_cell_root;
-#ifdef AFS_CACHING_SUPPORT
-static cachefs_match_val_t afs_cell_cache_match(void *target,
- const void *entry);
-static void afs_cell_cache_update(void *source, void *entry);
-
-struct cachefs_index_def afs_cache_cell_index_def = {
- .name = "cell_ix",
- .data_size = sizeof(struct afs_cache_cell),
- .keys[0] = { CACHEFS_INDEX_KEYS_ASCIIZ, 64 },
- .match = afs_cell_cache_match,
- .update = afs_cell_cache_update,
+#ifdef CONFIG_AFS_FSCACHE
+static uint16_t afs_cell_cache_get_key(const void *cookie_netfs_data,
+ void *buffer, uint16_t buflen);
+static uint16_t afs_cell_cache_get_aux(const void *cookie_netfs_data,
+ void *buffer, uint16_t buflen);
+static fscache_checkaux_t afs_cell_cache_check_aux(void *cookie_netfs_data,
+ const void *buffer,
+ uint16_t buflen);
+
+static struct fscache_cookie_def afs_cell_cache_index_def = {
+ .name = "AFS cell",
+ .type = FSCACHE_COOKIE_TYPE_INDEX,
+ .get_key = afs_cell_cache_get_key,
+ .get_aux = afs_cell_cache_get_aux,
+ .check_aux = afs_cell_cache_check_aux,
};
#endif
@@ -115,12 +119,11 @@ int afs_cell_create(const char *name, ch
if (ret < 0)
goto error;
-#ifdef AFS_CACHING_SUPPORT
- /* put it up for caching */
- cachefs_acquire_cookie(afs_cache_netfs.primary_index,
- &afs_vlocation_cache_index_def,
- cell,
- &cell->cache);
+#ifdef CONFIG_AFS_FSCACHE
+ /* put it up for caching (this never returns an error) */
+ cell->cache = fscache_acquire_cookie(afs_cache_netfs.primary_index,
+ &afs_cell_cache_index_def,
+ cell);
#endif
/* add to the cell lists */
@@ -345,8 +348,8 @@ static void afs_cell_destroy(struct afs_
list_del_init(&cell->proc_link);
up_write(&afs_proc_cells_sem);
-#ifdef AFS_CACHING_SUPPORT
- cachefs_relinquish_cookie(cell->cache, 0);
+#ifdef CONFIG_AFS_FSCACHE
+ fscache_relinquish_cookie(cell->cache, 0);
#endif
up_write(&afs_cells_sem);
@@ -526,44 +529,62 @@ void afs_cell_purge(void)
/*****************************************************************************/
/*
- * match a cell record obtained from the cache
+ * set the key for the index entry
*/
-#ifdef AFS_CACHING_SUPPORT
-static cachefs_match_val_t afs_cell_cache_match(void *target,
- const void *entry)
+#ifdef CONFIG_AFS_FSCACHE
+static uint16_t afs_cell_cache_get_key(const void *cookie_netfs_data,
+ void *buffer, uint16_t bufmax)
{
- const struct afs_cache_cell *ccell = entry;
- struct afs_cell *cell = target;
+ const struct afs_cell *cell = cookie_netfs_data;
+ uint16_t klen;
- _enter("{%s},{%s}", ccell->name, cell->name);
+ _enter("%p,%p,%u", cell, buffer, bufmax);
- if (strncmp(ccell->name, cell->name, sizeof(ccell->name)) == 0) {
- _leave(" = SUCCESS");
- return CACHEFS_MATCH_SUCCESS;
- }
+ klen = strlen(cell->name);
+ if (klen > bufmax)
+ return 0;
+
+ memcpy(buffer, cell->name, klen);
+ return klen;
- _leave(" = FAILED");
- return CACHEFS_MATCH_FAILED;
-} /* end afs_cell_cache_match() */
+} /* end afs_cell_cache_get_key() */
#endif
/*****************************************************************************/
/*
- * update a cell record in the cache
+ * provide new auxilliary cache data
*/
-#ifdef AFS_CACHING_SUPPORT
-static void afs_cell_cache_update(void *source, void *entry)
+#ifdef CONFIG_AFS_FSCACHE
+static uint16_t afs_cell_cache_get_aux(const void *cookie_netfs_data,
+ void *buffer, uint16_t bufmax)
{
- struct afs_cache_cell *ccell = entry;
- struct afs_cell *cell = source;
+ const struct afs_cell *cell = cookie_netfs_data;
+ uint16_t dlen;
- _enter("%p,%p", source, entry);
+ _enter("%p,%p,%u", cell, buffer, bufmax);
- strncpy(ccell->name, cell->name, sizeof(ccell->name));
+ dlen = cell->vl_naddrs * sizeof(cell->vl_addrs[0]);
+ dlen = min(dlen, bufmax);
+ dlen &= ~(sizeof(cell->vl_addrs[0]) - 1);
- memcpy(ccell->vl_servers,
- cell->vl_addrs,
- min(sizeof(ccell->vl_servers), sizeof(cell->vl_addrs)));
+ memcpy(buffer, cell->vl_addrs, dlen);
+
+ return dlen;
+
+} /* end afs_cell_cache_get_aux() */
+#endif
+
+/*****************************************************************************/
+/*
+ * check that the auxilliary data indicates that the entry is still valid
+ */
+#ifdef CONFIG_AFS_FSCACHE
+static fscache_checkaux_t afs_cell_cache_check_aux(void *cookie_netfs_data,
+ const void *buffer,
+ uint16_t buflen)
+{
+ _leave(" = OKAY");
+ return FSCACHE_CHECKAUX_OKAY;
-} /* end afs_cell_cache_update() */
+} /* end afs_cell_cache_check_aux() */
#endif
diff --git a/fs/afs/cell.h b/fs/afs/cell.h
index 4834910..d670502 100644
--- a/fs/afs/cell.h
+++ b/fs/afs/cell.h
@@ -13,7 +13,7 @@
#define _LINUX_AFS_CELL_H
#include "types.h"
-#include "cache.h"
+#include <linux/fscache.h>
#define AFS_CELL_MAX_ADDRS 15
@@ -21,16 +21,6 @@ extern volatile int afs_cells_being_purg
/*****************************************************************************/
/*
- * entry in the cached cell catalogue
- */
-struct afs_cache_cell
-{
- char name[64]; /* cell name (padded with NULs) */
- struct in_addr vl_servers[15]; /* cached cell VL servers */
-};
-
-/*****************************************************************************/
-/*
* AFS cell record
*/
struct afs_cell
@@ -39,8 +29,8 @@ struct afs_cell
struct list_head link; /* main cell list link */
struct list_head proc_link; /* /proc cell list link */
struct proc_dir_entry *proc_dir; /* /proc dir for this cell */
-#ifdef AFS_CACHING_SUPPORT
- struct cachefs_cookie *cache; /* caching cookie */
+#ifdef CONFIG_AFS_FSCACHE
+ struct fscache_cookie *cache; /* caching cookie */
#endif
/* server record management */
diff --git a/fs/afs/cmservice.c b/fs/afs/cmservice.c
index 3d097fd..f87d5a7 100644
--- a/fs/afs/cmservice.c
+++ b/fs/afs/cmservice.c
@@ -24,7 +24,7 @@
#include "internal.h"
static unsigned afscm_usage; /* AFS cache manager usage count */
-static struct rw_semaphore afscm_sem; /* AFS cache manager start/stop semaphore */
+static DECLARE_RWSEM(afscm_sem); /* AFS cache manager start/stop semaphore */
static int afscm_new_call(struct rxrpc_call *call);
static void afscm_attention(struct rxrpc_call *call);
diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index a6dff6a..b8e7c32 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -145,7 +145,7 @@ static inline void afs_dir_check_page(st
qty /= sizeof(union afs_dir_block);
/* check them */
- dbuf = page_address(page);
+ dbuf = kmap_atomic(page, KM_USER0);
for (tmp = 0; tmp < qty; tmp++) {
if (dbuf->blocks[tmp].pagehdr.magic != AFS_DIR_MAGIC) {
printk("kAFS: %s(%lu): bad magic %d/%d is %04hx\n",
@@ -154,12 +154,12 @@ static inline void afs_dir_check_page(st
goto error;
}
}
+ kunmap_atomic(dbuf, KM_USER0);
- SetPageChecked(page);
return;
error:
- SetPageChecked(page);
+ kunmap_atomic(dbuf, KM_USER0);
SetPageError(page);
} /* end afs_dir_check_page() */
@@ -170,7 +170,6 @@ static inline void afs_dir_check_page(st
*/
static inline void afs_dir_put_page(struct page *page)
{
- kunmap(page);
page_cache_release(page);
} /* end afs_dir_put_page() */
@@ -190,11 +189,9 @@ static struct page *afs_dir_get_page(str
NULL);
if (!IS_ERR(page)) {
wait_on_page_locked(page);
- kmap(page);
if (!PageUptodate(page))
goto fail;
- if (!PageChecked(page))
- afs_dir_check_page(dir, page);
+ afs_dir_check_page(dir, page);
if (PageError(page))
goto fail;
}
@@ -359,7 +356,7 @@ static int afs_dir_iterate(struct inode
limit = blkoff & ~(PAGE_SIZE - 1);
- dbuf = page_address(page);
+ dbuf = kmap_atomic(page, KM_USER0);
/* deal with the individual blocks stashed on this page */
do {
@@ -368,6 +365,7 @@ static int afs_dir_iterate(struct inode
ret = afs_dir_iterate_block(fpos, dblock, blkoff,
cookie, filldir);
if (ret != 1) {
+ kunmap_atomic(dbuf, KM_USER0);
afs_dir_put_page(page);
goto out;
}
@@ -376,6 +374,7 @@ static int afs_dir_iterate(struct inode
} while (*fpos < dir->i_size && blkoff < limit);
+ kunmap_atomic(dbuf, KM_USER0);
afs_dir_put_page(page);
ret = 0;
}
diff --git a/fs/afs/file.c b/fs/afs/file.c
index 7bb7168..7521cd5 100644
--- a/fs/afs/file.c
+++ b/fs/afs/file.c
@@ -16,12 +16,15 @@
#include <linux/slab.h>
#include <linux/fs.h>
#include <linux/pagemap.h>
+#include <linux/pagevec.h>
#include <linux/buffer_head.h>
#include "volume.h"
#include "vnode.h"
#include <rxrpc/call.h>
#include "internal.h"
+#define list_to_page(head) (list_entry((head)->prev, struct page, lru))
+
#if 0
static int afs_file_open(struct inode *inode, struct file *file);
static int afs_file_release(struct inode *inode, struct file *file);
@@ -30,30 +33,68 @@ static int afs_file_release(struct inode
static int afs_file_readpage(struct file *file, struct page *page);
static void afs_file_invalidatepage(struct page *page, unsigned long offset);
static int afs_file_releasepage(struct page *page, gfp_t gfp_flags);
+static int afs_file_mmap(struct file * file, struct vm_area_struct * vma);
+
+#ifdef CONFIG_AFS_FSCACHE
+static int afs_file_readpages(struct file *filp, struct address_space *mapping,
+ struct list_head *pages, unsigned nr_pages);
+static int afs_file_page_mkwrite(struct vm_area_struct *vma, struct page *page);
+#endif
struct inode_operations afs_file_inode_operations = {
.getattr = afs_inode_getattr,
};
+struct file_operations afs_file_file_operations = {
+ .read = generic_file_read,
+ .mmap = afs_file_mmap,
+};
+
struct address_space_operations afs_fs_aops = {
.readpage = afs_file_readpage,
+#ifdef CONFIG_AFS_FSCACHE
+ .readpages = afs_file_readpages,
+#endif
.sync_page = block_sync_page,
.set_page_dirty = __set_page_dirty_nobuffers,
.releasepage = afs_file_releasepage,
.invalidatepage = afs_file_invalidatepage,
};
+static struct vm_operations_struct afs_fs_vm_operations = {
+ .nopage = filemap_nopage,
+ .populate = filemap_populate,
+#ifdef CONFIG_AFS_FSCACHE
+ .page_mkwrite = afs_file_page_mkwrite,
+#endif
+};
+
+/*****************************************************************************/
+/*
+ * set up a memory mapping on an AFS file
+ * - we set our own VMA ops so that we can catch the page becoming writable for
+ * userspace for shared-writable mmap
+ */
+static int afs_file_mmap(struct file *file, struct vm_area_struct *vma)
+{
+ _enter("");
+
+ file_accessed(file);
+ vma->vm_ops = &afs_fs_vm_operations;
+ return 0;
+
+} /* end afs_file_mmap() */
+
/*****************************************************************************/
/*
* deal with notification that a page was read from the cache
*/
-#ifdef AFS_CACHING_SUPPORT
-static void afs_file_readpage_read_complete(void *cookie_data,
- struct page *page,
+#ifdef CONFIG_AFS_FSCACHE
+static void afs_file_readpage_read_complete(struct page *page,
void *data,
int error)
{
- _enter("%p,%p,%p,%d", cookie_data, page, data, error);
+ _enter("%p,%p,%d", page, data, error);
if (error)
SetPageError(page);
@@ -68,15 +109,16 @@ static void afs_file_readpage_read_compl
/*
* deal with notification that a page was written to the cache
*/
-#ifdef AFS_CACHING_SUPPORT
-static void afs_file_readpage_write_complete(void *cookie_data,
- struct page *page,
+#ifdef CONFIG_AFS_FSCACHE
+static void afs_file_readpage_write_complete(struct page *page,
void *data,
int error)
{
- _enter("%p,%p,%p,%d", cookie_data, page, data, error);
+ _enter("%p,%p,%d", page, data, error);
- unlock_page(page);
+ /* note that the page has been written to the cache and can now be
+ * modified */
+ end_page_fs_misc(page);
} /* end afs_file_readpage_write_complete() */
#endif
@@ -88,16 +130,13 @@ static void afs_file_readpage_write_comp
static int afs_file_readpage(struct file *file, struct page *page)
{
struct afs_rxfs_fetch_descriptor desc;
-#ifdef AFS_CACHING_SUPPORT
- struct cachefs_page *pageio;
-#endif
struct afs_vnode *vnode;
struct inode *inode;
int ret;
inode = page->mapping->host;
- _enter("{%lu},{%lu}", inode->i_ino, page->index);
+ _enter("{%lu},%p{%lu}", inode->i_ino, page, page->index);
vnode = AFS_FS_I(inode);
@@ -107,13 +146,9 @@ static int afs_file_readpage(struct file
if (vnode->flags & AFS_VNODE_DELETED)
goto error;
-#ifdef AFS_CACHING_SUPPORT
- ret = cachefs_page_get_private(page, &pageio, GFP_NOIO);
- if (ret < 0)
- goto error;
-
+#ifdef CONFIG_AFS_FSCACHE
/* is it cached? */
- ret = cachefs_read_or_alloc_page(vnode->cache,
+ ret = fscache_read_or_alloc_page(vnode->cache,
page,
afs_file_readpage_read_complete,
NULL,
@@ -123,18 +158,20 @@ static int afs_file_readpage(struct file
#endif
switch (ret) {
- /* read BIO submitted and wb-journal entry found */
- case 1:
- BUG(); // TODO - handle wb-journal match
-
/* read BIO submitted (page in cache) */
case 0:
break;
- /* no page available in cache */
- case -ENOBUFS:
+ /* page not yet cached */
case -ENODATA:
+ _debug("cache said ENODATA");
+ goto go_on;
+
+ /* page will not be cached */
+ case -ENOBUFS:
+ _debug("cache said ENOBUFS");
default:
+ go_on:
desc.fid = vnode->fid;
desc.offset = page->index << PAGE_CACHE_SHIFT;
desc.size = min((size_t) (inode->i_size - desc.offset),
@@ -148,34 +185,40 @@ static int afs_file_readpage(struct file
ret = afs_vnode_fetch_data(vnode, &desc);
kunmap(page);
if (ret < 0) {
- if (ret==-ENOENT) {
- _debug("got NOENT from server"
+ if (ret == -ENOENT) {
+ kdebug("got NOENT from server"
" - marking file deleted and stale");
vnode->flags |= AFS_VNODE_DELETED;
ret = -ESTALE;
}
-#ifdef AFS_CACHING_SUPPORT
- cachefs_uncache_page(vnode->cache, page);
+#ifdef CONFIG_AFS_FSCACHE
+ fscache_uncache_page(vnode->cache, page);
+ ClearPagePrivate(page);
#endif
goto error;
}
SetPageUptodate(page);
-#ifdef AFS_CACHING_SUPPORT
- if (cachefs_write_page(vnode->cache,
- page,
- afs_file_readpage_write_complete,
- NULL,
- GFP_KERNEL) != 0
- ) {
- cachefs_uncache_page(vnode->cache, page);
- unlock_page(page);
+ /* send the page to the cache */
+#ifdef CONFIG_AFS_FSCACHE
+ if (PagePrivate(page)) {
+ if (TestSetPageFsMisc(page))
+ BUG();
+ if (fscache_write_page(vnode->cache,
+ page,
+ afs_file_readpage_write_complete,
+ NULL,
+ GFP_KERNEL) != 0
+ ) {
+ fscache_uncache_page(vnode->cache, page);
+ ClearPagePrivate(page);
+ end_page_fs_misc(page);
+ }
}
-#else
- unlock_page(page);
#endif
+ unlock_page(page);
}
_leave(" = 0");
@@ -192,20 +235,63 @@ static int afs_file_readpage(struct file
/*****************************************************************************/
/*
- * get a page cookie for the specified page
+ * read a set of pages
*/
-#ifdef AFS_CACHING_SUPPORT
-int afs_cache_get_page_cookie(struct page *page,
- struct cachefs_page **_page_cookie)
+#ifdef CONFIG_AFS_FSCACHE
+static int afs_file_readpages(struct file *filp, struct address_space *mapping,
+ struct list_head *pages, unsigned nr_pages)
{
- int ret;
+ struct afs_vnode *vnode;
+#if 0
+ struct pagevec lru_pvec;
+ unsigned page_idx;
+#endif
+ int ret = 0;
- _enter("");
- ret = cachefs_page_get_private(page,_page_cookie, GFP_NOIO);
+ _enter(",{%lu},,%d", mapping->host->i_ino, nr_pages);
- _leave(" = %d", ret);
+ vnode = AFS_FS_I(mapping->host);
+ if (vnode->flags & AFS_VNODE_DELETED) {
+ _leave(" = -ESTALE");
+ return -ESTALE;
+ }
+
+ /* attempt to read as many of the pages as possible */
+ ret = fscache_read_or_alloc_pages(vnode->cache,
+ mapping,
+ pages,
+ &nr_pages,
+ afs_file_readpage_read_complete,
+ NULL,
+ mapping_gfp_mask(mapping));
+
+ switch (ret) {
+ /* all pages are being read from the cache */
+ case 0:
+ BUG_ON(!list_empty(pages));
+ BUG_ON(nr_pages != 0);
+ _leave(" = 0 [reading all]");
+ return 0;
+
+ /* there were pages that couldn't be read from the cache */
+ case -ENODATA:
+ case -ENOBUFS:
+ break;
+
+ /* other error */
+ default:
+ _leave(" = %d", ret);
+ return ret;
+ }
+
+ /* load the missing pages from the network */
+ ret = read_cache_pages(mapping, pages,
+ (void *) afs_file_readpage, NULL);
+
+ _leave(" = %d [netting]", ret);
return ret;
-} /* end afs_cache_get_page_cookie() */
+
+} /* end afs_file_readpages() */
#endif
/*****************************************************************************/
@@ -214,35 +300,23 @@ int afs_cache_get_page_cookie(struct pag
*/
static void afs_file_invalidatepage(struct page *page, unsigned long offset)
{
- int ret = 1;
-
_enter("{%lu},%lu", page->index, offset);
BUG_ON(!PageLocked(page));
- if (PagePrivate(page)) {
-#ifdef AFS_CACHING_SUPPORT
- struct afs_vnode *vnode = AFS_FS_I(page->mapping->host);
- cachefs_uncache_page(vnode->cache,page);
-#endif
-
+ if (PagePrivate(page) && offset == 0) {
/* We release buffers only if the entire page is being
* invalidated.
* The get_block cached value has been unconditionally
* invalidated, so real IO is not possible anymore.
*/
- if (offset == 0) {
- BUG_ON(!PageLocked(page));
-
- ret = 0;
- if (!PageWriteback(page))
- ret = page->mapping->a_ops->releasepage(page,
- 0);
- /* possibly should BUG_ON(!ret); - neilb */
- }
+ if (!PageWriteback(page))
+ if (page->mapping->a_ops->releasepage(page, 0) != 0)
+ BUG();
}
- _leave(" = %d", ret);
+ _leave("");
+
} /* end afs_file_invalidatepage() */
/*****************************************************************************/
@@ -251,23 +325,29 @@ static void afs_file_invalidatepage(stru
*/
static int afs_file_releasepage(struct page *page, gfp_t gfp_flags)
{
- struct cachefs_page *pageio;
-
_enter("{%lu},%x", page->index, gfp_flags);
- if (PagePrivate(page)) {
-#ifdef AFS_CACHING_SUPPORT
- struct afs_vnode *vnode = AFS_FS_I(page->mapping->host);
- cachefs_uncache_page(vnode->cache, page);
+#ifdef CONFIG_AFS_FSCACHE
+ wait_on_page_fs_misc(page);
+ fscache_uncache_page(AFS_FS_I(page->mapping->host)->cache, page);
+ ClearPagePrivate(page);
#endif
- pageio = (struct cachefs_page *) page_private(page);
- set_page_private(page, 0);
- ClearPagePrivate(page);
-
- kfree(pageio);
- }
-
_leave(" = 0");
return 0;
+
} /* end afs_file_releasepage() */
+
+/*****************************************************************************/
+/*
+ * wait for the disc cache to finish writing before permitting modification of
+ * our page in the page cache
+ */
+#ifdef CONFIG_AFS_FSCACHE
+static int afs_file_page_mkwrite(struct vm_area_struct *vma, struct page *page)
+{
+ wait_on_page_fs_misc(page);
+ return 0;
+
+} /* end afs_file_page_mkwrite() */
+#endif
diff --git a/fs/afs/fsclient.c b/fs/afs/fsclient.c
index 61bc371..c88c41a 100644
--- a/fs/afs/fsclient.c
+++ b/fs/afs/fsclient.c
@@ -398,6 +398,8 @@ int afs_rxfs_fetch_file_status(struct af
bp++; /* spare6 */
}
+ _debug("Data Version %llx\n", vnode->status.version);
+
/* success */
ret = 0;
@@ -408,7 +410,7 @@ int afs_rxfs_fetch_file_status(struct af
out_put_conn:
afs_server_release_callslot(server, &callslot);
out:
- _leave("");
+ _leave(" = %d", ret);
return ret;
abort:
diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 4ebb30a..d188380 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -65,6 +65,11 @@ static int afs_inode_map_status(struct a
return -EBADMSG;
}
+#ifdef CONFIG_AFS_FSCACHE
+ if (vnode->status.size != inode->i_size)
+ fscache_set_i_size(vnode->cache, vnode->status.size);
+#endif
+
inode->i_nlink = vnode->status.nlink;
inode->i_uid = vnode->status.owner;
inode->i_gid = 0;
@@ -101,13 +106,33 @@ static int afs_inode_fetch_status(struct
struct afs_vnode *vnode;
int ret;
+ _enter("");
+
vnode = AFS_FS_I(inode);
ret = afs_vnode_fetch_status(vnode);
- if (ret == 0)
+ if (ret == 0) {
+#ifdef CONFIG_AFS_FSCACHE
+ if (vnode->cache == FSCACHE_NEGATIVE_COOKIE) {
+ vnode->cache =
+ fscache_acquire_cookie(vnode->volume->cache,
+ &afs_vnode_cache_index_def,
+ vnode);
+ if (!vnode->cache)
+ printk("Negative\n");
+ }
+#endif
ret = afs_inode_map_status(vnode);
+#ifdef CONFIG_AFS_FSCACHE
+ if (ret < 0) {
+ fscache_relinquish_cookie(vnode->cache, 0);
+ vnode->cache = FSCACHE_NEGATIVE_COOKIE;
+ }
+#endif
+ }
+ _leave(" = %d", ret);
return ret;
} /* end afs_inode_fetch_status() */
@@ -122,6 +147,7 @@ static int afs_iget5_test(struct inode *
return inode->i_ino == data->fid.vnode &&
inode->i_version == data->fid.unique;
+
} /* end afs_iget5_test() */
/*****************************************************************************/
@@ -179,20 +205,11 @@ inline int afs_iget(struct super_block *
return ret;
}
-#ifdef AFS_CACHING_SUPPORT
- /* set up caching before reading the status, as fetch-status reads the
- * first page of symlinks to see if they're really mntpts */
- cachefs_acquire_cookie(vnode->volume->cache,
- NULL,
- vnode,
- &vnode->cache);
-#endif
-
/* okay... it's a new inode */
inode->i_flags |= S_NOATIME;
vnode->flags |= AFS_VNODE_CHANGED;
ret = afs_inode_fetch_status(inode);
- if (ret<0)
+ if (ret < 0)
goto bad_inode;
/* success */
@@ -278,8 +295,8 @@ void afs_clear_inode(struct inode *inode
afs_vnode_give_up_callback(vnode);
-#ifdef AFS_CACHING_SUPPORT
- cachefs_relinquish_cookie(vnode->cache, 0);
+#ifdef CONFIG_AFS_FSCACHE
+ fscache_relinquish_cookie(vnode->cache, 0);
vnode->cache = NULL;
#endif
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 72febdf..0bddcdf 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -16,15 +16,17 @@
#include <linux/kernel.h>
#include <linux/fs.h>
#include <linux/pagemap.h>
+#include <linux/fscache.h>
/*
* debug tracing
*/
-#define kenter(FMT, a...) printk("==> %s("FMT")\n",__FUNCTION__ , ## a)
-#define kleave(FMT, a...) printk("<== %s()"FMT"\n",__FUNCTION__ , ## a)
-#define kdebug(FMT, a...) printk(FMT"\n" , ## a)
-#define kproto(FMT, a...) printk("### "FMT"\n" , ## a)
-#define knet(FMT, a...) printk(FMT"\n" , ## a)
+#define __kdbg(FMT, a...) printk("[%05d] "FMT"\n", current->pid , ## a)
+#define kenter(FMT, a...) __kdbg("==> %s("FMT")", __FUNCTION__ , ## a)
+#define kleave(FMT, a...) __kdbg("<== %s()"FMT, __FUNCTION__ , ## a)
+#define kdebug(FMT, a...) __kdbg(FMT , ## a)
+#define kproto(FMT, a...) __kdbg("### "FMT , ## a)
+#define knet(FMT, a...) __kdbg(FMT , ## a)
#ifdef __KDEBUG
#define _enter(FMT, a...) kenter(FMT , ## a)
@@ -56,9 +58,6 @@ static inline void afs_discard_my_signal
*/
extern struct rw_semaphore afs_proc_cells_sem;
extern struct list_head afs_proc_cells;
-#ifdef AFS_CACHING_SUPPORT
-extern struct cachefs_index_def afs_cache_cell_index_def;
-#endif
/*
* dir.c
@@ -72,11 +71,6 @@ extern const struct file_operations afs_
extern struct address_space_operations afs_fs_aops;
extern struct inode_operations afs_file_inode_operations;
-#ifdef AFS_CACHING_SUPPORT
-extern int afs_cache_get_page_cookie(struct page *page,
- struct cachefs_page **_page_cookie);
-#endif
-
/*
* inode.c
*/
@@ -97,8 +91,8 @@ extern void afs_key_unregister(void);
/*
* main.c
*/
-#ifdef AFS_CACHING_SUPPORT
-extern struct cachefs_netfs afs_cache_netfs;
+#ifdef CONFIG_AFS_FSCACHE
+extern struct fscache_netfs afs_cache_netfs;
#endif
/*
diff --git a/fs/afs/main.c b/fs/afs/main.c
index 913c689..5840bb2 100644
--- a/fs/afs/main.c
+++ b/fs/afs/main.c
@@ -1,6 +1,6 @@
/* main.c: AFS client file system
*
- * Copyright (C) 2002 Red Hat, Inc. All Rights Reserved.
+ * Copyright (C) 2002,5 Red Hat, Inc. All Rights Reserved.
* Written by David Howells (dhowells@redhat.com)
*
* This program is free software; you can redistribute it and/or
@@ -14,11 +14,11 @@
#include <linux/init.h>
#include <linux/sched.h>
#include <linux/completion.h>
+#include <linux/fscache.h>
#include <rxrpc/rxrpc.h>
#include <rxrpc/transport.h>
#include <rxrpc/call.h>
#include <rxrpc/peer.h>
-#include "cache.h"
#include "cell.h"
#include "server.h"
#include "fsclient.h"
@@ -51,12 +51,11 @@ static struct rxrpc_peer_ops afs_peer_op
struct list_head afs_cb_hash_tbl[AFS_CB_HASH_COUNT];
DEFINE_SPINLOCK(afs_cb_hash_lock);
-#ifdef AFS_CACHING_SUPPORT
-static struct cachefs_netfs_operations afs_cache_ops = {
- .get_page_cookie = afs_cache_get_page_cookie,
+#ifdef CONFIG_AFS_FSCACHE
+static struct fscache_netfs_operations afs_cache_ops = {
};
-struct cachefs_netfs afs_cache_netfs = {
+struct fscache_netfs afs_cache_netfs = {
.name = "afs",
.version = 0,
.ops = &afs_cache_ops,
@@ -83,10 +82,9 @@ static int __init afs_init(void)
if (ret < 0)
return ret;
-#ifdef AFS_CACHING_SUPPORT
+#ifdef CONFIG_AFS_FSCACHE
/* we want to be able to cache */
- ret = cachefs_register_netfs(&afs_cache_netfs,
- &afs_cache_cell_index_def);
+ ret = fscache_register_netfs(&afs_cache_netfs);
if (ret < 0)
goto error;
#endif
@@ -137,8 +135,8 @@ static int __init afs_init(void)
afs_key_unregister();
error_cache:
#endif
-#ifdef AFS_CACHING_SUPPORT
- cachefs_unregister_netfs(&afs_cache_netfs);
+#ifdef CONFIG_AFS_FSCACHE
+ fscache_unregister_netfs(&afs_cache_netfs);
error:
#endif
afs_cell_purge();
@@ -167,8 +165,8 @@ static void __exit afs_exit(void)
#ifdef CONFIG_KEYS_TURNED_OFF
afs_key_unregister();
#endif
-#ifdef AFS_CACHING_SUPPORT
- cachefs_unregister_netfs(&afs_cache_netfs);
+#ifdef CONFIG_AFS_FSCACHE
+ fscache_unregister_netfs(&afs_cache_netfs);
#endif
afs_proc_cleanup();
diff --git a/fs/afs/mntpt.c b/fs/afs/mntpt.c
index 4e6eeb5..91f57cf 100644
--- a/fs/afs/mntpt.c
+++ b/fs/afs/mntpt.c
@@ -82,7 +82,7 @@ int afs_mntpt_check_symlink(struct afs_v
ret = -EIO;
wait_on_page_locked(page);
- buf = kmap(page);
+ buf = kmap_atomic(page, KM_USER0);
if (!PageUptodate(page))
goto out_free;
if (PageError(page))
@@ -105,7 +105,7 @@ int afs_mntpt_check_symlink(struct afs_v
ret = 0;
out_free:
- kunmap(page);
+ kunmap_atomic(buf, KM_USER0);
page_cache_release(page);
out:
_leave(" = %d", ret);
@@ -195,9 +195,9 @@ static struct vfsmount *afs_mntpt_do_aut
if (!PageUptodate(page) || PageError(page))
goto error;
- buf = kmap(page);
+ buf = kmap_atomic(page, KM_USER0);
memcpy(devname, buf, size);
- kunmap(page);
+ kunmap_atomic(buf, KM_USER0);
page_cache_release(page);
page = NULL;
@@ -276,12 +276,12 @@ static void *afs_mntpt_follow_link(struc
*/
static void afs_mntpt_expiry_timed_out(struct afs_timer *timer)
{
- kenter("");
+// kenter("");
mark_mounts_for_expiry(&afs_vfsmounts);
afs_kafstimod_add_timer(&afs_mntpt_expiry_timer,
afs_mntpt_expiry_timeout * HZ);
- kleave("");
+// kleave("");
} /* end afs_mntpt_expiry_timed_out() */
diff --git a/fs/afs/proc.c b/fs/afs/proc.c
index 101d21b..db58488 100644
--- a/fs/afs/proc.c
+++ b/fs/afs/proc.c
@@ -177,6 +177,7 @@ int afs_proc_init(void)
*/
void afs_proc_cleanup(void)
{
+ remove_proc_entry("rootcell", proc_afs);
remove_proc_entry("cells", proc_afs);
remove_proc_entry("fs/afs", NULL);
diff --git a/fs/afs/server.c b/fs/afs/server.c
index 62b093a..7103e10 100644
--- a/fs/afs/server.c
+++ b/fs/afs/server.c
@@ -377,7 +377,6 @@ int afs_server_request_callslot(struct a
else if (list_empty(&server->fs_callq)) {
/* no one waiting */
server->fs_conn_cnt[nconn]++;
- spin_unlock(&server->fs_lock);
}
else {
/* someone's waiting - dequeue them and wake them up */
@@ -395,9 +394,9 @@ int afs_server_request_callslot(struct a
}
pcallslot->ready = 1;
wake_up_process(pcallslot->task);
- spin_unlock(&server->fs_lock);
}
+ spin_unlock(&server->fs_lock);
rxrpc_put_connection(callslot->conn);
callslot->conn = NULL;
diff --git a/fs/afs/vlocation.c b/fs/afs/vlocation.c
index eced206..cfab969 100644
--- a/fs/afs/vlocation.c
+++ b/fs/afs/vlocation.c
@@ -59,17 +59,21 @@ static LIST_HEAD(afs_vlocation_update_pe
static struct afs_vlocation *afs_vlocation_update; /* VL currently being updated */
static DEFINE_SPINLOCK(afs_vlocation_update_lock); /* lock guarding update queue */
-#ifdef AFS_CACHING_SUPPORT
-static cachefs_match_val_t afs_vlocation_cache_match(void *target,
- const void *entry);
-static void afs_vlocation_cache_update(void *source, void *entry);
-
-struct cachefs_index_def afs_vlocation_cache_index_def = {
- .name = "vldb",
- .data_size = sizeof(struct afs_cache_vlocation),
- .keys[0] = { CACHEFS_INDEX_KEYS_ASCIIZ, 64 },
- .match = afs_vlocation_cache_match,
- .update = afs_vlocation_cache_update,
+#ifdef CONFIG_AFS_FSCACHE
+static uint16_t afs_vlocation_cache_get_key(const void *cookie_netfs_data,
+ void *buffer, uint16_t buflen);
+static uint16_t afs_vlocation_cache_get_aux(const void *cookie_netfs_data,
+ void *buffer, uint16_t buflen);
+static fscache_checkaux_t afs_vlocation_cache_check_aux(void *cookie_netfs_data,
+ const void *buffer,
+ uint16_t buflen);
+
+static struct fscache_cookie_def afs_vlocation_cache_index_def = {
+ .name = "AFS.vldb",
+ .type = FSCACHE_COOKIE_TYPE_INDEX,
+ .get_key = afs_vlocation_cache_get_key,
+ .get_aux = afs_vlocation_cache_get_aux,
+ .check_aux = afs_vlocation_cache_check_aux,
};
#endif
@@ -300,13 +304,12 @@ int afs_vlocation_lookup(struct afs_cell
list_add_tail(&vlocation->link, &cell->vl_list);
-#ifdef AFS_CACHING_SUPPORT
+#ifdef CONFIG_AFS_FSCACHE
/* we want to store it in the cache, plus it might already be
* encached */
- cachefs_acquire_cookie(cell->cache,
- &afs_volume_cache_index_def,
- vlocation,
- &vlocation->cache);
+ vlocation->cache = fscache_acquire_cookie(cell->cache,
+ &afs_vlocation_cache_index_def,
+ vlocation);
if (vlocation->valid)
goto found_in_cache;
@@ -341,7 +344,7 @@ int afs_vlocation_lookup(struct afs_cell
active:
active = 1;
-#ifdef AFS_CACHING_SUPPORT
+#ifdef CONFIG_AFS_FSCACHE
found_in_cache:
#endif
/* try to look up a cached volume in the cell VL databases by ID */
@@ -423,9 +426,9 @@ int afs_vlocation_lookup(struct afs_cell
afs_kafstimod_add_timer(&vlocation->upd_timer, 10 * HZ);
-#ifdef AFS_CACHING_SUPPORT
+#ifdef CONFIG_AFS_FSCACHE
/* update volume entry in local cache */
- cachefs_update_cookie(vlocation->cache);
+ fscache_update_cookie(vlocation->cache);
#endif
*_vlocation = vlocation;
@@ -439,8 +442,8 @@ int afs_vlocation_lookup(struct afs_cell
}
else {
list_del(&vlocation->link);
-#ifdef AFS_CACHING_SUPPORT
- cachefs_relinquish_cookie(vlocation->cache, 0);
+#ifdef CONFIG_AFS_FSCACHE
+ fscache_relinquish_cookie(vlocation->cache, 0);
#endif
afs_put_cell(vlocation->cell);
kfree(vlocation);
@@ -538,8 +541,8 @@ void afs_vlocation_do_timeout(struct afs
}
/* we can now destroy it properly */
-#ifdef AFS_CACHING_SUPPORT
- cachefs_relinquish_cookie(vlocation->cache, 0);
+#ifdef CONFIG_AFS_FSCACHE
+ fscache_relinquish_cookie(vlocation->cache, 0);
#endif
afs_put_cell(cell);
@@ -890,65 +893,103 @@ static void afs_vlocation_update_discard
/*****************************************************************************/
/*
- * match a VLDB record stored in the cache
- * - may also load target from entry
+ * set the key for the index entry
*/
-#ifdef AFS_CACHING_SUPPORT
-static cachefs_match_val_t afs_vlocation_cache_match(void *target,
- const void *entry)
-{
- const struct afs_cache_vlocation *vldb = entry;
- struct afs_vlocation *vlocation = target;
-
- _enter("{%s},{%s}", vlocation->vldb.name, vldb->name);
-
- if (strncmp(vlocation->vldb.name, vldb->name, sizeof(vldb->name)) == 0
- ) {
- if (!vlocation->valid ||
- vlocation->vldb.rtime == vldb->rtime
- ) {
- vlocation->vldb = *vldb;
- vlocation->valid = 1;
- _leave(" = SUCCESS [c->m]");
- return CACHEFS_MATCH_SUCCESS;
- }
- /* need to update cache if cached info differs */
- else if (memcmp(&vlocation->vldb, vldb, sizeof(*vldb)) != 0) {
- /* delete if VIDs for this name differ */
- if (memcmp(&vlocation->vldb.vid,
- &vldb->vid,
- sizeof(vldb->vid)) != 0) {
- _leave(" = DELETE");
- return CACHEFS_MATCH_SUCCESS_DELETE;
- }
+#ifdef CONFIG_AFS_FSCACHE
+static uint16_t afs_vlocation_cache_get_key(const void *cookie_netfs_data,
+ void *buffer, uint16_t bufmax)
+{
+ const struct afs_vlocation *vlocation = cookie_netfs_data;
+ uint16_t klen;
- _leave(" = UPDATE");
- return CACHEFS_MATCH_SUCCESS_UPDATE;
- }
- else {
- _leave(" = SUCCESS");
- return CACHEFS_MATCH_SUCCESS;
- }
- }
+ _enter("{%s},%p,%u", vlocation->vldb.name, buffer, bufmax);
+
+ klen = strnlen(vlocation->vldb.name, sizeof(vlocation->vldb.name));
+ if (klen > bufmax)
+ return 0;
+
+ memcpy(buffer, vlocation->vldb.name, klen);
+
+ _leave(" = %u", klen);
+ return klen;
- _leave(" = FAILED");
- return CACHEFS_MATCH_FAILED;
-} /* end afs_vlocation_cache_match() */
+} /* end afs_vlocation_cache_get_key() */
#endif
/*****************************************************************************/
/*
- * update a VLDB record stored in the cache
+ * provide new auxilliary cache data
*/
-#ifdef AFS_CACHING_SUPPORT
-static void afs_vlocation_cache_update(void *source, void *entry)
+#ifdef CONFIG_AFS_FSCACHE
+static uint16_t afs_vlocation_cache_get_aux(const void *cookie_netfs_data,
+ void *buffer, uint16_t bufmax)
{
- struct afs_cache_vlocation *vldb = entry;
- struct afs_vlocation *vlocation = source;
+ const struct afs_vlocation *vlocation = cookie_netfs_data;
+ uint16_t dlen;
+
+ _enter("{%s},%p,%u", vlocation->vldb.name, buffer, bufmax);
+
+ dlen = sizeof(struct afs_cache_vlocation);
+ dlen -= offsetof(struct afs_cache_vlocation, nservers);
+ if (dlen > bufmax)
+ return 0;
+
+ memcpy(buffer, (uint8_t *)&vlocation->vldb.nservers, dlen);
- _enter("");
+ _leave(" = %u", dlen);
+ return dlen;
+
+} /* end afs_vlocation_cache_get_aux() */
+#endif
+
+/*****************************************************************************/
+/*
+ * check that the auxilliary data indicates that the entry is still valid
+ */
+#ifdef CONFIG_AFS_FSCACHE
+static fscache_checkaux_t afs_vlocation_cache_check_aux(void *cookie_netfs_data,
+ const void *buffer,
+ uint16_t buflen)
+{
+ const struct afs_cache_vlocation *cvldb;
+ struct afs_vlocation *vlocation = cookie_netfs_data;
+ uint16_t dlen;
+
+ _enter("{%s},%p,%u", vlocation->vldb.name, buffer, buflen);
+
+ /* check the size of the data is what we're expecting */
+ dlen = sizeof(struct afs_cache_vlocation);
+ dlen -= offsetof(struct afs_cache_vlocation, nservers);
+ if (dlen != buflen)
+ return FSCACHE_CHECKAUX_OBSOLETE;
+
+ cvldb = container_of(buffer, struct afs_cache_vlocation, nservers);
+
+ /* if what's on disk is more valid than what's in memory, then use the
+ * VL record from the cache */
+ if (!vlocation->valid || vlocation->vldb.rtime == cvldb->rtime) {
+ memcpy((uint8_t *)&vlocation->vldb.nservers, buffer, dlen);
+ vlocation->valid = 1;
+ _leave(" = SUCCESS [c->m]");
+ return FSCACHE_CHECKAUX_OKAY;
+ }
+
+ /* need to update the cache if the cached info differs */
+ if (memcmp(&vlocation->vldb, buffer, dlen) != 0) {
+ /* delete if the volume IDs for this name differ */
+ if (memcmp(&vlocation->vldb.vid, &cvldb->vid,
+ sizeof(cvldb->vid)) != 0
+ ) {
+ _leave(" = OBSOLETE");
+ return FSCACHE_CHECKAUX_OBSOLETE;
+ }
+
+ _leave(" = UPDATE");
+ return FSCACHE_CHECKAUX_NEEDS_UPDATE;
+ }
- *vldb = vlocation->vldb;
+ _leave(" = OKAY");
+ return FSCACHE_CHECKAUX_OKAY;
-} /* end afs_vlocation_cache_update() */
+} /* end afs_vlocation_cache_check_aux() */
#endif
diff --git a/fs/afs/vnode.c b/fs/afs/vnode.c
index 9867fef..7116128 100644
--- a/fs/afs/vnode.c
+++ b/fs/afs/vnode.c
@@ -29,17 +29,30 @@ struct afs_timer_ops afs_vnode_cb_timed_
.timed_out = afs_vnode_cb_timed_out,
};
-#ifdef AFS_CACHING_SUPPORT
-static cachefs_match_val_t afs_vnode_cache_match(void *target,
- const void *entry);
-static void afs_vnode_cache_update(void *source, void *entry);
-
-struct cachefs_index_def afs_vnode_cache_index_def = {
- .name = "vnode",
- .data_size = sizeof(struct afs_cache_vnode),
- .keys[0] = { CACHEFS_INDEX_KEYS_BIN, 4 },
- .match = afs_vnode_cache_match,
- .update = afs_vnode_cache_update,
+#ifdef CONFIG_AFS_FSCACHE
+static uint16_t afs_vnode_cache_get_key(const void *cookie_netfs_data,
+ void *buffer, uint16_t buflen);
+static void afs_vnode_cache_get_attr(const void *cookie_netfs_data,
+ uint64_t *size);
+static uint16_t afs_vnode_cache_get_aux(const void *cookie_netfs_data,
+ void *buffer, uint16_t buflen);
+static fscache_checkaux_t afs_vnode_cache_check_aux(void *cookie_netfs_data,
+ const void *buffer,
+ uint16_t buflen);
+static void afs_vnode_cache_mark_pages_cached(void *cookie_netfs_data,
+ struct address_space *mapping,
+ struct pagevec *cached_pvec);
+static void afs_vnode_cache_now_uncached(void *cookie_netfs_data);
+
+struct fscache_cookie_def afs_vnode_cache_index_def = {
+ .name = "AFS.vnode",
+ .type = FSCACHE_COOKIE_TYPE_DATAFILE,
+ .get_key = afs_vnode_cache_get_key,
+ .get_attr = afs_vnode_cache_get_attr,
+ .get_aux = afs_vnode_cache_get_aux,
+ .check_aux = afs_vnode_cache_check_aux,
+ .mark_pages_cached = afs_vnode_cache_mark_pages_cached,
+ .now_uncached = afs_vnode_cache_now_uncached,
};
#endif
@@ -189,6 +202,8 @@ int afs_vnode_fetch_status(struct afs_vn
if (vnode->update_cnt > 0) {
/* someone else started a fetch */
+ _debug("conflict");
+
set_current_state(TASK_UNINTERRUPTIBLE);
add_wait_queue(&vnode->update_waitq, &myself);
@@ -220,6 +235,7 @@ int afs_vnode_fetch_status(struct afs_vn
spin_unlock(&vnode->lock);
set_current_state(TASK_RUNNING);
+ _leave(" [conflicted, %d", !!(vnode->flags & AFS_VNODE_DELETED));
return vnode->flags & AFS_VNODE_DELETED ? -ENOENT : 0;
}
@@ -342,54 +358,197 @@ int afs_vnode_give_up_callback(struct af
/*****************************************************************************/
/*
- * match a vnode record stored in the cache
+ * set the key for the index entry
*/
-#ifdef AFS_CACHING_SUPPORT
-static cachefs_match_val_t afs_vnode_cache_match(void *target,
- const void *entry)
+#ifdef CONFIG_AFS_FSCACHE
+static uint16_t afs_vnode_cache_get_key(const void *cookie_netfs_data,
+ void *buffer, uint16_t bufmax)
{
- const struct afs_cache_vnode *cvnode = entry;
- struct afs_vnode *vnode = target;
+ const struct afs_vnode *vnode = cookie_netfs_data;
+ uint16_t klen;
- _enter("{%x,%x,%Lx},{%x,%x,%Lx}",
- vnode->fid.vnode,
- vnode->fid.unique,
- vnode->status.version,
- cvnode->vnode_id,
- cvnode->vnode_unique,
- cvnode->data_version);
-
- if (vnode->fid.vnode != cvnode->vnode_id) {
- _leave(" = FAILED");
- return CACHEFS_MATCH_FAILED;
- }
+ _enter("{%x,%x,%Lx},%p,%u",
+ vnode->fid.vnode, vnode->fid.unique, vnode->status.version,
+ buffer, bufmax);
+
+ klen = sizeof(vnode->fid.vnode);
+ if (klen > bufmax)
+ return 0;
+
+ memcpy(buffer, &vnode->fid.vnode, sizeof(vnode->fid.vnode));
+
+ _leave(" = %u", klen);
+ return klen;
+
+} /* end afs_vnode_cache_get_key() */
+#endif
+
+/*****************************************************************************/
+/*
+ * provide an updated file attributes
+ */
+#ifdef CONFIG_AFS_FSCACHE
+static void afs_vnode_cache_get_attr(const void *cookie_netfs_data,
+ uint64_t *size)
+{
+ const struct afs_vnode *vnode = cookie_netfs_data;
+
+ _enter("{%x,%x,%Lx},",
+ vnode->fid.vnode, vnode->fid.unique, vnode->status.version);
+
+ *size = i_size_read((struct inode *) &vnode->vfs_inode);
+
+} /* end afs_vnode_cache_get_attr() */
+#endif
+
+/*****************************************************************************/
+/*
+ * provide new auxilliary cache data
+ */
+#ifdef CONFIG_AFS_FSCACHE
+static uint16_t afs_vnode_cache_get_aux(const void *cookie_netfs_data,
+ void *buffer, uint16_t bufmax)
+{
+ const struct afs_vnode *vnode = cookie_netfs_data;
+ uint16_t dlen;
+
+ _enter("{%x,%x,%Lx},%p,%u",
+ vnode->fid.vnode, vnode->fid.unique, vnode->status.version,
+ buffer, bufmax);
- if (vnode->fid.unique != cvnode->vnode_unique ||
- vnode->status.version != cvnode->data_version) {
- _leave(" = DELETE");
- return CACHEFS_MATCH_SUCCESS_DELETE;
+ dlen = sizeof(vnode->fid.unique) + sizeof(vnode->status.version);
+ if (dlen > bufmax)
+ return 0;
+
+ memcpy(buffer, &vnode->fid.unique, sizeof(vnode->fid.unique));
+ buffer += sizeof(vnode->fid.unique);
+ memcpy(buffer, &vnode->status.version, sizeof(vnode->status.version));
+
+ _leave(" = %u", dlen);
+ return dlen;
+
+} /* end afs_vnode_cache_get_aux() */
+#endif
+
+/*****************************************************************************/
+/*
+ * check that the auxilliary data indicates that the entry is still valid
+ */
+#ifdef CONFIG_AFS_FSCACHE
+static fscache_checkaux_t afs_vnode_cache_check_aux(void *cookie_netfs_data,
+ const void *buffer,
+ uint16_t buflen)
+{
+ struct afs_vnode *vnode = cookie_netfs_data;
+ uint16_t dlen;
+
+ _enter("{%x,%x,%Lx},%p,%u",
+ vnode->fid.vnode, vnode->fid.unique, vnode->status.version,
+ buffer, buflen);
+
+ /* check the size of the data is what we're expecting */
+ dlen = sizeof(vnode->fid.unique) + sizeof(vnode->status.version);
+ if (dlen != buflen) {
+ _leave(" = OBSOLETE [len %hx != %hx]", dlen, buflen);
+ return FSCACHE_CHECKAUX_OBSOLETE;
+ }
+
+ if (memcmp(buffer,
+ &vnode->fid.unique,
+ sizeof(vnode->fid.unique)
+ ) != 0
+ ) {
+ unsigned unique;
+
+ memcpy(&unique, buffer, sizeof(unique));
+
+ _leave(" = OBSOLETE [uniq %x != %x]",
+ unique, vnode->fid.unique);
+ return FSCACHE_CHECKAUX_OBSOLETE;
+ }
+
+ if (memcmp(buffer + sizeof(vnode->fid.unique),
+ &vnode->status.version,
+ sizeof(vnode->status.version)
+ ) != 0
+ ) {
+ afs_dataversion_t version;
+
+ memcpy(&version, buffer + sizeof(vnode->fid.unique),
+ sizeof(version));
+
+ _leave(" = OBSOLETE [vers %llx != %llx]",
+ version, vnode->status.version);
+ return FSCACHE_CHECKAUX_OBSOLETE;
}
_leave(" = SUCCESS");
- return CACHEFS_MATCH_SUCCESS;
-} /* end afs_vnode_cache_match() */
+ return FSCACHE_CHECKAUX_OKAY;
+
+} /* end afs_vnode_cache_check_aux() */
#endif
/*****************************************************************************/
/*
- * update a vnode record stored in the cache
+ * indication of pages that now have cache metadata retained
+ * - this function should mark the specified pages as now being cached
*/
-#ifdef AFS_CACHING_SUPPORT
-static void afs_vnode_cache_update(void *source, void *entry)
+#ifdef CONFIG_AFS_FSCACHE
+static void afs_vnode_cache_mark_pages_cached(void *cookie_netfs_data,
+ struct address_space *mapping,
+ struct pagevec *cached_pvec)
{
- struct afs_cache_vnode *cvnode = entry;
- struct afs_vnode *vnode = source;
+ unsigned long loop;
- _enter("");
+ for (loop = 0; loop < cached_pvec->nr; loop++) {
+ struct page *page = cached_pvec->pages[loop];
- cvnode->vnode_id = vnode->fid.vnode;
- cvnode->vnode_unique = vnode->fid.unique;
- cvnode->data_version = vnode->status.version;
+ _debug("- mark %p{%lx}", page, page->index);
-} /* end afs_vnode_cache_update() */
+ SetPagePrivate(page);
+ }
+
+} /* end afs_vnode_cache_mark_pages_cached() */
#endif
+
+/*****************************************************************************/
+/*
+ * indication the cookie is no longer uncached
+ * - this function is called when the backing store currently caching a cookie
+ * is removed
+ * - the netfs should use this to clean up any markers indicating cached pages
+ * - this is mandatory for any object that may have data
+ */
+static void afs_vnode_cache_now_uncached(void *cookie_netfs_data)
+{
+ struct afs_vnode *vnode = cookie_netfs_data;
+ struct pagevec pvec;
+ pgoff_t first;
+ int loop, nr_pages;
+
+ _enter("{%x,%x,%Lx}",
+ vnode->fid.vnode, vnode->fid.unique, vnode->status.version);
+
+ pagevec_init(&pvec, 0);
+ first = 0;
+
+ for (;;) {
+ /* grab a bunch of pages to clean */
+ nr_pages = find_get_pages(vnode->vfs_inode.i_mapping, first,
+ PAGEVEC_SIZE, pvec.pages);
+ if (!nr_pages)
+ break;
+
+ for (loop = 0; loop < nr_pages; loop++)
+ ClearPagePrivate(pvec.pages[loop]);
+
+ first = pvec.pages[nr_pages - 1]->index + 1;
+
+ pvec.nr = nr_pages;
+ pagevec_release(&pvec);
+ cond_resched();
+ }
+
+ _leave("");
+
+} /* end afs_vnode_cache_now_uncached() */
diff --git a/fs/afs/vnode.h b/fs/afs/vnode.h
index b86a971..3f0602d 100644
--- a/fs/afs/vnode.h
+++ b/fs/afs/vnode.h
@@ -13,9 +13,9 @@
#define _LINUX_AFS_VNODE_H
#include <linux/fs.h>
+#include <linux/fscache.h>
#include "server.h"
#include "kafstimod.h"
-#include "cache.h"
#ifdef __KERNEL__
@@ -32,8 +32,8 @@ struct afs_cache_vnode
afs_dataversion_t data_version; /* data version */
};
-#ifdef AFS_CACHING_SUPPORT
-extern struct cachefs_index_def afs_vnode_cache_index_def;
+#ifdef CONFIG_AFS_FSCACHE
+extern struct fscache_cookie_def afs_vnode_cache_index_def;
#endif
/*****************************************************************************/
@@ -47,8 +47,8 @@ struct afs_vnode
struct afs_volume *volume; /* volume on which vnode resides */
struct afs_fid fid; /* the file identifier for this inode */
struct afs_file_status status; /* AFS status info for this file */
-#ifdef AFS_CACHING_SUPPORT
- struct cachefs_cookie *cache; /* caching cookie */
+#ifdef CONFIG_AFS_FSCACHE
+ struct fscache_cookie *cache; /* caching cookie */
#endif
wait_queue_head_t update_waitq; /* status fetch waitqueue */
diff --git a/fs/afs/volume.c b/fs/afs/volume.c
index 0ff4b86..0bd5578 100644
--- a/fs/afs/volume.c
+++ b/fs/afs/volume.c
@@ -15,10 +15,10 @@
#include <linux/slab.h>
#include <linux/fs.h>
#include <linux/pagemap.h>
+#include <linux/fscache.h>
#include "volume.h"
#include "vnode.h"
#include "cell.h"
-#include "cache.h"
#include "cmservice.h"
#include "fsclient.h"
#include "vlclient.h"
@@ -28,18 +28,14 @@
static const char *afs_voltypes[] = { "R/W", "R/O", "BAK" };
#endif
-#ifdef AFS_CACHING_SUPPORT
-static cachefs_match_val_t afs_volume_cache_match(void *target,
- const void *entry);
-static void afs_volume_cache_update(void *source, void *entry);
-
-struct cachefs_index_def afs_volume_cache_index_def = {
- .name = "volume",
- .data_size = sizeof(struct afs_cache_vhash),
- .keys[0] = { CACHEFS_INDEX_KEYS_BIN, 1 },
- .keys[1] = { CACHEFS_INDEX_KEYS_BIN, 1 },
- .match = afs_volume_cache_match,
- .update = afs_volume_cache_update,
+#ifdef CONFIG_AFS_FSCACHE
+static uint16_t afs_volume_cache_get_key(const void *cookie_netfs_data,
+ void *buffer, uint16_t buflen);
+
+static struct fscache_cookie_def afs_volume_cache_index_def = {
+ .name = "AFS.volume",
+ .type = FSCACHE_COOKIE_TYPE_INDEX,
+ .get_key = afs_volume_cache_get_key,
};
#endif
@@ -214,11 +210,10 @@ int afs_volume_lookup(const char *name,
}
/* attach the cache and volume location */
-#ifdef AFS_CACHING_SUPPORT
- cachefs_acquire_cookie(vlocation->cache,
- &afs_vnode_cache_index_def,
- volume,
- &volume->cache);
+#ifdef CONFIG_AFS_FSCACHE
+ volume->cache = fscache_acquire_cookie(vlocation->cache,
+ &afs_volume_cache_index_def,
+ volume);
#endif
afs_get_vlocation(vlocation);
@@ -286,8 +281,8 @@ void afs_put_volume(struct afs_volume *v
up_write(&vlocation->cell->vl_sem);
/* finish cleaning up the volume */
-#ifdef AFS_CACHING_SUPPORT
- cachefs_relinquish_cookie(volume->cache, 0);
+#ifdef CONFIG_AFS_FSCACHE
+ fscache_relinquish_cookie(volume->cache, 0);
#endif
afs_put_vlocation(vlocation);
@@ -481,40 +476,25 @@ int afs_volume_release_fileserver(struct
/*****************************************************************************/
/*
- * match a volume hash record stored in the cache
+ * set the key for the index entry
*/
-#ifdef AFS_CACHING_SUPPORT
-static cachefs_match_val_t afs_volume_cache_match(void *target,
- const void *entry)
+#ifdef CONFIG_AFS_FSCACHE
+static uint16_t afs_volume_cache_get_key(const void *cookie_netfs_data,
+ void *buffer, uint16_t bufmax)
{
- const struct afs_cache_vhash *vhash = entry;
- struct afs_volume *volume = target;
+ const struct afs_volume *volume = cookie_netfs_data;
+ uint16_t klen;
- _enter("{%u},{%u}", volume->type, vhash->vtype);
+ _enter("{%u},%p,%u", volume->type, buffer, bufmax);
- if (volume->type == vhash->vtype) {
- _leave(" = SUCCESS");
- return CACHEFS_MATCH_SUCCESS;
- }
-
- _leave(" = FAILED");
- return CACHEFS_MATCH_FAILED;
-} /* end afs_volume_cache_match() */
-#endif
-
-/*****************************************************************************/
-/*
- * update a volume hash record stored in the cache
- */
-#ifdef AFS_CACHING_SUPPORT
-static void afs_volume_cache_update(void *source, void *entry)
-{
- struct afs_cache_vhash *vhash = entry;
- struct afs_volume *volume = source;
+ klen = sizeof(volume->type);
+ if (klen > bufmax)
+ return 0;
- _enter("");
+ memcpy(buffer, &volume->type, sizeof(volume->type));
- vhash->vtype = volume->type;
+ _leave(" = %u", klen);
+ return klen;
-} /* end afs_volume_cache_update() */
+} /* end afs_volume_cache_get_key() */
#endif
diff --git a/fs/afs/volume.h b/fs/afs/volume.h
index bfdcf19..fc9895a 100644
--- a/fs/afs/volume.h
+++ b/fs/afs/volume.h
@@ -12,11 +12,11 @@
#ifndef _LINUX_AFS_VOLUME_H
#define _LINUX_AFS_VOLUME_H
+#include <linux/fscache.h>
#include "types.h"
#include "fsclient.h"
#include "kafstimod.h"
#include "kafsasyncd.h"
-#include "cache.h"
typedef enum {
AFS_VLUPD_SLEEP, /* sleeping waiting for update timer to fire */
@@ -45,24 +45,6 @@ struct afs_cache_vlocation
time_t rtime; /* last retrieval time */
};
-#ifdef AFS_CACHING_SUPPORT
-extern struct cachefs_index_def afs_vlocation_cache_index_def;
-#endif
-
-/*****************************************************************************/
-/*
- * volume -> vnode hash table entry
- */
-struct afs_cache_vhash
-{
- afs_voltype_t vtype; /* which volume variation */
- uint8_t hash_bucket; /* which hash bucket this represents */
-} __attribute__((packed));
-
-#ifdef AFS_CACHING_SUPPORT
-extern struct cachefs_index_def afs_volume_cache_index_def;
-#endif
-
/*****************************************************************************/
/*
* AFS volume location record
@@ -73,8 +55,8 @@ struct afs_vlocation
struct list_head link; /* link in cell volume location list */
struct afs_timer timeout; /* decaching timer */
struct afs_cell *cell; /* cell to which volume belongs */
-#ifdef AFS_CACHING_SUPPORT
- struct cachefs_cookie *cache; /* caching cookie */
+#ifdef CONFIG_AFS_FSCACHE
+ struct fscache_cookie *cache; /* caching cookie */
#endif
struct afs_cache_vlocation vldb; /* volume information DB record */
struct afs_volume *vols[3]; /* volume access record pointer (index by type) */
@@ -109,8 +91,8 @@ struct afs_volume
atomic_t usage;
struct afs_cell *cell; /* cell to which belongs (unrefd ptr) */
struct afs_vlocation *vlocation; /* volume location */
-#ifdef AFS_CACHING_SUPPORT
- struct cachefs_cookie *cache; /* caching cookie */
+#ifdef CONFIG_AFS_FSCACHE
+ struct fscache_cookie *cache; /* caching cookie */
#endif
afs_volid_t vid; /* volume ID */
afs_voltype_t type; /* type of volume */
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 3/7] FS-Cache: Avoid ENFILE checking for kernel-specific open files
2006-04-20 16:59 ` [PATCH 3/7] FS-Cache: Avoid ENFILE checking for kernel-specific open files David Howells
@ 2006-04-20 17:18 ` Christoph Hellwig
2006-04-20 18:06 ` David Howells
2006-04-21 0:07 ` Andrew Morton
1 sibling, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2006-04-20 17:18 UTC (permalink / raw)
To: David Howells
Cc: torvalds, akpm, steved, sct, aviro, linux-fsdevel, linux-cachefs,
nfsv4, linux-kernel
On Thu, Apr 20, 2006 at 05:59:33PM +0100, David Howells wrote:
> Make it possible to avoid ENFILE checking for kernel specific open files, such
> as are used by the CacheFiles module.
>
> After, for example, tarring up a kernel source tree over the network, the
> CacheFiles module may easily have 20000+ files open in the backing filesystem,
> thus causing all non-root processes to be given error ENFILE when they try to
> open a file, socket, pipe, etc..
No, just increase the limit. The whole point of the limit is to avoid resource
exaustion. A file doesn't use any less ressources just becuase it's opened
from kernelspace. In doubt increase the limit or even the default limit.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 4/7] FS-Cache: Export find_get_pages()
2006-04-20 16:59 ` [PATCH 4/7] FS-Cache: Export find_get_pages() David Howells
@ 2006-04-20 17:19 ` Christoph Hellwig
2006-04-20 17:45 ` David Howells
0 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2006-04-20 17:19 UTC (permalink / raw)
To: David Howells
Cc: torvalds, akpm, steved, sct, aviro, linux-fsdevel, linux-cachefs,
nfsv4, linux-kernel
On Thu, Apr 20, 2006 at 05:59:35PM +0100, David Howells wrote:
> The attached patch exports find_get_pages() for use by the kAFS filesystem in
> conjunction with it caching patch.
Why don't you use pagevec ?
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/7] FS-Cache: Add notification of page becoming writable to VMA ops
2006-04-20 16:59 ` [PATCH 2/7] FS-Cache: Add notification of page becoming writable to VMA ops David Howells
@ 2006-04-20 17:40 ` Zach Brown
2006-04-20 18:27 ` Anton Altaparmakov
0 siblings, 1 reply; 29+ messages in thread
From: Zach Brown @ 2006-04-20 17:40 UTC (permalink / raw)
To: David Howells
Cc: torvalds, akpm, steved, sct, aviro, linux-fsdevel, linux-cachefs,
nfsv4, linux-kernel
David Howells wrote:
> The attached patch adds a new VMA operation to notify a filesystem or other
> driver about the MMU generating a fault because userspace attempted to write
> to a page mapped through a read-only PTE.
This will almost certainly help OCFS2 get shared writable mmap() right,
too, though it probably won't be the whole story.
- z
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 4/7] FS-Cache: Export find_get_pages()
2006-04-20 17:19 ` Christoph Hellwig
@ 2006-04-20 17:45 ` David Howells
2006-04-21 0:15 ` Andrew Morton
0 siblings, 1 reply; 29+ messages in thread
From: David Howells @ 2006-04-20 17:45 UTC (permalink / raw)
To: Christoph Hellwig
Cc: David Howells, torvalds, akpm, steved, sct, aviro, linux-fsdevel,
linux-cachefs, nfsv4, linux-kernel
Christoph Hellwig <hch@infradead.org> wrote:
> > The attached patch exports find_get_pages() for use by the kAFS filesystem
> > in conjunction with it caching patch.
>
> Why don't you use pagevec ?
You mean pagevec_lookup() I suppose... That's probably reasonable, though
slower.
David
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/7] FS-Cache: Avoid ENFILE checking for kernel-specific open files
2006-04-20 17:18 ` Christoph Hellwig
@ 2006-04-20 18:06 ` David Howells
2006-04-21 0:11 ` Andrew Morton
0 siblings, 1 reply; 29+ messages in thread
From: David Howells @ 2006-04-20 18:06 UTC (permalink / raw)
To: Christoph Hellwig
Cc: David Howells, torvalds, akpm, steved, sct, aviro, linux-fsdevel,
linux-cachefs, nfsv4, linux-kernel
Christoph Hellwig <hch@infradead.org> wrote:
> > Make it possible to avoid ENFILE checking for kernel specific open files,
> > such as are used by the CacheFiles module.
> >
> > After, for example, tarring up a kernel source tree over the network, the
> > CacheFiles module may easily have 20000+ files open in the backing
> > filesystem, thus causing all non-root processes to be given error ENFILE
> > when they try to open a file, socket, pipe, etc..
>
> No, just increase the limit. The whole point of the limit is to avoid
> resource exaustion. A file doesn't use any less ressources just becuase
> it's opened from kernelspace. In doubt increase the limit or even the
> default limit.
As I saw it, the limit is there to prevent userspace from pinning too many
file resources. Yes, each userspace process is limited by its rlimit, but you
have to multiply that by the number of processes that can be around:
1024 * 32767 > 32 million files
Besides, you say "increase the limit", but there are two problems with that:
(1) Each AFS or NFS _dentry_ retained in the system pins a file in the
backing cache if it's also cached, whether or not it's open. So on my
desktop box, I've got about a million dentries cached. That means I
might also have anything up to a million files open... Except that the
netfs would be denied caching rights on any file beyond the ENFILE limit
- not that that matters, since you wouldn't be able to exec or open
anything if you weren't root - and that might include running su and the
like.
(2) And what should the limit actually _be_? You haven't said. If you
include the cache in the limit, you can at best open ENFILE limit / 2
files on the netfs before you get moaned at by the system... And then
closed files aren't immediately released by the cache, so you can quickly
find yourself backed into a corner...
With this patch, CacheFiles's consumption of files is controlled by the dcache
reclaimer and not by ENFILE, and allocating more files will cause other cache
files to be closed automatically.
The cache can't arbitrarily close backing files for which no userspace file
descriptor remains open: there may be dirty pages, or the file may be mmapped.
David
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/7] FS-Cache: Add notification of page becoming writable to VMA ops
2006-04-20 17:40 ` Zach Brown
@ 2006-04-20 18:27 ` Anton Altaparmakov
0 siblings, 0 replies; 29+ messages in thread
From: Anton Altaparmakov @ 2006-04-20 18:27 UTC (permalink / raw)
To: Zach Brown
Cc: David Howells, torvalds, akpm, steved, sct, aviro, linux-fsdevel,
linux-cachefs, nfsv4, linux-kernel
On Thu, 20 Apr 2006, Zach Brown wrote:
> David Howells wrote:
> > The attached patch adds a new VMA operation to notify a filesystem or other
> > driver about the MMU generating a fault because userspace attempted to write
> > to a page mapped through a read-only PTE.
>
> This will almost certainly help OCFS2 get shared writable mmap() right,
> too, though it probably won't be the whole story.
This is also required by NTFS for a correct implementation of "mmap write
into sparse region on ntfs volume with cluster size > PAGE_CACHE_SIZE".
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, http://www.linux-ntfs.org/
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/7] FS-Cache: Avoid ENFILE checking for kernel-specific open files
2006-04-20 16:59 ` [PATCH 3/7] FS-Cache: Avoid ENFILE checking for kernel-specific open files David Howells
2006-04-20 17:18 ` Christoph Hellwig
@ 2006-04-21 0:07 ` Andrew Morton
2006-04-21 12:33 ` David Howells
1 sibling, 1 reply; 29+ messages in thread
From: Andrew Morton @ 2006-04-21 0:07 UTC (permalink / raw)
To: David Howells
Cc: torvalds, steved, sct, aviro, linux-fsdevel, linux-cachefs, nfsv4,
linux-kernel
David Howells <dhowells@redhat.com> wrote:
>
> Make it possible to avoid ENFILE checking for kernel specific open files, such
> as are used by the CacheFiles module.
>
> After, for example, tarring up a kernel source tree over the network, the
> CacheFiles module may easily have 20000+ files open in the backing filesystem,
> thus causing all non-root processes to be given error ENFILE when they try to
> open a file, socket, pipe, etc..
>
> ...
>
>
> static struct percpu_counter nr_files __cacheline_aligned_in_smp;
> +static atomic_t nr_kernel_files;
So it's not performance-critical.
> -struct file *get_empty_filp(void)
> +struct file *get_empty_filp(int kernel)
I'd suggest a new get_empty_kernel_filp(void) rather than providing a magic
argument. (we can still have the magic argument in the new
__get_empty_filp(int), but it shouldn't be part of the caller-visible API).
> + if (!kernel) {
> + if (get_nr_files() >= files_stat.max_files &&
> + !capable(CAP_SYS_ADMIN)
> + ) {
ugly.
> + f->f_kernel_flags = kernel ? FKFLAGS_KERNEL : 0;
It would be more flexible to make the caller pass in the flags directly.
> f->f_uid = tsk->fsuid;
> f->f_gid = tsk->fsgid;
> eventpoll_init_file(f);
> @@ -235,6 +250,7 @@ struct file fastcall *fget_light(unsigne
> return file;
> }
>
> +EXPORT_SYMBOL(fget_light);
fget_light is not otherwise referenced in this patch.
this change is not changelogged.
why non-GPL?
> +EXPORT_SYMBOL(dentry_open_kernel);
_GPL?
> @@ -640,6 +643,7 @@ struct file {
> atomic_t f_count;
> unsigned int f_flags;
> mode_t f_mode;
> + unsigned short f_kernel_flags;
> loff_t f_pos;
> struct fown_struct f_owner;
> unsigned int f_uid, f_gid;
That's unfortunate. There's still room in f_flags. Was it hard to use that?
> @@ -943,7 +943,7 @@ static ctl_table fs_table[] = {
> .ctl_name = FS_NRFILE,
> .procname = "file-nr",
> .data = &files_stat,
> - .maxlen = 3*sizeof(int),
> + .maxlen = 4*sizeof(int),
> .mode = 0444,
> .proc_handler = &proc_nr_files,
This changes the format of /proc/sys/fs/file-nr. What will break?
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/7] FS-Cache: Avoid ENFILE checking for kernel-specific open files
2006-04-20 18:06 ` David Howells
@ 2006-04-21 0:11 ` Andrew Morton
2006-04-21 10:57 ` David Howells
0 siblings, 1 reply; 29+ messages in thread
From: Andrew Morton @ 2006-04-21 0:11 UTC (permalink / raw)
To: David Howells
Cc: hch, dhowells, torvalds, steved, sct, aviro, linux-fsdevel,
linux-cachefs, nfsv4, linux-kernel
David Howells <dhowells@redhat.com> wrote:
>
> (1) Each AFS or NFS _dentry_ retained in the system pins a file in the
> backing cache if it's also cached, whether or not it's open.
That would seem to be a great shortcoming in fscache.
I guess as memory reclaim reaps the top-level dentries those file*'s will
also be freed up, leading to their dentries becoming reclaimable, leading
to their inodes being reclaimable.
But still. Is it not possible to release those files-pinned-by-dcache when
the top-level files are closed?
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/7] FS-Cache: Provide a filesystem-specific sync'able page bit
2006-04-20 16:59 [PATCH 1/7] FS-Cache: Provide a filesystem-specific sync'able page bit David Howells
` (3 preceding siblings ...)
2006-04-20 16:59 ` [PATCH 6/7] FS-Cache: Make kAFS use FS-Cache David Howells
@ 2006-04-21 0:12 ` Andrew Morton
2006-04-21 10:22 ` David Howells
[not found] ` <20060420165937.9968.57149.stgit@warthog.cambridge.redhat.com>
[not found] ` <20060420165941.9968.13602.stgit@warthog.cambridge.redhat.com>
6 siblings, 1 reply; 29+ messages in thread
From: Andrew Morton @ 2006-04-21 0:12 UTC (permalink / raw)
To: David Howells
Cc: torvalds, steved, sct, aviro, linux-fsdevel, linux-cachefs, nfsv4,
linux-kernel
David Howells <dhowells@redhat.com> wrote:
>
> #define PG_checked 8 /* kill me in 2.5.<early>. */
> +#define PG_fs_misc 8
It would be better to rename PG_checked to PG_fs_misc kernel-wide.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 4/7] FS-Cache: Export find_get_pages()
2006-04-20 17:45 ` David Howells
@ 2006-04-21 0:15 ` Andrew Morton
2006-04-21 13:02 ` David Howells
0 siblings, 1 reply; 29+ messages in thread
From: Andrew Morton @ 2006-04-21 0:15 UTC (permalink / raw)
To: David Howells
Cc: hch, dhowells, torvalds, steved, sct, aviro, linux-fsdevel,
linux-cachefs, nfsv4, linux-kernel
David Howells <dhowells@redhat.com> wrote:
>
> Christoph Hellwig <hch@infradead.org> wrote:
>
> > > The attached patch exports find_get_pages() for use by the kAFS filesystem
> > > in conjunction with it caching patch.
> >
> > Why don't you use pagevec ?
>
> You mean pagevec_lookup() I suppose... That's probably reasonable, though
> slower.
>
But the code's using pagevecs now. In a strange manner.
+ nr_pages = find_get_pages(vnode->vfs_inode.i_mapping, first,
+ PAGEVEC_SIZE, pvec.pages);
that's an open-coded pagevec_lookup().
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 5/7] FS-Cache: Generic filesystem caching facility
[not found] ` <20060420165937.9968.57149.stgit@warthog.cambridge.redhat.com>
@ 2006-04-21 0:46 ` Andrew Morton
2006-04-21 14:15 ` David Howells
0 siblings, 1 reply; 29+ messages in thread
From: Andrew Morton @ 2006-04-21 0:46 UTC (permalink / raw)
To: David Howells
Cc: torvalds, steved, sct, aviro, linux-fsdevel, linux-cachefs, nfsv4,
linux-kernel
David Howells <dhowells@redhat.com> wrote:
>
> ...
>
> --- /dev/null
> +++ b/fs/fscache/cookie.c
> @@ -0,0 +1,1065 @@
> +/* cookie.c: general filesystem cache cookie management
> + *
> + * Copyright (C) 2004-5 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include "fscache-int.h"
> +
> +static LIST_HEAD(fscache_cache_tag_list);
> +static LIST_HEAD(fscache_cache_list);
> +static LIST_HEAD(fscache_netfs_list);
> +static DECLARE_RWSEM(fscache_addremove_sem);
> +static struct fscache_cache_tag fscache_nomem_tag;
> +
> +kmem_cache_t *fscache_cookie_jar;
> +
> +static void fscache_withdraw_object(struct fscache_cache *cache,
> + struct fscache_object *object);
> +
> +static void __fscache_cookie_put(struct fscache_cookie *cookie);
> +
> +static inline void fscache_cookie_put(struct fscache_cookie *cookie)
> +{
> +#ifdef CONFIG_DEBUG_SLAB
> + BUG_ON((atomic_read(&cookie->usage) & 0xffff0000) == 0x6b6b0000);
> +#endif
Interesting. Perhaps a comment in there?
> +/*****************************************************************************/
> +/*
> + * look up a cache tag
> + */
> +struct fscache_cache_tag *__fscache_lookup_cache_tag(const char *name)
> +{
> + struct fscache_cache_tag *tag, *xtag;
> +
> + /* firstly check for the existence of the tag under read lock */
> + down_read(&fscache_addremove_sem);
> +
> + list_for_each_entry(tag, &fscache_cache_tag_list, link) {
> + if (strcmp(tag->name, name) == 0) {
> + atomic_inc(&tag->usage);
> + up_read(&fscache_addremove_sem);
> + return tag;
> + }
> + }
> +
> + up_read(&fscache_addremove_sem);
> +
> + /* the tag does not exist - create a candidate */
> + xtag = kmalloc(sizeof(*tag) + strlen(name) + 1, GFP_KERNEL);
> + if (!xtag) {
> + /* return a dummy tag if out of memory */
> + up_read(&fscache_addremove_sem);
We already did an up_read().
> +/*****************************************************************************/
> +/*
> + * register a network filesystem for caching
> + */
> +int __fscache_register_netfs(struct fscache_netfs *netfs)
> +{
> + struct fscache_netfs *ptr;
> + int ret;
> +
> + _enter("{%s}", netfs->name);
> +
> + INIT_LIST_HEAD(&netfs->link);
> +
> + /* allocate a cookie for the primary index */
> + netfs->primary_index =
> + kmem_cache_alloc(fscache_cookie_jar, SLAB_KERNEL);
> +
> + if (!netfs->primary_index) {
> + _leave(" = -ENOMEM");
> + return -ENOMEM;
> + }
> +
> + /* initialise the primary index cookie */
> + memset(netfs->primary_index, 0, sizeof(*netfs->primary_index));
We have kmem_cache_zalloc() now.
> +EXPORT_SYMBOL(__fscache_register_netfs);
This code exports a huge number of symbols. Should they be non-GPL?
> +void fscache_withdraw_cache(struct fscache_cache *cache)
> +{
> + struct fscache_object *object;
> +
> + _enter("");
> +
> + printk(KERN_NOTICE
> + "FS-Cache: Withdrawing cache \"%s\"\n",
> + cache->tag->name);
> +
> + /* make the cache unavailable for cookie acquisition */
> + down_write(&cache->withdrawal_sem);
> +
> + down_write(&fscache_addremove_sem);
> + list_del_init(&cache->link);
> + cache->tag->cache = NULL;
> + up_write(&fscache_addremove_sem);
> +
> + /* mark all objects as being withdrawn */
> + spin_lock(&cache->object_list_lock);
> + list_for_each_entry(object, &cache->object_list, cache_link) {
> + set_bit(FSCACHE_OBJECT_WITHDRAWN, &object->flags);
> + }
> + spin_unlock(&cache->object_list_lock);
> +
> + /* make sure all pages pinned by operations on behalf of the netfs are
> + * written to disc */
> + cache->ops->sync_cache(cache);
> +
> + /* dissociate all the netfs pages backed by this cache from the block
> + * mappings in the cache */
> + cache->ops->dissociate_pages(cache);
> +
> + /* we now have to destroy all the active objects pertaining to this
> + * cache */
> + spin_lock(&cache->object_list_lock);
> +
> + while (!list_empty(&cache->object_list)) {
> + object = list_entry(cache->object_list.next,
> + struct fscache_object, cache_link);
> + list_del_init(&object->cache_link);
> + spin_unlock(&cache->object_list_lock);
> +
> + _debug("withdraw %p", object->cookie);
> +
> + /* we've extracted an active object from the tree - now dispose
> + * of it */
> + fscache_withdraw_object(cache, object);
> +
> + spin_lock(&cache->object_list_lock);
> + }
This looks livelockable.
> + spin_unlock(&cache->object_list_lock);
> +
> + fscache_release_cache_tag(cache->tag);
> + cache->tag = NULL;
> +
> + _leave("");
> +
> +} /* end fscache_withdraw_cache() */
> +
>
> ...
>
> +static struct fscache_object *fscache_lookup_object(struct fscache_cookie *cookie,
> + struct fscache_cache *cache)
> +{
> + struct fscache_cookie *parent = cookie->parent;
> + struct fscache_object *pobject, *object;
> + struct hlist_node *_p;
> +
> + _enter("{%s/%s},",
> + parent && parent->def ? parent->def->name : "",
> + cookie->def ? (char *) cookie->def->name : "<file>");
> +
> + if (test_bit(FSCACHE_IOERROR, &cache->flags))
> + return NULL;
> +
> + /* see if we have the backing object for this cookie + cache immediately
> + * to hand
> + */
> + object = NULL;
> + hlist_for_each_entry(object, _p,
> + &cookie->backing_objects, cookie_link
> + ) {
That's really weird-looking. How about
hlist_for_each_entry(object, _p, &cookie->backing_objects, cookie_link){
> + hlist_for_each_entry(pobject, _p,
> + &parent->backing_objects, cookie_link
> + ) {
> + if (pobject->cache == cache)
> + break;
> + }
Ditto.
> + if (!pobject) {
> + /* we don't know about the parent object */
> + up_read(&parent->sem);
> + down_write(&parent->sem);
> +
> + pobject = fscache_lookup_object(parent, cache);
> + if (IS_ERR(pobject)) {
> + up_write(&parent->sem);
> + _leave(" = %ld [no ipobj]", PTR_ERR(pobject));
> + return pobject;
> + }
> +
> + _debug("pobject=%p", pobject);
> +
> + BUG_ON(pobject->cookie != parent);
> +
> + downgrade_write(&parent->sem);
> + }
> +
> + /* now we can attempt to look up this object in the parent, possibly
> + * creating a representation on disc when we do so
> + */
> + object = cache->ops->lookup_object(cache, pobject, cookie);
> + up_read(&parent->sem);
> +
> + if (IS_ERR(object)) {
> + _leave(" = %ld [no obj]", PTR_ERR(object));
> + return object;
> + }
> +
> + /* keep track of it */
> + cache->ops->lock_object(object);
> +
> + BUG_ON(!hlist_unhashed(&object->cookie_link));
> +
> + /* attach to the cache's object list */
> + if (list_empty(&object->cache_link)) {
> + spin_lock(&cache->object_list_lock);
> + list_add(&object->cache_link, &cache->object_list);
> + spin_unlock(&cache->object_list_lock);
> + }
> +
> + /* attach to the cookie */
> + object->cookie = cookie;
> + atomic_inc(&cookie->usage);
> + hlist_add_head(&object->cookie_link, &cookie->backing_objects);
> +
> + /* done */
> + cache->ops->unlock_object(object);
I assume ->lock_object() provides the locking for the hlist_add_head()?
> + _leave(" = %p [new]", object);
> + return object;
> +
> +} /* end fscache_lookup_object() */
>
> ...
>
> +/*
> + * update the index entries backing a cookie
> + */
> +void __fscache_update_cookie(struct fscache_cookie *cookie)
> +{
> + struct fscache_object *object;
> + struct hlist_node *_p;
> +
> + if (cookie == FSCACHE_NEGATIVE_COOKIE) {
> + _leave(" [no cookie]");
> + return;
> + }
> +
> + _enter("{%s}", cookie->def->name);
> +
> + BUG_ON(!cookie->def->get_aux);
> +
> + down_write(&cookie->sem);
> + down_read(&cookie->parent->sem);
That's a surprising locking order. Normally things are parent-first.
> + /* update the index entry on disc in each cache backing this cookie */
> + hlist_for_each_entry(object, _p,
> + &cookie->backing_objects, cookie_link
> + ) {
> + if (!test_bit(FSCACHE_IOERROR, &object->cache->flags))
> + object->cache->ops->update_object(object);
> + }
> +
> + up_read(&cookie->parent->sem);
> + up_write(&cookie->sem);
> + _leave("");
> +
> +} /* end __fscache_update_cookie() */
> +
>
>...
>
> +int __fscache_pin_cookie(struct fscache_cookie *cookie)
> +{
> + struct fscache_object *object;
> + int ret;
> +
> + _enter("%p", cookie);
> +
> + if (hlist_empty(&cookie->backing_objects)) {
> + _leave(" = -ENOBUFS");
> + return -ENOBUFS;
> + }
> +
> + /* not supposed to use this for indexes */
> + BUG_ON(cookie->def->type == FSCACHE_COOKIE_TYPE_INDEX);
> +
> + /* prevent the file from being uncached whilst we access it and exclude
> + * read and write attempts on pages
> + */
> + down_write(&cookie->sem);
> +
> + ret = -ENOBUFS;
> + if (!hlist_empty(&cookie->backing_objects)) {
> + /* get and pin the backing object */
> + object = hlist_entry(cookie->backing_objects.first,
> + struct fscache_object, cookie_link);
> +
> + if (test_bit(FSCACHE_IOERROR, &object->cache->flags))
> + goto out;
> +
> + if (!object->cache->ops->pin_object) {
> + ret = -EOPNOTSUPP;
> + goto out;
> + }
> +
> + /* prevent the cache from being withdrawn */
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
trylock is often a sign that something is mucked up. A comment describing
why it is needed is always appropriate.
> + if (object->cache->ops->grab_object(object)) {
> + /* ask the cache to honour the operation */
> + ret = object->cache->ops->pin_object(object);
> +
> + object->cache->ops->put_object(object);
> + }
> +
> + up_read(&object->cache->withdrawal_sem);
> + }
> + }
> +
> +out:
> + up_write(&cookie->sem);
> + _leave(" = %d", ret);
> + return ret;
> +
> +} /* end __fscache_pin_cookie() */
> +
> +EXPORT_SYMBOL(__fscache_pin_cookie);
> +
> +/*****************************************************************************/
> +/*
> + * unpin an object into the cache
> + */
> +void __fscache_unpin_cookie(struct fscache_cookie *cookie)
> +{
> + struct fscache_object *object;
> + int ret;
> +
> + _enter("%p", cookie);
> +
> + if (hlist_empty(&cookie->backing_objects)) {
> + _leave(" [no obj]");
> + return;
> + }
> +
> + /* not supposed to use this for indexes */
> + BUG_ON(cookie->def->type == FSCACHE_COOKIE_TYPE_INDEX);
> +
> + /* prevent the file from being uncached whilst we access it and exclude
> + * read and write attempts on pages
> + */
> + down_write(&cookie->sem);
> +
> + ret = -ENOBUFS;
> + if (!hlist_empty(&cookie->backing_objects)) {
> + /* get and unpin the backing object */
> + object = hlist_entry(cookie->backing_objects.first,
> + struct fscache_object, cookie_link);
> +
> + if (test_bit(FSCACHE_IOERROR, &object->cache->flags))
> + goto out;
> +
> + if (!object->cache->ops->unpin_object)
> + goto out;
> +
> + /* prevent the cache from being withdrawn */
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
Ditto.
> + if (object->cache->ops->grab_object(object)) {
> + /* ask the cache to honour the operation */
> + object->cache->ops->unpin_object(object);
> +
> + object->cache->ops->put_object(object);
> + }
> +
> + up_read(&object->cache->withdrawal_sem);
> + }
> + }
> +
> +out:
> + up_write(&cookie->sem);
> + _leave("");
> +
> +} /* end __fscache_unpin_cookie() */
> +
>
>...
>
> +/*
> + * debug tracing
> + */
> +#define dbgprintk(FMT,...) \
> + printk("[%-6.6s] "FMT"\n",current->comm ,##__VA_ARGS__)
> +#define _dbprintk(FMT,...) do { } while(0)
> +
> +#define kenter(FMT,...) dbgprintk("==> %s("FMT")",__FUNCTION__ ,##__VA_ARGS__)
> +#define kleave(FMT,...) dbgprintk("<== %s()"FMT"",__FUNCTION__ ,##__VA_ARGS__)
> +#define kdebug(FMT,...) dbgprintk(FMT ,##__VA_ARGS__)
> +
> +#define kjournal(FMT,...) _dbprintk(FMT ,##__VA_ARGS__)
> +
> +#define dbgfree(ADDR) _dbprintk("%p:%d: FREEING %p",__FILE__,__LINE__,ADDR)
That's your fourth implementation of kenter(). Maybe we
need <linux/dhowells.h>?
> --- /dev/null
> +++ b/fs/fscache/fsdef.c
> @@ -0,0 +1,113 @@
> +/* fsdef.c: filesystem index definition
> + *
> + * Copyright (C) 2004 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include "fscache-int.h"
> +
> +static uint16_t fscache_fsdef_netfs_get_key(const void *cookie_netfs_data,
> + void *buffer, uint16_t bufmax);
> +
> +static uint16_t fscache_fsdef_netfs_get_aux(const void *cookie_netfs_data,
> + void *buffer, uint16_t bufmax);
u16 would be preferred.
> new file mode 100644
> index 0000000..613c3b1
> --- /dev/null
> +++ b/fs/fscache/main.c
> @@ -0,0 +1,150 @@
> +/* main.c: general filesystem caching manager
> + *
> + * Copyright (C) 2004 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/sched.h>
> +#include <linux/completion.h>
> +#include <linux/slab.h>
> +#include "fscache-int.h"
> +
> +int fscache_debug = 0;
unneeded initialisation
> +/*****************************************************************************/
> +/*
> + * initialise the fs caching module
> + */
> +static int fscache_init(void)
__init?
> +/*****************************************************************************/
> +/*
> + * release the ktype
> + */
> +static void fscache_ktype_release(struct kobject *kobject)
> +{
> +} /* end fscache_ktype_release() */
Don't empty kobject release() functions upset Greg?
> +#if 0
> +void __cyg_profile_func_enter (void *this_fn, void *call_site)
> +__attribute__((no_instrument_function));
> +
> +void __cyg_profile_func_enter (void *this_fn, void *call_site)
> +{
> + asm volatile(" movl %%esp,%%edi \n"
> + " andl %0,%%edi \n"
> + " addl %1,%%edi \n"
> + " movl %%esp,%%ecx \n"
> + " subl %%edi,%%ecx \n"
> + " shrl $2,%%ecx \n"
> + " movl $0xedededed,%%eax \n"
> + " rep stosl \n"
> + :
> + : "i"(~(THREAD_SIZE-1)), "i"(sizeof(struct thread_info))
> + : "eax", "ecx", "edi", "memory", "cc"
> + );
> +}
> +
> +void __cyg_profile_func_exit(void *this_fn, void *call_site)
> +__attribute__((no_instrument_function));
> +
> +void __cyg_profile_func_exit(void *this_fn, void *call_site)
> +{
> + asm volatile(" movl %%esp,%%edi \n"
> + " andl %0,%%edi \n"
> + " addl %1,%%edi \n"
> + " movl %%esp,%%ecx \n"
> + " subl %%edi,%%ecx \n"
> + " shrl $2,%%ecx \n"
> + " movl $0xdadadada,%%eax \n"
> + " rep stosl \n"
> + :
> + : "i"(~(THREAD_SIZE-1)), "i"(sizeof(struct thread_info))
> + : "eax", "ecx", "edi", "memory", "cc"
> + );
> +}
> +#endif
Removeable?
> diff --git a/fs/fscache/page.c b/fs/fscache/page.c
> new file mode 100644
> index 0000000..b197be7
> --- /dev/null
> +++ b/fs/fscache/page.c
> @@ -0,0 +1,548 @@
> +/* page.c: general filesystem cache cookie management
> + *
> + * Copyright (C) 2004-5 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/fscache-cache.h>
> +#include <linux/buffer_head.h>
> +#include <linux/pagevec.h>
> +#include "fscache-int.h"
> +
> +/*****************************************************************************/
> +/*
> + * set the data file size on an object in the cache
> + */
> +int __fscache_set_i_size(struct fscache_cookie *cookie, loff_t i_size)
> +{
> + struct fscache_object *object;
> + int ret;
> +
> + _enter("%p,%llu,", cookie, i_size);
> +
> + if (hlist_empty(&cookie->backing_objects)) {
> + _leave(" = -ENOBUFS");
> + return -ENOBUFS;
> + }
> +
> + /* not supposed to use this for indexes */
> + BUG_ON(cookie->def->type == FSCACHE_COOKIE_TYPE_INDEX);
> +
> + /* prevent the file from being uncached whilst we access it and exclude
> + * read and write attempts on pages
> + */
> + down_write(&cookie->sem);
> +
> + ret = -ENOBUFS;
> + if (!hlist_empty(&cookie->backing_objects)) {
> + /* get and pin the backing object */
> + object = hlist_entry(cookie->backing_objects.first,
> + struct fscache_object, cookie_link);
> +
> + if (test_bit(FSCACHE_IOERROR, &object->cache->flags))
> + goto out;
> +
> + /* prevent the cache from being withdrawn */
> + if (object->cache->ops->set_i_size &&
> + down_read_trylock(&object->cache->withdrawal_sem)
another trylock.
> + ) {
> + if (object->cache->ops->grab_object(object)) {
> + /* ask the cache to honour the operation */
> + ret = object->cache->ops->set_i_size(object,
> + i_size);
> +
> + object->cache->ops->put_object(object);
> + }
> +
> + up_read(&object->cache->withdrawal_sem);
> + }
> + }
> +
> +out:
> + up_write(&cookie->sem);
> + _leave(" = %d", ret);
> + return ret;
> +
> +} /* end __fscache_set_i_size() */
> +
> +EXPORT_SYMBOL(__fscache_set_i_size);
> +
> +/*****************************************************************************/
> +/*
> + * reserve space for an object
> + */
> +int __fscache_reserve_space(struct fscache_cookie *cookie, loff_t size)
> +{
> + struct fscache_object *object;
> + int ret;
> +
> + _enter("%p,%llu,", cookie, size);
> +
> + if (hlist_empty(&cookie->backing_objects)) {
> + _leave(" = -ENOBUFS");
> + return -ENOBUFS;
> + }
> +
> + /* not supposed to use this for indexes */
> + BUG_ON(cookie->def->type == FSCACHE_COOKIE_TYPE_INDEX);
> +
> + /* prevent the file from being uncached whilst we access it and exclude
> + * read and write attempts on pages
> + */
> + down_write(&cookie->sem);
> +
> + ret = -ENOBUFS;
> + if (!hlist_empty(&cookie->backing_objects)) {
> + /* get and pin the backing object */
> + object = hlist_entry(cookie->backing_objects.first,
> + struct fscache_object, cookie_link);
> +
> + if (test_bit(FSCACHE_IOERROR, &object->cache->flags))
> + goto out;
> +
> + if (!object->cache->ops->reserve_space) {
> + ret = -EOPNOTSUPP;
> + goto out;
> + }
> +
> + /* prevent the cache from being withdrawn */
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
and another.
> + if (object->cache->ops->grab_object(object)) {
> + /* ask the cache to honour the operation */
> + ret = object->cache->ops->reserve_space(object,
> + size);
> +
> + object->cache->ops->put_object(object);
> + }
> +
> + up_read(&object->cache->withdrawal_sem);
> + }
> + }
> +
> +out:
> + up_write(&cookie->sem);
> + _leave(" = %d", ret);
> + return ret;
> +
> +} /* end __fscache_reserve_space() */
> +
>
> ...
>
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
more.
> +
> +/* find the parent index object for a object */
> +static inline
> +struct fscache_object *fscache_find_parent_object(struct fscache_object *object)
> +{
> + struct fscache_object *parent;
> + struct fscache_cookie *cookie = object->cookie;
> + struct fscache_cache *cache = object->cache;
> + struct hlist_node *_p;
> +
> + hlist_for_each_entry(parent, _p,
> + &cookie->parent->backing_objects,
> + cookie_link
> + ) {
> + if (parent->cache == cache)
> + return parent;
> + }
> +
> + return NULL;
> +}
Large?
> +#endif /* _LINUX_FSCACHE_CACHE_H */
> diff --git a/include/linux/fscache.h b/include/linux/fscache.h
> new file mode 100644
> index 0000000..8aa464b
> --- /dev/null
> +++ b/include/linux/fscache.h
> @@ -0,0 +1,484 @@
> +/* fscache.h: general filesystem caching interface
> + *
> + * Copyright (C) 2004-5 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#ifndef _LINUX_FSCACHE_H
> +#define _LINUX_FSCACHE_H
> +
> +#include <linux/config.h>
> +#include <linux/fs.h>
> +#include <linux/list.h>
> +#include <linux/pagemap.h>
> +#include <linux/pagevec.h>
> +
> +#ifdef CONFIG_FSCACHE_MODULE
> +#define CONFIG_FSCACHE
> +#endif
Defining symbols which are owned by the Kconfig system isn't very nice.
> +
> +/*
> + * request a page be stored in the cache
> + * - this request may be ignored if no cache block is currently allocated, in
> + * which case it:
> + * - returns -ENOBUFS
> + * - if a cache block was already allocated:
> + * - a BIO will be dispatched to write the page (end_io_func will be called
> + * from the completion function)
> + * - returns 0
> + */
> +#ifdef CONFIG_FSCACHE
> +extern int __fscache_write_page(struct fscache_cookie *cookie,
> + struct page *page,
> + fscache_rw_complete_t end_io_func,
> + void *end_io_data,
> + gfp_t gfp);
hm. I tend to find it better for functions to be documented at the
definition site, not at the declaration site. I don't expect people even
think to go looking in a header file to learn about the function which
they're presently looking at.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 7/7] FS-Cache: CacheFiles: A cache that backs onto a mounted filesystem
[not found] ` <20060420165941.9968.13602.stgit@warthog.cambridge.redhat.com>
@ 2006-04-21 0:57 ` Andrew Morton
2006-04-21 1:16 ` Andrew Morton
1 sibling, 0 replies; 29+ messages in thread
From: Andrew Morton @ 2006-04-21 0:57 UTC (permalink / raw)
To: David Howells
Cc: torvalds, steved, sct, aviro, linux-fsdevel, linux-cachefs, nfsv4,
linux-kernel
David Howells <dhowells@redhat.com> wrote:
>
> + /* let keventd have some air occasionally */
> + max--;
> + if (max < 0 || need_resched()) {
> + if (!list_empty(&object->read_list))
> + schedule_work(&object->read_work);
> + _leave(" [maxed out]");
> + return;
> + }
That's perhaps not a terribly effective way of multiplexing keventd cycles.
If someone has done a schedule_work(), that will stick an entry onto
keventd's worklist, but it won't necessarily set need_resched().
We'd need to extend the workqueue API to be able to determine whether
there's other work pending.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 7/7] FS-Cache: CacheFiles: A cache that backs onto a mounted filesystem
[not found] ` <20060420165941.9968.13602.stgit@warthog.cambridge.redhat.com>
2006-04-21 0:57 ` [PATCH 7/7] FS-Cache: CacheFiles: A cache that backs onto a mounted filesystem Andrew Morton
@ 2006-04-21 1:16 ` Andrew Morton
2006-04-21 14:49 ` David Howells
1 sibling, 1 reply; 29+ messages in thread
From: Andrew Morton @ 2006-04-21 1:16 UTC (permalink / raw)
To: David Howells
Cc: torvalds, steved, sct, aviro, linux-fsdevel, linux-cachefs, nfsv4,
linux-kernel
David Howells <dhowells@redhat.com> wrote:
>
> ...
>
> + ret = 0;
> + }
> + else if (cachefiles_has_space(cache, 1) == 0) {
} else if (cachefiles_has_space(cache, 1) == 0) {
> + /* there's space in the cache we can use */
> + pagevec_add(&pagevec, page);
> + cookie->def->mark_pages_cached(cookie->netfs_data,
> + page->mapping, &pagevec);
> + ret = -ENODATA;
> + }
> + else {
} else {
(many instances)
> +unsigned long cachefiles_debug = 0;
Unneeded initialisation.
> +static int cachefiles_init(void)
__init?
> +#if 0
> +void __cyg_profile_func_enter (void *this_fn, void *call_site)
> +__attribute__((no_instrument_function));
> +
> +void __cyg_profile_func_enter (void *this_fn, void *call_site)
> +{
> + asm volatile(" movl %%esp,%%edi \n"
> + " andl %0,%%edi \n"
> + " addl %1,%%edi \n"
> + " movl %%esp,%%ecx \n"
> + " subl %%edi,%%ecx \n"
> + " shrl $2,%%ecx \n"
> + " movl $0xedededed,%%eax \n"
> + " rep stosl \n"
> + :
> + : "i"(~(THREAD_SIZE-1)), "i"(sizeof(struct thread_info))
> + : "eax", "ecx", "edi", "memory", "cc"
> + );
> +}
> +
> +void __cyg_profile_func_exit(void *this_fn, void *call_site)
> +__attribute__((no_instrument_function));
> +
> +void __cyg_profile_func_exit(void *this_fn, void *call_site)
> +{
> + asm volatile(" movl %%esp,%%edi \n"
> + " andl %0,%%edi \n"
> + " addl %1,%%edi \n"
> + " movl %%esp,%%ecx \n"
> + " subl %%edi,%%ecx \n"
> + " shrl $2,%%ecx \n"
> + " movl $0xdadadada,%%eax \n"
> + " rep stosl \n"
> + :
> + : "i"(~(THREAD_SIZE-1)), "i"(sizeof(struct thread_info))
> + : "eax", "ecx", "edi", "memory", "cc"
> + );
> +}
> +#endif
removeable?
> +/*
> + * delete an object representation from the cache
> + * - file backed objects are unlinked
> + * - directory backed objects are stuffed into the graveyard for userspace to
> + * delete
> + * - unlocks the directory mutex
> + */
> +static int cachefiles_bury_object(struct cachefiles_cache *cache,
> + struct dentry *dir,
> + struct dentry *rep)
> +{
> + struct dentry *grave, *alt, *trap;
> + struct qstr name;
> + const char *old_name;
> + char nbuffer[8 + 8 + 1];
> + int ret;
> +
> + _enter(",'%*.*s','%*.*s'",
> + dir->d_name.len, dir->d_name.len, dir->d_name.name,
> + rep->d_name.len, rep->d_name.len, rep->d_name.name);
> +
> + /* non-directories can just be unlinked */
> + if (!S_ISDIR(rep->d_inode->i_mode)) {
> + _debug("unlink stale object");
> + ret = dir->d_inode->i_op->unlink(dir->d_inode, rep);
> +
> + mutex_unlock(&dir->d_inode->i_mutex);
hm, what's going on here? It's strange for a callee to undo an i_mutex
which some caller took.
> EXPORT_SYMBOL(generic_file_buffered_write);
>
> +int
> +generic_file_buffered_write_one_kernel_page(struct file *file,
> + pgoff_t index,
> + struct page *src)
Some covering comments would be nice.
> +{
> + struct address_space *mapping = file->f_mapping;
> + struct address_space_operations *a_ops = mapping->a_ops;
> + struct pagevec lru_pvec;
> + struct page *page, *cached_page = NULL;
> + void *from, *to;
> + long status = 0;
> +
> + pagevec_init(&lru_pvec, 0);
> +
> + page = __grab_cache_page(mapping, index, &cached_page, &lru_pvec);
> + if (!page) {
> + BUG_ON(cached_page);
> + return -ENOMEM;
> + }
> +
> + status = a_ops->prepare_write(file, page, 0, PAGE_CACHE_SIZE);
> + if (unlikely(status)) {
> + loff_t isize = i_size_read(mapping->host);
If the hosts's i_mutex is held (it should be, but there are no comments)
then we can read inode->i_size directly. Minor thing.
> +
> + from = kmap_atomic(src, KM_USER0);
> + to = kmap_atomic(page, KM_USER1);
> + copy_page(to, from);
> + kunmap_atomic(from, KM_USER0);
> + kunmap_atomic(to, KM_USER1);
that's copy_highpage().
Sigh. It's all a huge pile of new code. And it's only used by AFS, the
number of users of which can be counted on the fingers of one foot. An NFS
implementation would make a testing phase much more useful.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/7] FS-Cache: Provide a filesystem-specific sync'able page bit
2006-04-21 0:12 ` [PATCH 1/7] FS-Cache: Provide a filesystem-specific sync'able page bit Andrew Morton
@ 2006-04-21 10:22 ` David Howells
2006-04-21 10:33 ` Andrew Morton
0 siblings, 1 reply; 29+ messages in thread
From: David Howells @ 2006-04-21 10:22 UTC (permalink / raw)
To: Andrew Morton
Cc: David Howells, torvalds, steved, sct, aviro, linux-fsdevel,
linux-cachefs, nfsv4, linux-kernel
Andrew Morton <akpm@osdl.org> wrote:
> It would be better to rename PG_checked to PG_fs_misc kernel-wide.
So would deleting PG_checked and changing the PageChecked() macros to:
#define PageChecked(page) PageFsMisc((page))
#define SetPageChecked(page) SetPageFsMisc((page))
#define ClearPageChecked(page) ClearPageFsMisc((page))
be acceptable? Or would you rather I replaced those too?
David
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/7] FS-Cache: Provide a filesystem-specific sync'able page bit
2006-04-21 10:22 ` David Howells
@ 2006-04-21 10:33 ` Andrew Morton
0 siblings, 0 replies; 29+ messages in thread
From: Andrew Morton @ 2006-04-21 10:33 UTC (permalink / raw)
To: David Howells
Cc: dhowells, torvalds, steved, sct, aviro, linux-fsdevel,
linux-cachefs, nfsv4, linux-kernel
David Howells <dhowells@redhat.com> wrote:
>
> Andrew Morton <akpm@osdl.org> wrote:
>
> > It would be better to rename PG_checked to PG_fs_misc kernel-wide.
>
> So would deleting PG_checked and changing the PageChecked() macros to:
>
> #define PageChecked(page) PageFsMisc((page))
> #define SetPageChecked(page) SetPageFsMisc((page))
> #define ClearPageChecked(page) ClearPageFsMisc((page))
>
> be acceptable? Or would you rather I replaced those too?
>
PG_checked is presently a misc bit which only filesystems use. So yes, I'd
say it's appropriate to remove PageChecked() and friends altogether.
That might break out-of-tree filesystems, but they'll work it out.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/7] FS-Cache: Avoid ENFILE checking for kernel-specific open files
2006-04-21 0:11 ` Andrew Morton
@ 2006-04-21 10:57 ` David Howells
0 siblings, 0 replies; 29+ messages in thread
From: David Howells @ 2006-04-21 10:57 UTC (permalink / raw)
To: Andrew Morton
Cc: David Howells, hch, torvalds, steved, sct, aviro, linux-fsdevel,
linux-cachefs, nfsv4, linux-kernel
Andrew Morton <akpm@osdl.org> wrote:
> That would seem to be a great shortcoming in fscache.
It's not something that's easy to get around. The file has to be "open" to be
able to do certain operations on it.
> I guess as memory reclaim reaps the top-level dentries those file*'s will
> also be freed up, leading to their dentries becoming reclaimable, leading
> to their inodes being reclaimable.
Exactly.
> But still. Is it not possible to release those files-pinned-by-dcache when
> the top-level files are closed?
No, for three (or maybe four) reasons:
(1) You assume there's a "top-level" file open. AFS lookup(), for example,
will read the cache to get the directory contents, but there will _not_
be an open top-level file.
We could open and close the cache file in each lookup(), but that could
be very bad for lookup performance.
(2) mmap() may still have the struct file open, even though the last close()
has happened.
(3) There may be pages not yet written to the cache outstanding. These
belong to the netfs *inode* not the netfs *file*. Whilst the flush or
release file operation could be made to wait for these, that's not
necessarily within their spec, and could take a long time. How far is
the flush op supposed to go anyway?
(4) It prevents the data file going away whilst we have a cookie for it
(someone might go into the cache and delete something they shouldn't).
Pinning the dentry might work just as well, I suppose, but it makes
little difference to the resource consumption.
We keep at least the dentry available so that we can honour the netfs's
requests to update the auxiliary data as quickly as possible.
David
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/7] FS-Cache: Avoid ENFILE checking for kernel-specific open files
2006-04-21 0:07 ` Andrew Morton
@ 2006-04-21 12:33 ` David Howells
2006-04-21 18:22 ` Andrew Morton
0 siblings, 1 reply; 29+ messages in thread
From: David Howells @ 2006-04-21 12:33 UTC (permalink / raw)
To: Andrew Morton
Cc: David Howells, torvalds, steved, sct, aviro, linux-fsdevel,
linux-cachefs, nfsv4, linux-kernel
Andrew Morton <akpm@osdl.org> wrote:
> > static struct percpu_counter nr_files __cacheline_aligned_in_smp;
> > +static atomic_t nr_kernel_files;
>
> So it's not performance-critical.
Hmmm... nowhere near as critical as the ENFILE accounting, plus the only place
we actually read it is for the sysctl file.
It could actually be dispensed with entirely, I suppose.
> > -struct file *get_empty_filp(void)
> > +struct file *get_empty_filp(int kernel)
>
> I'd suggest a new get_empty_kernel_filp(void) rather than providing a magic
> argument. (we can still have the magic argument in the new
> __get_empty_filp(int), but it shouldn't be part of the caller-visible API).
> ...
> It would be more flexible to make the caller pass in the flags directly.
So:
struct file *get_empty_kernel_filp(unsigned short flags);
which devolves to get_empty_filp() if flags == 0?
> > +EXPORT_SYMBOL(fget_light);
>
> fget_light is not otherwise referenced in this patch.
Good point. I'll move it into the cachefiles patch.
> > +EXPORT_SYMBOL(dentry_open_kernel);
>
> _GPL?
If you wish.
> That's unfortunate. There's still room in f_flags. Was it hard to use that?
Yeah... but the usage of f_flags is constrained by O_xxxx flags that are part
of the userspace interface. Using those up for purely kernel things is a bad
idea.
Note that I've not actually increased the size of the struct file - f_mode is
a 16-bit value, hence why I chose an unsigned short.
> This changes the format of /proc/sys/fs/file-nr. What will break?
As far as I can tell, not a lot. I've grepped through various etc, lib and
bin directories on my FC5 system, and the only match I've found is:
/usr/lib64/sa/sadc
I'll present the count through a separate file to make sure.
David
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 4/7] FS-Cache: Export find_get_pages()
2006-04-21 0:15 ` Andrew Morton
@ 2006-04-21 13:02 ` David Howells
0 siblings, 0 replies; 29+ messages in thread
From: David Howells @ 2006-04-21 13:02 UTC (permalink / raw)
To: Andrew Morton
Cc: David Howells, hch, torvalds, steved, sct, aviro, linux-fsdevel,
linux-cachefs, nfsv4, linux-kernel
Andrew Morton <akpm@osdl.org> wrote:
> that's an open-coded pagevec_lookup().
Whilst that's true, it is still slower to use pagevec_lookup(). But since you
insist, I'll do that anyway.
David
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 5/7] FS-Cache: Generic filesystem caching facility
2006-04-21 0:46 ` [PATCH 5/7] FS-Cache: Generic filesystem caching facility Andrew Morton
@ 2006-04-21 14:15 ` David Howells
2006-04-21 18:38 ` Andrew Morton
0 siblings, 1 reply; 29+ messages in thread
From: David Howells @ 2006-04-21 14:15 UTC (permalink / raw)
To: Andrew Morton
Cc: David Howells, torvalds, steved, sct, aviro, linux-fsdevel,
linux-cachefs, nfsv4, linux-kernel
Andrew Morton <akpm@osdl.org> wrote:
> Interesting. Perhaps a comment in there?
Yep. It might even be worth making the actual check in linux/slab.h.
> We already did an up_read().
Yep. Also the initialisation was done on the wrong variable (tag not
xtag)... which only worked because list_for_each_entry() always leaves a valid
pointer in tag.
> We have kmem_cache_zalloc() now.
It's not in the NFS tree I've just pulled. Of course, git may be being a
complete git again. I blame the original author:-)
> This looks livelockable.
How so? The bit at the top of the function makes sure that we can't get any
new objects.
> > + cache->ops->unlock_object(object);
>
> I assume ->lock_object() provides the locking for the hlist_add_head()?
Half of it... Cookie lookup and withdrawal has to deal with the interaction
between two locks:
(1) The cookie lock. This is taken first when approached from the netfs side.
(2) The object lock. This is taken first when approached from the cache side.
The cookie lock is superior to the netfs lock, and fscache_withdraw_object()
has to do lock reordering when withdrawing a cache from the system.
fscache_lookup_object() - to which you refer - must be called with the cookie
already write-locked.
> > + down_write(&cookie->sem);
> > + down_read(&cookie->parent->sem);
>
> That's a surprising locking order. Normally things are parent-first.
But the operation starts from the child cookie.
The order I've defined is that if both a cookie and its parent already exist,
then you must get the lock on the child before the lock on the parent if you
want both.
Lookup is okay, since the new cookie doesn't exist to the rest of the system
until we've finished, and should we need to walk up the index tree
instantiating indices, the locking is in the right order to just do that.
> > + /* prevent the cache from being withdrawn */
> > + if (down_read_trylock(&object->cache->withdrawal_sem)) {
>
> trylock is often a sign that something is mucked up. A comment describing
> why it is needed is always appropriate.
In this case, it's not something mucked up. The only thing that write-locks
cache->withdrawal_sem is the fscache_withdraw_cache(), and that then prevents
any further operations on that cache succeeding.
Obviously, before we withdraw the cache, down_read_trylock() will always
succeed, and after we withdraw the cache it will always fail; but not only
that, _whilst_ there's an operation in progress, the withdrawal will be made to
wait because that does down_write() *not* down_write_trylock().
I wonder if I should write some docs on the locking procedures used by
FS-Cache.
> That's your fourth implementation of kenter(). Maybe we
> need <linux/dhowells.h>?
:-)
Maybe I should move my debugging macros into include/linux, but then everyone
else would complain if their own versions weren't put in there, or would
complain if they were forced to use mine.
> u16 would be preferred.
As I recall, Linus said something like that uint16_t and co are fine in one's
own interfaces. I did actually ask him about that.
u16 should die really, IMO.
> > +int fscache_debug = 0;
>
> unneeded initialisation
Yep.
> > +static int fscache_init(void)
>
> __init?
Yep.
> Don't empty kobject release() functions upset Greg?
I hope so... then he might improve his docs:-)
The function would seem to be mandatory, but there really isn't anything for it
to do, since the object is static:-/
> Removeable?
Yeah. It's handy to have around for debugging, but it's very arch specific,
and can be added back later easily enough.
> Large?
Depends how you define "large".
It doesn't actually produce very much code.
> Defining symbols which are owned by the Kconfig system isn't very nice.
Kconfig is still broken:
warthog>grep -r CONFIG_FSCACHE include/linux/autoconf.h
#define CONFIG_FSCACHE_MODULE 1
warthog>
Modules that might depend on fscache need to know that it's there, and having
to double up every #if to detect both is stupid.
Would you suggest then:
#if defined(CONFIG_FSCACHE) || defined(CONFIG_FSCACHE_MODULE)
#define FSCACHE_AVAILABLE 1
#endif
> hm. I tend to find it better for functions to be documented at the
> definition site, not at the declaration site. I don't expect people even
> think to go looking in a header file to learn about the function which
> they're presently looking at.
Two points:
(1) The function they'll be looking for isn't in a .c file.
(2) They *should* be looking in Documentation/ where all the kernel's
interface functions are of course documented. They will find the
documentation for the FS-Cache interfaces there. I'll add obvious banners
to the tops of the header files telling them where to look. I can even
add them to the .c files, I suppose.
Now I admit that I haven't added docs for get_empty_filp(),
dentry_open_kernel() and suchlike because there's nowhere to actually add
them yet.
David
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 7/7] FS-Cache: CacheFiles: A cache that backs onto a mounted filesystem
2006-04-21 1:16 ` Andrew Morton
@ 2006-04-21 14:49 ` David Howells
0 siblings, 0 replies; 29+ messages in thread
From: David Howells @ 2006-04-21 14:49 UTC (permalink / raw)
To: Andrew Morton
Cc: David Howells, torvalds, steved, sct, aviro, linux-fsdevel,
linux-cachefs, nfsv4, linux-kernel
Andrew Morton <akpm@osdl.org> wrote:
> > +unsigned long cachefiles_debug = 0;
>
> Unneeded initialisation.
Yep.
> > +static int cachefiles_init(void)
>
> __init?
Yep.
> removeable?
Yep.
> hm, what's going on here? It's strange for a callee to undo an i_mutex
> which some caller took.
It happens occasionally. The problem here is that I want to call this from
three different places, but if I drop the mutex before calling the burial
function, I have to get the mutex again to do the unlink; but as it is, I have
to drop it before I can do the rename:-/
It's not nice, but...
You have to note also that the directory's i_mutex is quite important for
interacting with the daemon also. The wonders of working through an existing
filesystem, and the the wonders of co-operating with userspace.
> > +int
> > +generic_file_buffered_write_one_kernel_page(struct file *file,
> > + pgoff_t index,
> > + struct page *src)
>
> Some covering comments would be nice.
I copied those of generic_file_buffered_write() and rearranged them a bit:-)
I'll add a comment to my function.
> If the hosts's i_mutex is held (it should be, but there are no comments)
> then we can read inode->i_size directly. Minor thing.
Ah. Do we though? I just copied generic_file_buffered_write() and cut it
down. The same is done there. The comments at the top of that function
weren't exactly forthcoming on the preconditions for calling that function.
> that's copy_highpage().
Good point.
> Sigh. It's all a huge pile of new code. And it's only used by AFS, the
> number of users of which can be counted on the fingers of one foot. An NFS
> implementation would make a testing phase much more useful.
Yes... Whilst I have it working with NFS, the NFS anti-aliasing problems are
still there and still need to be sorted. I thought I'd got them nailed, but
then Trond changed his mind:-(
But that does not preclude putting what I can release up for review.
David
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/7] FS-Cache: Avoid ENFILE checking for kernel-specific open files
2006-04-21 12:33 ` David Howells
@ 2006-04-21 18:22 ` Andrew Morton
2006-04-21 19:29 ` David Howells
0 siblings, 1 reply; 29+ messages in thread
From: Andrew Morton @ 2006-04-21 18:22 UTC (permalink / raw)
To: David Howells
Cc: dhowells, torvalds, steved, sct, aviro, linux-fsdevel,
linux-cachefs, nfsv4, linux-kernel
David Howells <dhowells@redhat.com> wrote:
>
> > > +struct file *get_empty_filp(int kernel)
> >
> > I'd suggest a new get_empty_kernel_filp(void) rather than providing a magic
> > argument. (we can still have the magic argument in the new
> > __get_empty_filp(int), but it shouldn't be part of the caller-visible API).
> > ...
> > It would be more flexible to make the caller pass in the flags directly.
>
> So:
>
> struct file *get_empty_kernel_filp(unsigned short flags);
>
> which devolves to get_empty_filp() if flags == 0?
>
argh, I forgot about the flag. Oh well. I'd suggest:
static inline struct file *get_empty_filp(void)
{
return __get_empty_filp(0);
}
static inline struct file *get_empty_kernel_filp(void)
{
return __get_empty_filp(0);
}
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 5/7] FS-Cache: Generic filesystem caching facility
2006-04-21 14:15 ` David Howells
@ 2006-04-21 18:38 ` Andrew Morton
2006-04-21 19:33 ` David Howells
0 siblings, 1 reply; 29+ messages in thread
From: Andrew Morton @ 2006-04-21 18:38 UTC (permalink / raw)
To: David Howells
Cc: dhowells, torvalds, steved, sct, aviro, linux-fsdevel,
linux-cachefs, nfsv4, linux-kernel
David Howells <dhowells@redhat.com> wrote:
>
> > That's your fourth implementation of kenter(). Maybe we
> > need <linux/dhowells.h>?
>
> :-)
>
> Maybe I should move my debugging macros into include/linux, but then everyone
> else would complain if their own versions weren't put in there, or would
> complain if they were forced to use mine.
The number of home-made debugging macro implementations we have is quite
demented. Developing (and maintaining) a common set would be a good idea,
IMO.
> It doesn't actually produce very much code.
>
> > Defining symbols which are owned by the Kconfig system isn't very nice.
>
> Kconfig is still broken:
>
> warthog>grep -r CONFIG_FSCACHE include/linux/autoconf.h
> #define CONFIG_FSCACHE_MODULE 1
> warthog>
>
> Modules that might depend on fscache need to know that it's there,
In theory, module A isn't supposed to care whether module B was configured,
because module B might be compiled separately, or dowloaded from elsewhere
or whatever.
> and having
> to double up every #if to detect both is stupid.
>
> Would you suggest then:
>
> #if defined(CONFIG_FSCACHE) || defined(CONFIG_FSCACHE_MODULE)
> #define FSCACHE_AVAILABLE 1
> #endif
yup.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/7] FS-Cache: Avoid ENFILE checking for kernel-specific open files
2006-04-21 18:22 ` Andrew Morton
@ 2006-04-21 19:29 ` David Howells
0 siblings, 0 replies; 29+ messages in thread
From: David Howells @ 2006-04-21 19:29 UTC (permalink / raw)
To: Andrew Morton
Cc: David Howells, torvalds, steved, sct, aviro, linux-fsdevel,
linux-cachefs, nfsv4, linux-kernel
Andrew Morton <akpm@osdl.org> wrote:
> argh, I forgot about the flag. Oh well. I'd suggest:
I presume you mean for the second to pass an argument of 1 to
__get_empty_filp().
David
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 5/7] FS-Cache: Generic filesystem caching facility
2006-04-21 18:38 ` Andrew Morton
@ 2006-04-21 19:33 ` David Howells
0 siblings, 0 replies; 29+ messages in thread
From: David Howells @ 2006-04-21 19:33 UTC (permalink / raw)
To: Andrew Morton
Cc: David Howells, torvalds, steved, sct, aviro, linux-fsdevel,
linux-cachefs, nfsv4, linux-kernel
Andrew Morton <akpm@osdl.org> wrote:
> > Modules that might depend on fscache need to know that it's there,
>
> In theory, module A isn't supposed to care whether module B was configured,
> because module B might be compiled separately, or dowloaded from elsewhere
> or whatever.
In this case it's sort of necessary - unless you're suggesting I make FS-Cache
mandatory...
The problem is that I don't want NFS or whatever to be carrying around the
cookie pointers if FS-Cache isn't compiled as that saves memory. But that
involves conditionally changing the composition of structures, something
that's most clearly done with cpp-conditionals.
David
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2006-04-21 19:34 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-04-20 16:59 [PATCH 1/7] FS-Cache: Provide a filesystem-specific sync'able page bit David Howells
2006-04-20 16:59 ` [PATCH 2/7] FS-Cache: Add notification of page becoming writable to VMA ops David Howells
2006-04-20 17:40 ` Zach Brown
2006-04-20 18:27 ` Anton Altaparmakov
2006-04-20 16:59 ` [PATCH 3/7] FS-Cache: Avoid ENFILE checking for kernel-specific open files David Howells
2006-04-20 17:18 ` Christoph Hellwig
2006-04-20 18:06 ` David Howells
2006-04-21 0:11 ` Andrew Morton
2006-04-21 10:57 ` David Howells
2006-04-21 0:07 ` Andrew Morton
2006-04-21 12:33 ` David Howells
2006-04-21 18:22 ` Andrew Morton
2006-04-21 19:29 ` David Howells
2006-04-20 16:59 ` [PATCH 4/7] FS-Cache: Export find_get_pages() David Howells
2006-04-20 17:19 ` Christoph Hellwig
2006-04-20 17:45 ` David Howells
2006-04-21 0:15 ` Andrew Morton
2006-04-21 13:02 ` David Howells
2006-04-20 16:59 ` [PATCH 6/7] FS-Cache: Make kAFS use FS-Cache David Howells
2006-04-21 0:12 ` [PATCH 1/7] FS-Cache: Provide a filesystem-specific sync'able page bit Andrew Morton
2006-04-21 10:22 ` David Howells
2006-04-21 10:33 ` Andrew Morton
[not found] ` <20060420165937.9968.57149.stgit@warthog.cambridge.redhat.com>
2006-04-21 0:46 ` [PATCH 5/7] FS-Cache: Generic filesystem caching facility Andrew Morton
2006-04-21 14:15 ` David Howells
2006-04-21 18:38 ` Andrew Morton
2006-04-21 19:33 ` David Howells
[not found] ` <20060420165941.9968.13602.stgit@warthog.cambridge.redhat.com>
2006-04-21 0:57 ` [PATCH 7/7] FS-Cache: CacheFiles: A cache that backs onto a mounted filesystem Andrew Morton
2006-04-21 1:16 ` Andrew Morton
2006-04-21 14:49 ` David Howells
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox