Re: [PATCH] ext3,4:fdatasync should skip metadata writeout

2007-11-16 Thread Bryan Henderson
fsync() will sync an inode even if only i_atime was changed.
fdatasync() would ignore such changes.  I guess atime was the major
reason for creating fdatasync() in the first place.

I think it was mtime.  One doesn't normally call any kind of sync when one 
is just reading the file.  But keeping an accurate mtime is often not 
worth the I/O.

And theoretically, there could be all kinds of truly meta metadata that 
changes as you write to the file but would probably be considered more 
expendable than the file's actual data.

But I think it was always intended that fdatasync() would sync the data in 
a meaningful way -- i.e. such that the data can be retrieved after a 
system failure; it surely wasn't meant for the user to understand 
filesystem internals.  I've heard the term data-related metadata to 
distinguish such things as allocation maps and pointer blocks from mtime, 
permissions, etc.

--
Bryan Henderson IBM Almaden Research Center
San Jose CA Filesystems

-
To unsubscribe from this list: send the line unsubscribe linux-ext4 in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] Add buffer head related helper functions

2007-11-16 Thread Andrew Morton
On Thu, 15 Nov 2007 17:40:43 +0530
Aneesh Kumar K.V [EMAIL PROTECTED] wrote:

 Add buffer head related helper function
 bh_uptodate_or_lock and bh_submit_read
 which can be used by file system
 

The patches look sane.

 ---
  include/linux/buffer_head.h |   29 +
  1 files changed, 29 insertions(+), 0 deletions(-)
 
 diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
 index da0d83f..82cc9ef 100644
 --- a/include/linux/buffer_head.h
 +++ b/include/linux/buffer_head.h
 @@ -327,6 +327,35 @@ static inline void lock_buffer(struct buffer_head *bh)
  
  extern int __set_page_dirty_buffers(struct page *page);
  
 +/* Return true if the buffer is up-to-date.
 + * Return false, with the buffer locked, if not.
 + */
 +static inline int bh_uptodate_or_lock(struct buffer_head *bh)
 +{
 + if (!buffer_uptodate(bh)) {
 + lock_buffer(bh);
 + if (!buffer_uptodate(bh))
 + return 0;
 + unlock_buffer(bh);
 + }
 + return 1;
 +}
 +/*
 + * Submit a locked buffer for reading,
 + * return a negative error and release
 + * the buffer if failed.
 + */
 +static inline int bh_submit_read(struct buffer_head *bh)
 +{
 + get_bh(bh);
 + bh-b_end_io = end_buffer_read_sync;
 + submit_bh(READ, bh);
 + wait_on_buffer(bh);
 + if (buffer_uptodate(bh))
 + return 0;
 + brelse(bh);
 + return -EIO;
 +}
  #else /* CONFIG_BLOCK */

But these are waay too big to be inlined.  Could I ask that they be
turned into regular EXPORT_SYMBOL()ed functions in buffer.c?

Might as well turn the comments into regular kerneldoc format, too.

The function names are a bit awkward, but I can't think of anything better.

I'm surprised that we don't already have a function which does what
bh_submit_read() does.

bh_submit_read() might become a bit simpler if it called ll_rw_block().

I'd have thought that to be more general, bh_submit_read() should return
immediately if the buffer is uptodate.  ll_rw_block() sort of helps there.


-
To unsubscribe from this list: send the line unsubscribe linux-ext4 in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html