Skip to contentSkip to conversation

do_generic_file_read / readahead adjustments

PATCH In review David Howells 4 participants 6 messages Oct 16, 2002

Subsystems (5)
Memory management VFS MM core Page cache +1 more subsystems
lore
Share & tools
lore

David Howells

author
#0 lore raw

PATCH 1/1 do_generic_file_read / readahead adjustments

The attached patch does the following three things:

(1) Makes the functions in mm/readahead.c only use struct file* to pass to readpage(). address_mapping* and file_ra_state* are used instead to keep track of readahead stuff.

(2) Adds a new function do_generic_mapping_read() that is similar to do_generic_file_read(), except that it uses a mapping pointer and a readahead state pointer to access a file. The file* is only used to pass to readpage().

(3) Turns do_generic_file_read() into an inline function in linux/fs.h that simply wraps do_generic_mapping_read().

This should mean that it is no longer necessary to have a struct file to access a file in this manner. Just an inode or address space should be sufficient.

It also means alternate read-ahead structures can be maintained.

It may be worth adding a read-ahead struct init function too.

David

include/linux/fs.h +14 -1
diff -u -x '*.o' -x '*.cmd' -x '*~' linux-2.5.43/include/linux/fs.h linux-readahead-2543/include/linux/fs.h
@@ -1251,7 +1251,8 @@
 ssize_t generic_file_write_nolock(struct file *file, const struct iovec *iov,
 				unsigned long nr_segs, loff_t *ppos);
 extern ssize_t generic_file_sendfile(struct file *, struct file *, loff_t *, size_t);
-extern void do_generic_file_read(struct file *, loff_t *, read_descriptor_t *, read_actor_t);
+extern void do_generic_mapping_read(struct address_space *, struct file_ra_state *, struct file *,
+				    loff_t *, read_descriptor_t *, read_actor_t);
 extern ssize_t generic_file_direct_IO(int rw, struct file *file,
 	const struct iovec *iov, loff_t offset, unsigned long nr_segs);
 extern int generic_direct_IO(int rw, struct inode *inode, const struct iovec 

@@ -1268,6 +1269,18 @@
 extern int generic_vm_writeback(struct page *page,
 				struct writeback_control *wbc);
 
+static inline void do_generic_file_read(struct file * filp, loff_t *ppos,
+					read_descriptor_t * desc,
+					read_actor_t actor)
+{
+	do_generic_mapping_read(filp->f_dentry->d_inode->i_mapping,
+				&filp->f_ra,
+				filp,
+				ppos,
+				desc,
+				actor);
+}
+
 extern struct file_operations generic_ro_fops;
 
 extern int vfs_readlink(struct dentry *, char *, int, const char *);
include/linux/mm.h +11 -4
diff -u -x '*.o' -x '*.cmd' -x '*~' linux-2.5.43/include/linux/mm.h linux-readahead-2543/include/linux/mm.h
@@ -513,11 +513,18 @@
 /* readahead.c */
 #define VM_MAX_READAHEAD	128	/* kbytes */
 #define VM_MIN_READAHEAD	16	/* kbytes (includes current page) */
-int do_page_cache_readahead(struct file *file,
+int do_page_cache_readahead(struct address_space *mapping, struct file *filp,
 			unsigned long offset, unsigned long nr_to_read);
-void page_cache_readahead(struct file *file, unsigned long offset);
-void page_cache_readaround(struct file *file, unsigned long offset);
-void handle_ra_miss(struct file *file);
+void page_cache_readahead(struct address_space *mapping, 
+			  struct file_ra_state *ra,
+			  struct file *filp,
+			  unsigned long offset);
+void page_cache_readaround(struct address_space *mapping, 
+			   struct file_ra_state *ra,
+			   struct file *filp,
+			   unsigned long offset);
+void handle_ra_miss(struct address_space *mapping, 
+		    struct file_ra_state *ra);
 
 /* Do stack extension */
 extern int expand_stack(struct vm_area_struct * vma, unsigned long address);
kernel/ksyms.c +1 -1
diff -ur -x '*.o' -x '*.cmd' -x '*~' linux-2.5.43/kernel/ksyms.c linux-readahead-2543/kernel/ksyms.c
@@ -229,7 +229,7 @@
 EXPORT_SYMBOL(generic_block_bmap);
 EXPORT_SYMBOL(generic_file_read);
 EXPORT_SYMBOL(generic_file_sendfile);
-EXPORT_SYMBOL(do_generic_file_read);
+EXPORT_SYMBOL(do_generic_mapping_read);
 EXPORT_SYMBOL(generic_file_write);
 EXPORT_SYMBOL(generic_file_write_nolock);
 EXPORT_SYMBOL(generic_file_mmap);
mm/filemap.c +17 -11
diff -ur -x '*.o' -x '*.cmd' -x '*~' linux-2.5.43/mm/filemap.c linux-readahead-2543/mm/filemap.c
@@ -572,10 +572,14 @@
  * This is really ugly. But the goto's actually try to clarify some
  * of the logic when it comes to error handling etc.
  * - note the struct file * is only passed for the use of readpage
  */
-void do_generic_file_read(struct file * filp, loff_t *ppos, read_descriptor_t * desc, read_actor_t actor)
+void do_generic_mapping_read(struct address_space *mapping,
+			     struct file_ra_state *ra,
+			     struct file * filp,
+			     loff_t *ppos,
+			     read_descriptor_t * desc,
+			     read_actor_t actor)
 {
-	struct address_space *mapping = filp->f_dentry->d_inode->i_mapping;
 	struct inode *inode = mapping->host;
 	unsigned long index, offset;
 	struct page *cached_page;

@@ -599,7 +603,7 @@
 				break;
 		}
 
-		page_cache_readahead(filp, index);
+		page_cache_readahead(mapping, ra, filp, index);
 
 		nr = nr - offset;
 

@@ -611,7 +615,7 @@
 		page = radix_tree_lookup(&mapping->page_tree, index);
 		if (!page) {
 			read_unlock(&mapping->page_lock);
-			handle_ra_miss(filp);
+			handle_ra_miss(mapping,ra);
 			goto no_cached_page;
 		}
 		page_cache_get(page);

@@ -947,9 +951,9 @@
 }
 
 static ssize_t
-do_readahead(struct file *file, unsigned long index, unsigned long nr)
+do_readahead(struct address_space *mapping, struct file *filp,
+	     unsigned long index, unsigned long nr)
 {
-	struct address_space *mapping = file->f_dentry->d_inode->i_mapping;
 	unsigned long max;
 	unsigned long active;
 	unsigned long inactive;

@@ -963,7 +967,7 @@
 	if (nr > max)
 		nr = max;
 
-	do_page_cache_readahead(file, index, nr);
+	do_page_cache_readahead(mapping, filp, index, nr);
 	return 0;
 }
 

@@ -976,10 +980,11 @@
 	file = fget(fd);
 	if (file) {
 		if (file->f_mode & FMODE_READ) {
+			struct address_space *mapping = file->f_dentry->d_inode->i_mapping;
 			unsigned long start = offset >> PAGE_CACHE_SHIFT;
 			unsigned long end = (offset + count - 1) >> PAGE_CACHE_SHIFT;
 			unsigned long len = end - start + 1;
-			ret = do_readahead(file, start, len);
+			ret = do_readahead(mapping, file, start, len);
 		}
 		fput(file);
 	}

@@ -1000,6 +1005,7 @@
 	int error;
 	struct file *file = area->vm_file;
 	struct address_space *mapping = file->f_dentry->d_inode->i_mapping;
+	struct file_ra_state *ra = &file->f_ra;
 	struct inode *inode = mapping->host;
 	struct page *page;
 	unsigned long size, pgoff, endoff;

@@ -1032,7 +1038,7 @@
 	 */
 	if (VM_SequentialReadHint(area)) {
 		did_readahead = 1;
-		page_cache_readahead(area->vm_file, pgoff);
+		page_cache_readahead(mapping, ra, file, pgoff);
 	}
 
 	/*

@@ -1041,7 +1047,7 @@
 	 */
 	if ((pgoff < size) && !VM_RandomReadHint(area)) {
 		did_readahead = 1;
-		page_cache_readaround(file, pgoff);
+		page_cache_readaround(mapping, ra, file, pgoff);
 	}
 
 	/*

@@ -1051,7 +1057,7 @@
 	page = find_get_page(mapping, pgoff);
 	if (!page) {
 		if (did_readahead) {
-			handle_ra_miss(file);
+			handle_ra_miss(mapping,ra);
 			did_readahead = 0;
 		}
 		goto no_cached_page;
mm/madvise.c +1 -1
diff -ur -x '*.o' -x '*.cmd' -x '*~' linux-2.5.43/mm/madvise.c linux-readahead-2543/mm/madvise.c
@@ -80,7 +80,7 @@
 	if ((vma->vm_mm->rss + (end - start)) > rlim_rss)
 		return error;
 
-	do_page_cache_readahead(file, start, end - start);
+	do_page_cache_readahead(file->f_dentry->d_inode->i_mapping, file, start, end - start);
 	return 0;
 }
mm/readahead.c +23 -23

128 lines not on this page · raw message

#1 lore raw
Quoted 7 lines · David Howells

The attached patch does the following three things:

(1) Makes the functions in mm/readahead.c only use struct file* to pass to readpage(). address_mapping* and file_ra_state* are used instead to keep track of readahead stuff.

Quoted 8 lines

(2) Adds a new function do_generic_mapping_read() that is similar to do_generic_file_read(), except that it uses a mapping pointer and a readahead state pointer to access a file. The file* is only used to pass to readpage().

(3) Turns do_generic_file_read() into an inline function in linux/fs.h that simply wraps do_generic_mapping_read().

Seems sensible. Is there something out there which actually uses this?

#2 lore raw
Quoted 5 lines · Andrew Morton
Quoted 2 lines · David Howells

The attached patch does the following three things:

Seems sensible. Is there something out there which actually uses this?

I think David has a few things up his sleeve. The patch looks sensible.

Rik

Signature · 4 lines
A: No.
Q: Should I include quotations after my reply?

http://www.surriel.com/		http://distro.conectiva.com/
#3 lore raw
Quoted 10 lines · Andrew Morton
Quoted 5 lines
Quoted 2 lines · David Howells

The attached patch does the following three things:

Seems sensible. Is there something out there which actually uses this?

I think David has a few things up his sleeve. The patch looks sensible.

I'm writing a general cache for filesystems such as AFS, NFSv4, and Lustre. Block devices are made available to the "cache manager" by means of a filesystem that can be mounted. I'm storing meta data in an inode in the cache, but to scan this at the moment I need to gain a "struct file" to use with do_generic_file_read().

This involves either creating a dummy dentry and struct file (which will cause Al Viro to come looking for me with a shotgun), or to use an extra auxilliary filesystem mounted with do_kern_mount(), neither of which are particularly appealing.

This patch is the alternative: make a function (do_generic_mapping_read()) that I can pass an inode or an address_space to, and make do_generic_file_read() call that. This allows me to make use of readahead semantics without having to reinvent them for myself.

David

#4 lore raw
Quoted 6 lines · David Howells

This patch is the alternative: make a function (do_generic_mapping_read()) that I can pass an inode or an address_space to, and make do_generic_file_read() call that. This allows me to make use of readahead semantics without having to reinvent them for myself.

OK. The current readahead and mpage code is really designed just for ext2-style filesystems. It was always expected that it would have to grow as more sophisticated filesytems put demands upon it.

Your change is a perfectly sensible generalisation. The reiserfs team have been making noises about lower-level readahead hooks as well, and I think your patch largely addresses those. I shall ping them.

#5 lore raw
Quoted 9 lines
Quoted 6 lines · David Howells

This patch is the alternative: make a function (do_generic_mapping_read()) that I can pass an inode or an address_space to, and make do_generic_file_read() call that. This allows me to make use of readahead semantics without having to reinvent them for myself.

Quoted 7 lines

OK. The current readahead and mpage code is really designed just for ext2-style filesystems. It was always expected that it would have to grow as more sophisticated filesytems put demands upon it.

Your change is a perfectly sensible generalisation. The reiserfs team have been making noises about lower-level readahead hooks as well, and I think your patch largely addresses those. I shall ping them.

I'd just finished sending the patch to Linus. If you want to divert or stall it, can you tell him.

David