[patch 13/18] generic_file_write() cleanup

From: Andrew Morton (akpm@zip.com.au)
Date: Sun May 26 2002 - 15:47:09 EST


Fixes all the goto spaghetti in generic_file_write() and turns it into
something which humans can understand.

Andi tells me that gcc3 does a decent job of relocating blocks out of
line anyway. This patch gives the compiler a helping hand with
appropriate use of likely() and unlikely().

=====================================

--- 2.5.18/mm/filemap.c~generic_file_write_cleanup Sat May 25 23:25:49 2002
+++ 2.5.18-akpm/mm/filemap.c Sat May 25 23:26:25 2002
@@ -2074,48 +2074,43 @@ inline void remove_suid(struct dentry *d
 /*
  * Write to a file through the page cache.
  *
- * We currently put everything into the page cache prior to writing it.
- * This is not a problem when writing full pages. With partial pages,
- * however, we first have to read the data into the cache, then
- * dirty the page, and finally schedule it for writing. Alternatively, we
- * could write-through just the portion of data that would go into that
- * page, but that would kill performance for applications that write data
- * line by line, and it's prone to race conditions.
- *
- * Note that this routine doesn't try to keep track of dirty pages. Each
- * file system has to do this all by itself, unfortunately.
+ * We put everything into the page cache prior to writing it. This is not a
+ * problem when writing full pages. With partial pages, however, we first have
+ * to read the data into the cache, then dirty the page, and finally schedule
+ * it for writing by marking it dirty.
  * okir@monad.swb.de
  */
 ssize_t
-generic_file_write(struct file *file,const char *buf,size_t count, loff_t *ppos)
+generic_file_write(struct file *file, const char *buf,
+ size_t count, loff_t *ppos)
 {
- struct address_space *mapping = file->f_dentry->d_inode->i_mapping;
- struct inode *inode = mapping->host;
+ struct address_space * mapping = file->f_dentry->d_inode->i_mapping;
+ struct address_space_operations *a_ops = mapping->a_ops;
+ struct inode *inode = mapping->host;
         unsigned long limit = current->rlim[RLIMIT_FSIZE].rlim_cur;
+ long status = 0;
         loff_t pos;
- struct page *page, *cached_page;
+ struct page *page;
+ struct page *cached_page = NULL;
         ssize_t written;
- long status = 0;
         int err;
         unsigned bytes;
 
- if ((ssize_t) count < 0)
+ if (unlikely((ssize_t) count < 0))
                 return -EINVAL;
 
- if (!access_ok(VERIFY_READ, buf, count))
+ if (unlikely(!access_ok(VERIFY_READ, buf, count)))
                 return -EFAULT;
 
- cached_page = NULL;
-
         down(&inode->i_sem);
-
         pos = *ppos;
- err = -EINVAL;
- if (pos < 0)
+ if (unlikely(pos < 0)) {
+ err = -EINVAL;
                 goto out;
+ }
 
- err = file->f_error;
- if (err) {
+ if (unlikely(file->f_error)) {
+ err = file->f_error;
                 file->f_error = 0;
                 goto out;
         }
@@ -2129,11 +2124,10 @@ generic_file_write(struct file *file,con
         /*
          * Check whether we've reached the file size limit.
          */
- err = -EFBIG;
-
- if (limit != RLIM_INFINITY) {
+ if (unlikely(limit != RLIM_INFINITY)) {
                 if (pos >= limit) {
                         send_sig(SIGXFSZ, current, 0);
+ err = -EFBIG;
                         goto out;
                 }
                 if (pos > 0xFFFFFFFFULL || count > limit - (u32)pos) {
@@ -2143,11 +2137,13 @@ generic_file_write(struct file *file,con
         }
 
         /*
- * LFS rule
+ * LFS rule
          */
- if ( pos + count > MAX_NON_LFS && !(file->f_flags&O_LARGEFILE)) {
+ if (unlikely(pos + count > MAX_NON_LFS &&
+ !(file->f_flags & O_LARGEFILE))) {
                 if (pos >= MAX_NON_LFS) {
                         send_sig(SIGXFSZ, current, 0);
+ err = -EFBIG;
                         goto out;
                 }
                 if (count > MAX_NON_LFS - (u32)pos) {
@@ -2157,18 +2153,14 @@ generic_file_write(struct file *file,con
         }
 
         /*
- * Are we about to exceed the fs block limit ?
- *
- * If we have written data it becomes a short write
- * If we have exceeded without writing data we send
- * a signal and give them an EFBIG.
+ * Are we about to exceed the fs block limit ?
          *
- * Linus frestrict idea will clean these up nicely..
+ * If we have written data it becomes a short write. If we have
+ * exceeded without writing data we send a signal and return EFBIG.
+ * Linus frestrict idea will clean these up nicely..
          */
-
- if (!S_ISBLK(inode->i_mode)) {
- if (pos >= inode->i_sb->s_maxbytes)
- {
+ if (likely(!S_ISBLK(inode->i_mode))) {
+ if (unlikely(pos >= inode->i_sb->s_maxbytes)) {
                         if (count || pos > inode->i_sb->s_maxbytes) {
                                 send_sig(SIGXFSZ, current, 0);
                                 err = -EFBIG;
@@ -2177,7 +2169,7 @@ generic_file_write(struct file *file,con
                         /* zero-length writes at ->s_maxbytes are OK */
                 }
 
- if (pos + count > inode->i_sb->s_maxbytes)
+ if (unlikely(pos + count > inode->i_sb->s_maxbytes))
                         count = inode->i_sb->s_maxbytes - pos;
         } else {
                 if (is_read_only(inode->i_rdev)) {
@@ -2200,21 +2192,37 @@ generic_file_write(struct file *file,con
                 goto out;
 
         remove_suid(file->f_dentry);
- inode->i_ctime = inode->i_mtime = CURRENT_TIME;
+ inode->i_ctime = CURRENT_TIME;
+ inode->i_mtime = CURRENT_TIME;
         mark_inode_dirty_sync(inode);
 
- if (file->f_flags & O_DIRECT)
- goto o_direct;
+ if (unlikely(file->f_flags & O_DIRECT)) {
+ written = generic_file_direct_IO(WRITE, file,
+ (char *) buf, count, pos);
+ if (written > 0) {
+ loff_t end = pos + written;
+ if (end > inode->i_size && !S_ISBLK(inode->i_mode)) {
+ inode->i_size = end;
+ mark_inode_dirty(inode);
+ }
+ *ppos = end;
+ invalidate_inode_pages2(mapping);
+ }
+ /*
+ * Sync the fs metadata but not the minor inode changes and
+ * of course not the data as we did direct DMA for the IO.
+ */
+ if (written >= 0 && file->f_flags & O_SYNC)
+ status = generic_osync_inode(inode, OSYNC_METADATA);
+ goto out_status;
+ }
 
         do {
- unsigned long index, offset;
+ unsigned long index;
+ unsigned long offset;
                 long page_fault;
                 char *kaddr;
 
- /*
- * Try to find the page in the cache. If it isn't there,
- * allocate a free page.
- */
                 offset = (pos & (PAGE_CACHE_SIZE -1)); /* Within page */
                 index = pos >> PAGE_CACHE_SHIFT;
                 bytes = PAGE_CACHE_SIZE - offset;
@@ -2232,96 +2240,67 @@ generic_file_write(struct file *file,con
                         __get_user(dummy, buf+bytes-1);
                 }
 
- status = -ENOMEM; /* we'll assign it later anyway */
                 page = __grab_cache_page(mapping, index, &cached_page);
- if (!page)
+ if (!page) {
+ status = -ENOMEM;
                         break;
-
- /* We have exclusive IO access to the page.. */
- if (!PageLocked(page)) {
- PAGE_BUG(page);
                 }
 
                 kaddr = kmap(page);
- status = mapping->a_ops->prepare_write(file, page, offset, offset+bytes);
- if (status)
- goto sync_failure;
- page_fault = __copy_from_user(kaddr+offset, buf, bytes);
+ status = a_ops->prepare_write(file, page, offset, offset+bytes);
+ if (unlikely(status)) {
+ /*
+ * prepare_write() may have instantiated a few blocks
+ * outside i_size. Trim these off again.
+ */
+ kunmap(page);
+ unlock_page(page);
+ page_cache_release(page);
+ if (pos + bytes > inode->i_size)
+ vmtruncate(inode, inode->i_size);
+ break;
+ }
+ page_fault = __copy_from_user(kaddr + offset, buf, bytes);
                 flush_dcache_page(page);
- status = mapping->a_ops->commit_write(file, page, offset, offset+bytes);
- if (page_fault)
- goto fail_write;
- if (!status)
- status = bytes;
-
- if (status >= 0) {
- written += status;
- count -= status;
- pos += status;
- buf += status;
+ status = a_ops->commit_write(file, page, offset, offset+bytes);
+ if (unlikely(page_fault)) {
+ status = -EFAULT;
+ } else {
+ if (!status)
+ status = bytes;
+
+ if (status >= 0) {
+ written += status;
+ count -= status;
+ pos += status;
+ buf += status;
+ }
                 }
-unlock:
                 kunmap(page);
- /* Mark it unlocked again and drop the page.. */
                 SetPageReferenced(page);
                 unlock_page(page);
                 page_cache_release(page);
-
                 if (status < 0)
                         break;
                 balance_dirty_pages_ratelimited(mapping);
         } while (count);
-done:
         *ppos = pos;
 
         if (cached_page)
                 page_cache_release(cached_page);
 
- /* For now, when the user asks for O_SYNC, we'll actually
- * provide O_DSYNC. */
+ /*
+ * For now, when the user asks for O_SYNC, we'll actually give O_DSYNC
+ */
         if (status >= 0) {
                 if ((file->f_flags & O_SYNC) || IS_SYNC(inode))
- status = generic_osync_inode(inode, OSYNC_METADATA|OSYNC_DATA);
+ status = generic_osync_inode(inode,
+ OSYNC_METADATA|OSYNC_DATA);
         }
         
 out_status:
         err = written ? written : status;
 out:
-
         up(&inode->i_sem);
         return err;
-fail_write:
- status = -EFAULT;
- goto unlock;
-
-sync_failure:
- /*
- * If blocksize < pagesize, prepare_write() may have instantiated a
- * few blocks outside i_size. Trim these off again.
- */
- kunmap(page);
- unlock_page(page);
- page_cache_release(page);
- if (pos + bytes > inode->i_size)
- vmtruncate(inode, inode->i_size);
- goto done;
-
-o_direct:
- written = generic_file_direct_IO(WRITE, file, (char *) buf, count, pos);
- if (written > 0) {
- loff_t end = pos + written;
- if (end > inode->i_size && !S_ISBLK(inode->i_mode)) {
- inode->i_size = end;
- mark_inode_dirty(inode);
- }
- *ppos = end;
- invalidate_inode_pages2(mapping);
- }
- /*
- * Sync the fs metadata but not the minor inode changes and
- * of course not the data as we did direct DMA for the IO.
- */
- if (written >= 0 && file->f_flags & O_SYNC)
- status = generic_osync_inode(inode, OSYNC_METADATA);
- goto out_status;
 }

-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Fri May 31 2002 - 22:00:18 EST