inode->i_dirty_buffers redundant ?

Stephen C. Tweedie <[email protected]> wrote:
: Hi,

: On Wed, Jan 24, 2001 at 03:25:16PM +0530, V Ganesh wrote:
:> now that we have inode->i_mapping->dirty_pages, what do we need :> inode->i_dirty_buffers for ?

: Metadata. Specifically, directory contents and indirection blocks.

: --Stephen

ah, mark_buffer_dirty_inode(). thanks.
so if I understand it,
1. read() and mmap read faults will put the page in i_mapping->clean_pages
2. mmaped writes will (eventually, from msync or unmap or swapout) put the page in i_mapping->dirty_pages
3. write() will put pages into i_dirty_buffers (__block_commit_write() calls buffer_insert_inode_queue()).

so i_dirty_buffers contains buffer_heads of pages coming from write() as well as metadata buffers from mark_buffer_dirty_inode(). a dirty MAP_SHARED page which has been write()n to will potentially exist in both lists. won't doing a set_dirty_page() instead of buffer_insert_inode_queue() in __block_commit_write() make things much simpler ? then we'd have i_dirty_buffers having _only_ metadata, and all data pages in the i_mapping->*_pages lists.

ganesh
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Hi,

That would only complicate things: it would mean we'd have to scan both lists on fsync instead of just the one, for example. There are a number of places where we need buffer lists for dirty data anyway, such as for bdflush's background sync to disk. We also maintain the per-page buffer lists as caches of the virtual-to-physical mapping to avoid redundant bmap()ping. So, removing the buffer_heads which alias the page cache data isn't an option. Given that, it's as well to keep all the inode's dirty buffers in the one place.

Cheers,
Stephen
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Could you clarify that one, please?

--
Daniel
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Btw,

We probably want another kind of "IO buffer" abstraction for 2.5 which can support buffer's bigger than PAGE_SIZE.

Do you have any thoughts on that, Stephen?

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Daniel,

With "physical mapping" Stephen means on-disk block number.

If the buffer(s) for a page are mapped with valid information (ie BH_Mapped) we avoid calling get_block().

See?

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Hi,

The buffer contains a physical label for the block's location on disk. The page cache is indexed purely by logical location, so doing IO to/from the page cache requires us to lookup the physical locations of each block within the page.

Caching the buffer_heads for page cache pages means that once those lookups are done once, further IO on the same page can bypass the lookup and go straight to disk.

--Stephen
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Hi,

XFS is already doing this, with pagebufs being used in their cache, and kiobufs used for the IO submission.

--Stephen
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

OK, this was just a terminology problem - I normally say 'filesystem block' or just 'block' where you said 'physical', and if you say 'physical' to me, I'll hear 'physical memory address' :-/ I was also confused by 'bmap', which to me means the bad old way of generic_reading, and to you means 'mapping a buffer to a disk location'. Um. Which I would express as 'mapping a buffer to a block'.

Leaving that confusing behind, I am now working out an approach to mapping an inode's index blocks into the page cache. Probably, I am reinventing the wheel, but here it is. I plan to create another address_space mapping in the inode's ext2-private area, called i_index_mapping. This maps all possible index blocks onto the i_index_mapping, IOW, for each possible data block we can directly compute the index into the i_index_mapping at which we will search for the datablock's parent index block. Normally we will find the index block there (because it was read recently) and not have to search down through its parents as we must with the buffer cache approach. Perhaps then the inode->dirty_buffers list can go away.

Obviously the impact on ext2_get_block is fairly major, but I think the end result will be simpler than what we have now.

--
Daniel
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/