]> git.neil.brown.name Git - history.git/commitdiff
NTFS 2.0.3: Small bug fixes, cleanups, and performance improvements.
authorAnton Altaparmakov <aia21@cantab.net>
Fri, 26 Apr 2002 14:11:12 +0000 (15:11 +0100)
committerAnton Altaparmakov <aia21@cantab.net>
Fri, 26 Apr 2002 14:11:12 +0000 (15:11 +0100)
- Remove some dead code from mft.c.
- Optimize readpage and read_block functions throughout aops.c so that
  only initialized blocks are read. Non-initialized ones have their
  buffer head mapped, zeroed, and set up to date, without scheduling
  any i/o. Thanks to Al Viro for advice on how to avoid the device i/o.
Thanks go to Andrew Morton for spotting the below:
- Fix buglet in allocate_compression_buffers() error code path.
- Call flush_dcache_page() after modifying page cache page contents in
  ntfs_file_readpage().
- Check for existence of page buffers throughout aops.c before calling
  create_empty_buffers(). This happens when an I/O error occurs and the
  read is retried. (It also happens once writing is implemented so that
  needed doing anyway but I had left it for later...)
- Don't BUG_ON() uptodate and/or mapped buffers throughout aops.c in
  readpage and read_block functions. Reasoning same as above (i.e. I/O
  error retries and future write code paths.)

Documentation/filesystems/ntfs.txt
fs/ntfs/ChangeLog
fs/ntfs/Makefile
fs/ntfs/aops.c
fs/ntfs/compress.c
fs/ntfs/mft.c

index 7159cbc5ccac669bade0406580becb1b8b5ab2a5..aab204881c23aecaa0a34852675b93763c770f4b 100644 (file)
@@ -168,6 +168,8 @@ ChangeLog
 
 Note that a technical ChangeLog aimed at kernel hackers is in fs/ntfs/ChangeLog.
 
+2.0.3:
+       - Small bug fixes, cleanups, and performance improvements.
 2.0.2:
        - Use default fmask of 0177 so that files are no executable by default.
          If you want owner executable files, just use fmask=0077.
@@ -177,7 +179,6 @@ Note that a technical ChangeLog aimed at kernel hackers is in fs/ntfs/ChangeLog.
 2.0.1:
        - Minor updates, primarily set the executable bit by default on files
          so they can be executed.
-
 2.0.0:
        - Started ChangeLog.
 
index 5c1f457c2ee44a425ec1c61def00e484922b7acd..118c75010fce2227d3288c68741403618b93b6d0 100644 (file)
@@ -27,6 +27,25 @@ ToDo:
          quite big. Modularising them a bit, e.g. a-la get_block(), will make
          them cleaner and make code reuse easier.
 
+2.0.3 - Small bug fixes, cleanups, and performance improvements.
+
+       - Remove some dead code from mft.c.
+       - Optimize readpage and read_block functions throughout aops.c so that
+         only initialized blocks are read. Non-initialized ones have their
+         buffer head mapped, zeroed, and set up to date, without scheduling
+         any i/o. Thanks to Al Viro for advice on how to avoid the device i/o.
+       Thanks go to Andrew Morton for spotting the below:
+       - Fix buglet in allocate_compression_buffers() error code path.
+       - Call flush_dcache_page() after modifying page cache page contents in
+         ntfs_file_readpage().
+       - Check for existence of page buffers throughout aops.c before calling
+         create_empty_buffers(). This happens when an I/O error occurs and the
+         read is retried. (It also happens once writing is implemented so that
+         needed doing anyway but I had left it for later...)
+       - Don't BUG_ON() uptodate and/or mapped buffers throughout aops.c in
+         readpage and read_block functions. Reasoning same as above (i.e. I/O
+         error retries and future write code paths.)
+
 2.0.2 - Minor updates and cleanups.
 
        - Cleanup: rename mst.c::__post_read_mst_fixup to post_write_mst_fixup
index 2323122ac86ce908df64d7d003d109dea44b6ed3..3485df2de6ef8693e06377352c0aaed8e062c752 100644 (file)
@@ -7,7 +7,7 @@ obj-y   := time.o unistr.o inode.o file.o mft.o super.o debug.o aops.o \
 
 obj-m   := $(O_TARGET)
 
-EXTRA_CFLAGS = -DNTFS_VERSION=\"2.0.2\"
+EXTRA_CFLAGS = -DNTFS_VERSION=\"2.0.3\"
 
 ifeq ($(CONFIG_NTFS_DEBUG),y)
 EXTRA_CFLAGS += -DDEBUG
index 999f777879004bf18d6c917c59b06d1d1822ae9d..0eeae5c5964899577cf42ac109b6e84b0a915685 100644 (file)
@@ -1,4 +1,4 @@
-/*
+/**
  * aops.c - NTFS kernel address space operations and page cache handling.
  *         Part of the Linux-NTFS project.
  *
@@ -35,7 +35,9 @@
 #define page_buffers(page)     (page)->buffers
 #endif
 
-/*
+/**
+ * end_buffer_read_file_async -
+ *
  * Async io completion handler for accessing files. Adapted from
  * end_buffer_read_mst_async().
  */
@@ -94,7 +96,11 @@ still_busy:
        return;
 }
 
-/* NTFS version of block_read_full_page(). Adapted from ntfs_mst_readpage(). */
+/**
+ * ntfs_file_read_block -
+ *
+ * NTFS version of block_read_full_page(). Adapted from ntfs_mst_readpage().
+ */
 static int ntfs_file_read_block(struct page *page)
 {
        VCN vcn;
@@ -102,7 +108,7 @@ static int ntfs_file_read_block(struct page *page)
        ntfs_inode *ni;
        ntfs_volume *vol;
        struct buffer_head *bh, *head, *arr[MAX_BUF_PER_PAGE];
-       sector_t iblock, lblock;
+       sector_t iblock, lblock, zblock;
        unsigned int blocksize, blocks, vcn_ofs;
        int i, nr;
        unsigned char blocksize_bits;
@@ -113,7 +119,8 @@ static int ntfs_file_read_block(struct page *page)
        blocksize_bits = VFS_I(ni)->i_blkbits;
        blocksize = 1 << blocksize_bits;
 
-       create_empty_buffers(page, blocksize);
+       if (!page_has_buffers(page))
+               create_empty_buffers(page, blocksize);
        bh = head = page_buffers(page);
        if (!bh)
                return -ENOMEM;
@@ -121,6 +128,7 @@ static int ntfs_file_read_block(struct page *page)
        blocks = PAGE_CACHE_SIZE >> blocksize_bits;
        iblock = page->index << (PAGE_CACHE_SHIFT - blocksize_bits);
        lblock = (ni->allocated_size + blocksize - 1) >> blocksize_bits;
+       zblock = (ni->initialized_size + blocksize - 1) >> blocksize_bits;
 
 #ifdef DEBUG
        if (unlikely(!ni->mft_no)) {
@@ -133,7 +141,12 @@ static int ntfs_file_read_block(struct page *page)
        /* Loop through all the buffers in the page. */
        nr = i = 0;
        do {
-               BUG_ON(buffer_mapped(bh) || buffer_uptodate(bh));
+               if (unlikely(buffer_uptodate(bh)))
+                       continue;
+               if (unlikely(buffer_mapped(bh))) {
+                       arr[nr++] = bh;
+                       continue;
+               }
                bh->b_dev = VFS_I(ni)->i_dev;
                /* Is the block within the allowed limits? */
                if (iblock < lblock) {
@@ -155,8 +168,13 @@ retry_remap:
                                bh->b_blocknr = ((lcn << vol->cluster_size_bits)
                                                + vcn_ofs) >> blocksize_bits;
                                bh->b_state |= (1UL << BH_Mapped);
-                               arr[nr++] = bh;
-                               continue;
+                               /* Only read initialized data blocks. */
+                               if (iblock < zblock) {
+                                       arr[nr++] = bh;
+                                       continue;
+                               }
+                               /* Fully non-initialized data block, zero it. */
+                               goto handle_zblock;
                        }
                        /* It is a hole, need to zero it. */
                        if (lcn == LCN_HOLE)
@@ -183,6 +201,7 @@ retry_remap:
 handle_hole:
                bh->b_blocknr = -1UL;
                bh->b_state &= ~(1UL << BH_Mapped);
+handle_zblock:
                memset(kmap(page) + i * blocksize, 0, blocksize);
                flush_dcache_page(page);
                kunmap(page);
@@ -301,6 +320,7 @@ static int ntfs_file_readpage(struct file *file, struct page *page)
                                bytes);
        } else
                memset(addr, 0, PAGE_CACHE_SIZE);
+       flush_dcache_page(page);
        kunmap(page);
 
        SetPageUptodate(page);
@@ -313,7 +333,9 @@ unl_err_out:
        return err;
 }
 
-/*
+/**
+ * end_buffer_read_mftbmp_async -
+ *
  * Async io completion handler for accessing mft bitmap. Adapted from
  * end_buffer_read_mst_async().
  */
@@ -373,13 +395,17 @@ still_busy:
        return;
 }
 
-/* Readpage for accessing mft bitmap. Adapted from ntfs_mst_readpage(). */
+/**
+ * ntfs_mftbmp_readpage -
+ *
+ * Readpage for accessing mft bitmap. Adapted from ntfs_mst_readpage().
+ */
 static int ntfs_mftbmp_readpage(ntfs_volume *vol, struct page *page)
 {
        VCN vcn;
        LCN lcn;
        struct buffer_head *bh, *head, *arr[MAX_BUF_PER_PAGE];
-       sector_t iblock, lblock;
+       sector_t iblock, lblock, zblock;
        unsigned int blocksize, blocks, vcn_ofs;
        int nr, i;
        unsigned char blocksize_bits;
@@ -389,20 +415,28 @@ static int ntfs_mftbmp_readpage(ntfs_volume *vol, struct page *page)
 
        blocksize = vol->sb->s_blocksize;
        blocksize_bits = vol->sb->s_blocksize_bits;
-       
-       create_empty_buffers(page, blocksize);
+
+       if (!page_has_buffers(page))
+               create_empty_buffers(page, blocksize);
        bh = head = page_buffers(page);
        if (!bh)
                return -ENOMEM;
-       
+
        blocks = PAGE_CACHE_SIZE >> blocksize_bits;
        iblock = page->index << (PAGE_CACHE_SHIFT - blocksize_bits);
        lblock = (vol->mftbmp_allocated_size + blocksize - 1) >> blocksize_bits;
-       
+       zblock = (vol->mftbmp_initialized_size + blocksize - 1) >>
+                       blocksize_bits;
+
        /* Loop through all the buffers in the page. */
        nr = i = 0;
        do {
-               BUG_ON(buffer_mapped(bh) || buffer_uptodate(bh));
+               if (unlikely(buffer_uptodate(bh)))
+                       continue;
+               if (unlikely(buffer_mapped(bh))) {
+                       arr[nr++] = bh;
+                       continue;
+               }
                bh->b_dev = vol->mft_ino->i_dev;
                /* Is the block within the allowed limits? */
                if (iblock < lblock) {
@@ -421,8 +455,13 @@ static int ntfs_mftbmp_readpage(ntfs_volume *vol, struct page *page)
                                bh->b_blocknr = ((lcn << vol->cluster_size_bits)
                                                + vcn_ofs) >> blocksize_bits;
                                bh->b_state |= (1UL << BH_Mapped);
-                               arr[nr++] = bh;
-                               continue;
+                               /* Only read initialized data blocks. */
+                               if (iblock < zblock) {
+                                       arr[nr++] = bh;
+                                       continue;
+                               }
+                               /* Fully non-initialized data block, zero it. */
+                               goto handle_zblock;
                        }
                        if (lcn != LCN_HOLE) {
                                /* Hard error, zero out region. */
@@ -442,6 +481,7 @@ static int ntfs_mftbmp_readpage(ntfs_volume *vol, struct page *page)
                 */
                bh->b_blocknr = -1UL;
                bh->b_state &= ~(1UL << BH_Mapped);
+handle_zblock:
                memset(kmap(page) + i * blocksize, 0, blocksize);
                flush_dcache_page(page);
                kunmap(page);
@@ -593,15 +633,6 @@ still_busy:
  * the page before finally marking it uptodate and unlocking it.
  *
  * Contains an adapted version of fs/buffer.c::block_read_full_page().
- *
- * TODO:/FIXME: The current implementation is simple but wasteful as we perform
- * actual i/o from disk for all data up to allocated size completely ignoring
- * the fact that initialized size, and data size for that matter, may well be
- * lower and hence there is no point in reading them in. We can just zero the
- * page range, which is what is currently done in our async i/o completion
- * handler anyway, once the read from disk completes. However, I am not sure how
- * to setup the buffer heads in that case, so for now we do the pointless i/o.
- * Any help with this would be appreciated...
  */
 int ntfs_mst_readpage(struct file *dir, struct page *page)
 {
@@ -610,7 +641,7 @@ int ntfs_mst_readpage(struct file *dir, struct page *page)
        ntfs_inode *ni;
        ntfs_volume *vol;
        struct buffer_head *bh, *head, *arr[MAX_BUF_PER_PAGE];
-       sector_t iblock, lblock;
+       sector_t iblock, lblock, zblock;
        unsigned int blocksize, blocks, vcn_ofs;
        int i, nr;
        unsigned char blocksize_bits;
@@ -624,7 +655,8 @@ int ntfs_mst_readpage(struct file *dir, struct page *page)
        blocksize_bits = VFS_I(ni)->i_blkbits;
        blocksize = 1 << blocksize_bits;
 
-       create_empty_buffers(page, blocksize);
+       if (!page_has_buffers(page))
+               create_empty_buffers(page, blocksize);
        bh = head = page_buffers(page);
        if (!bh)
                return -ENOMEM;
@@ -632,6 +664,7 @@ int ntfs_mst_readpage(struct file *dir, struct page *page)
        blocks = PAGE_CACHE_SIZE >> blocksize_bits;
        iblock = page->index << (PAGE_CACHE_SHIFT - blocksize_bits);
        lblock = (ni->allocated_size + blocksize - 1) >> blocksize_bits;
+       zblock = (ni->initialized_size + blocksize - 1) >> blocksize_bits;
 
 #ifdef DEBUG
        if (unlikely(!ni->run_list.rl && !ni->mft_no))
@@ -642,7 +675,12 @@ int ntfs_mst_readpage(struct file *dir, struct page *page)
        /* Loop through all the buffers in the page. */
        nr = i = 0;
        do {
-               BUG_ON(buffer_mapped(bh) || buffer_uptodate(bh));
+               if (unlikely(buffer_uptodate(bh)))
+                       continue;
+               if (unlikely(buffer_mapped(bh))) {
+                       arr[nr++] = bh;
+                       continue;
+               }
                bh->b_dev = VFS_I(ni)->i_dev;
                /* Is the block within the allowed limits? */
                if (iblock < lblock) {
@@ -664,8 +702,13 @@ retry_remap:
                                bh->b_blocknr = ((lcn << vol->cluster_size_bits)
                                                + vcn_ofs) >> blocksize_bits;
                                bh->b_state |= (1UL << BH_Mapped);
-                               arr[nr++] = bh;
-                               continue;
+                               /* Only read initialized data blocks. */
+                               if (iblock < zblock) {
+                                       arr[nr++] = bh;
+                                       continue;
+                               }
+                               /* Fully non-initialized data block, zero it. */
+                               goto handle_zblock;
                        }
                        /* It is a hole, need to zero it. */
                        if (lcn == LCN_HOLE)
@@ -692,6 +735,7 @@ retry_remap:
 handle_hole:
                bh->b_blocknr = -1UL;
                bh->b_state &= ~(1UL << BH_Mapped);
+handle_zblock:
                memset(kmap(page) + i * blocksize, 0, blocksize);
                flush_dcache_page(page);
                kunmap(page);
@@ -721,7 +765,9 @@ handle_hole:
        return nr;
 }
 
-/* Address space operations for accessing normal file data. */
+/**
+ * ntfs_file_aops - address space operations for accessing normal file data
+ */
 struct address_space_operations ntfs_file_aops = {
        writepage:      NULL,                   /* Write dirty page to disk. */
        readpage:       ntfs_file_readpage,     /* Fill page with data. */
@@ -733,7 +779,9 @@ struct address_space_operations ntfs_file_aops = {
 
 typedef int readpage_t(struct file *, struct page *);
 
-/* Address space operations for accessing mftbmp. */
+/**
+ * ntfs_mftbmp_aops - address space operations for accessing mftbmp
+ */
 struct address_space_operations ntfs_mftbmp_aops = {
        writepage:      NULL,                   /* Write dirty page to disk. */
        readpage:       (readpage_t*)ntfs_mftbmp_readpage, /* Fill page with
@@ -744,7 +792,9 @@ struct address_space_operations ntfs_mftbmp_aops = {
        commit_write:   NULL,                   /* . */
 };
 
-/*
+/**
+ * ntfs_dir_aops -
+ *
  * Address space operations for accessing normal directory data (i.e. index
  * allocation attribute). We can't just use the same operations as for files
  * because 1) the attribute is different and even more importantly 2) the index
index 8dcb67edf256fe8de27123805757bf881885d081..a67ecde99783d02737b38b494d6ae8af7bd90c75 100644 (file)
@@ -69,7 +69,7 @@ int allocate_compression_buffers(void)
 
        BUG_ON(ntfs_compression_buffers);
 
-       ntfs_compression_buffers =  (u8**)kmalloc(smp_num_cpus * sizeof(u8 *),
+       ntfs_compression_buffers =  (u8**)kmalloc(smp_num_cpus * sizeof(u8*),
                        GFP_KERNEL);
        if (!ntfs_compression_buffers)
                return -ENOMEM;
@@ -81,7 +81,7 @@ int allocate_compression_buffers(void)
        if (i == smp_num_cpus)
                return 0;
        /* Allocation failed, cleanup and return error. */
-       for (j = 0; i < j; j++)
+       for (j = 0; j < i; j++)
                vfree(ntfs_compression_buffers[j]);
        kfree(ntfs_compression_buffers);
        return -ENOMEM;
index 2132b5ff0b35a91fd76385f2ed81dde28e9efab9..01605418109407efc414964ef52976a1dbfb66ed 100644 (file)
@@ -69,44 +69,6 @@ static void __format_mft_record(MFT_RECORD *m, const int size,
        a->length = cpu_to_le32(0);
 }
 
-/**
- * format_mft_record2 - initialize an empty mft record
- * @vfs_sb:    vfs super block of volume
- * @inum:      mft record number / inode number to format
- * @mft_rec:   mapped, pinned and locked mft record (optional)
- *
- * Initialize an empty mft record. This is used when extending the MFT.
- *
- * If @mft_rec is NULL, we call map_mft_record() to obtain the record and we
- * unmap it again when finished.
- *
- * We return 0 on success or -errno on error.
- */
-#if 0
-// Can't do this as iget_map_mft_record no longer exists...
-int format_mft_record2(struct super_block *vfs_sb, const unsigned long inum,
-               MFT_RECORD *mft_rec)
-{
-       MFT_RECORD *m;
-       ntfs_inode *ni;
-
-       if (mft_rec)
-               m = mft_rec;
-       else {
-               m = iget_map_mft_record(WRITE, vfs_sb, inum, &ni);
-               if (IS_ERR(m))
-                       return PTR_ERR(m);
-       }
-       __format_mft_record(m, NTFS_SB(vfs_sb)->mft_record_size, inum);
-       if (!mft_rec) {
-               // TODO: dirty mft record
-               unmap_mft_record(WRITE, ni);
-               // TODO: Do stuff to get rid of the ntfs_inode
-       }
-       return 0;
-}
-#endif
-
 /**
  * format_mft_record - initialize an empty mft record
  * @ni:                ntfs inode of mft record
@@ -340,71 +302,6 @@ MFT_RECORD *map_mft_record(const int rw, ntfs_inode *ni)
        return m;
 }
 
-/**
- * iget_map_mft_record - iget, map, pin, lock an mft record
- * @rw:                map for read (rw = READ) or write (rw = WRITE)
- * @vfs_sb:    vfs super block of mounted volume
- * @inum:      inode number / MFT record number whose mft record to map
- * @vfs_ino:   output parameter which we set to the inode on successful return
- *
- * Does the same as map_mft_record(), except that it starts out only with the
- * knowledge of the super block (@vfs_sb) and the mft record number which is of
- * course the same as the inode number (@inum).
- *
- * On success, *@vfs_ino will contain a pointer to the inode structure of the
- * mft record on return. On error return, *@vfs_ino is undefined.
- *
- * See map_mft_record() description for details and for a description of how
- * errors are returned and what error codes are defined.
- *
- * IMPROTANT: The caller is responsible for calling iput(@vfs_ino) when
- * finished with the inode, i.e. after unmap_mft_record() has been called. If
- * that is omitted you will get busy inodes upon umount...
- */
-#if 0
-// this is no longer possible. iget() cannot be called as we may be loading
-// an ntfs inode which will never have a corresponding vfs inode counter part.
-// this is not going to be pretty. )-:
-// we need our own hash for ntfs inodes now, ugh. )-:
-// not having vfs inodes associated with all ntfs inodes is a bad mistake I am
-// getting the impression. this will in the end turn out uglier than just
-// having iget_no_wait().
-// my only hope is that we can get away without this functionality in the driver
-// altogether. we are ok for extent inodes already because we only handle them
-// via map_extent_mft_record().
-// if we really need it, we could have a list or hash of "pure ntfs inodes"
-// to cope with this situation, so the lookup would be:
-// look for the inode and if not present look for pure ntfs inode and if not
-// present add a new pure ntfs inode. under this scheme extent inodes have to
-// also be added to the list/hash of pure inodes.
-MFT_RECORD *iget_map_mft_record(const int rw, struct super_block *vfs_sb,
-               const unsigned long inum, struct inode **vfs_ino)
-{
-       struct inode *inode;
-       MFT_RECORD *mrec;
-
-       /*
-        * The corresponding iput() happens when clear_inode() is called on the
-        * base mft record of this extent mft record.
-        * When used on base mft records, caller has to perform the iput().
-        */
-       inode = iget(vfs_sb, inum);
-       if (inode && !is_bad_inode(inode)) {
-               mrec = map_mft_record(rw, inode);
-               if (!IS_ERR(mrec)) {
-                       ntfs_debug("Success for i_ino 0x%lx.", inum);
-                       *vfs_ino = inode;
-                       return mrec;
-               }
-       } else
-               mrec = ERR_PTR(-EIO);
-       if (inode)
-               iput(inode);
-       ntfs_debug("Failed for i_ino 0x%lx.", inum);
-       return mrec;
-}
-#endif
-
 /**
  * unmap_mft_record - release a mapped mft record
  * @rw:                unmap from read (@rw = READ) or write (@rw = WRITE)