Re: partial reiser4 review comments

From: Nate Diller
Date: Fri Aug 11 2006 - 19:00:11 EST


On 8/11/06, Hans Reiser <reiser@xxxxxxxxxxx> wrote:
Nate Diller wrote:

> On 8/9/06, Hans Reiser <reiser@xxxxxxxxxxx> wrote:
>
>> Christoph Hellwig wrote:
>>
>> > I must admit that standalone code snipplet doesn't really tell me a
>> lot.
>> >
>> >Do you mean the possibility to pass around a filesystem-defined
>> structure
>> >to multiple allocator calls? I'm pretty sure can add that, I though it
>> >would be useful multiple times in the past but always found ways around
>> >it.
>> >
>> >
>> >
>> Assuming I understand your discussion, I see two ways to go, one is to
>> pass around fs specific state and continue to call into the FS many
>> times, and the other is to instead provide the fs with helper functions
>> that accomplish readahead calculation, page allocation, etc., and let
>> the FS keep its state naturally without having to preserve it in some fs
>> defined structure. The second approach would be cleaner code design,
>> that would also ease cross-os porting of filesystems, in my view.
>
>
> the second approach is the one i was heading towards with my
> unfinished a_ops patches. *please* won't someone pay me to do that
> work...
>
> NATE
>
>
You might describe it in a paragraph or so instead of just mentioning
it.....;-)


start by making tree_lock (write) private, using the interface
detailed below. No one should be able to add/remove pages from the
address space without going through the a_ops interface. this patch
is part of a (much) larger unfinished, and outdated, set intended to
put this interface in place. people familiar with this code will
immediately note that there are a few hard problems to solve here,
most notably the various {truncate|invalidate}_mapping_pages calls,
and the locking involved. Cleaning up the inode reclaim paths a bit
should help this, and that work is unfinished as well.

I'm always hesitant to post stuff like this, because -ENOPATCH is
really an appropriate response here.

NATE

diff -urpN linux-2.6.15-rc5-mm1/include/linux/fs.h
linux-extent/include/linux/fs.h
--- linux-2.6.15-rc5-mm1/include/linux/fs.h 2005-12-10 16:49:30.000000000 -0800
+++ linux-extent/include/linux/fs.h 2006-08-11 15:47:19.000000000 -0700
@@ -340,18 +340,33 @@ struct address_space;
struct writeback_control;

struct address_space_operations {
- int (*writepage)(struct page *page, struct writeback_control *wbc);
- int (*readpage)(struct file *, struct page *);
- int (*sync_page)(struct page *);
-
- /* Write back some dirty pages from this mapping. */
- int (*writepages)(struct address_space *, struct writeback_control *);
-
/* Set a page dirty */
+ /* takes lock to move lists, update counts */
int (*set_page_dirty)(struct page *page);
+ /* get rid of this, all it does is unplug blkdev */
+ int (*sync_page)(struct page *);

+ /*
+ * Write back dirty pages / invalidate all pages that fall within
+ * the given page range (end byte inclusive). These only affect
+ * pages already cached in this mapping.
+ */
+ int (*writepages)(struct address_space *mapping,
+ struct writeback_control *wbc);
+ int (*invalidate_range)(struct address_space *mapping,
+ pgoff_t index, unsigned nr_pages);
+
+ /*
+ * Read / create pages within the given index extent. These
+ * silently skip any pages which are already cached in this mapping.
+ *
+ * Return the number of pages allocated within the range, or an error.
+ */
+ /* filp here is wierd */
int (*readpages)(struct file *filp, struct address_space *mapping,
- struct list_head *pages, unsigned nr_pages);
+ pgoff_t index, unsigned nr_pages);
+ int (*instantiate_range)(struct address_space *mapping, pgoff_t index,
+ unsigned nr_pages);

/*
* ext3 requires that a successful prepare_write() call be followed
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/