public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* REISER4: fix for reiser4_write_extent
@ 2007-04-05 22:42 Ignatich
  2007-04-06  0:05 ` Reiser4. BEST FILESYSTEM EVER? I need help johnrobertbanks
  2007-04-07 12:51 ` REISER4: fix for reiser4_write_extent Laurent Riffard
  0 siblings, 2 replies; 50+ messages in thread
From: Ignatich @ 2007-04-05 22:42 UTC (permalink / raw)
  To: reiserfs-list, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 342 bytes --]

While trying to find the cause of problems with reiser4 in recent 
kernels I came across this.

Incomplete write handling seem to be missing from reiser4_write_extent() 
thanks to reiser4-temp-fix.patch. Strangely, there is a patch by Edward 
Shishkin that should address that issue, but it is missing from -mm 
tree. Please check.

    Max


[-- Attachment #2: reiser4-fix-write_extent.patch --]
[-- Type: text/plain, Size: 6591 bytes --]

------------------------------------------------------
Subject: reiser4: fix write_extent
From: Edward Shishkin <[EMAIL PROTECTED]>

. Fix reiser4_write_extent():
   1) handling incomplete writes missed in reiser4-temp-fix.patch
   2) bugs in the case of returned errors


Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
---

 fs/reiser4/plugin/item/extent_file_ops.c |   64 ++++++++++++---------
 1 file changed, 37 insertions(+), 27 deletions(-)

diff -puN fs/reiser4/plugin/item/extent_file_ops.c~reiser4-fix-write_extent 
fs/reiser4/plugin/item/extent_file_ops.c
--- a/fs/reiser4/plugin/item/extent_file_ops.c~reiser4-fix-write_extent
+++ a/fs/reiser4/plugin/item/extent_file_ops.c
@@ -941,15 +941,15 @@ static int write_extent_reserve_space(st
  * reiser4_write_extent - write method of extent item plugin
  * @file: file to write to
  * @buf: address of user-space buffer
- * @write_amount: number of bytes to write
- * @off: position in file to write to
+ * @count: number of bytes to write
+ * @pos: position in file to write to
  *
  */
 ssize_t reiser4_write_extent(struct file *file, const char __user *buf,
                             size_t count, loff_t *pos)
 {
        int have_to_update_extent;
-       int nr_pages;
+       int nr_pages, nr_dirty;
        struct page *page;
        jnode *jnodes[WRITE_GRANULARITY + 1];
        struct inode *inode;
@@ -958,7 +958,7 @@ ssize_t reiser4_write_extent(struct file
        int i;
        int to_page, page_off;
        size_t left, written;
-       int result;
+       int result = 0;
 
        inode = file->f_dentry->d_inode;
        if (write_extent_reserve_space(inode))
@@ -972,10 +972,12 @@ ssize_t reiser4_write_extent(struct file
 
        BUG_ON(get_current_context()->trans->atom != NULL);
 
+       left = count;
        index = *pos >> PAGE_CACHE_SHIFT;
        /* calculate number of pages which are to be written */
        end = ((*pos + count - 1) >> PAGE_CACHE_SHIFT);
        nr_pages = end - index + 1;
+       nr_dirty = 0;
        assert("", nr_pages <= WRITE_GRANULARITY + 1);
 
        /* get pages and jnodes */
@@ -983,22 +985,17 @@ ssize_t reiser4_write_extent(struct file
                page = find_or_create_page(inode->i_mapping, index + i,
                                           reiser4_ctx_gfp_mask_get());
                if (page == NULL) {
-                       while(i --) {
-                               unlock_page(jnode_page(jnodes[i]));
-                               page_cache_release(jnode_page(jnodes[i]));
-                       }
-                       return RETERR(-ENOMEM);
+                       nr_pages = i;
+                       result = RETERR(-ENOMEM);
+                       goto out;
                }
-
                jnodes[i] = jnode_of_page(page);
                if (IS_ERR(jnodes[i])) {
                        unlock_page(page);
                        page_cache_release(page);
-                       while (i --) {
-                               jput(jnodes[i]);
-                               page_cache_release(jnode_page(jnodes[i]));
-                       }
-                       return RETERR(-ENOMEM);
+                       nr_pages = i;
+                       result = RETERR(-ENOMEM);
+                       goto out;
                }
                /* prevent jnode and page from disconnecting */
                JF_SET(jnodes[i], JNODE_WRITE_PREPARED);
@@ -1009,7 +1006,6 @@ ssize_t reiser4_write_extent(struct file
 
        have_to_update_extent = 0;
 
-       left = count;
        page_off = (*pos & (PAGE_CACHE_SIZE - 1));
        for (i = 0; i < nr_pages; i ++) {
                to_page = PAGE_CACHE_SIZE - page_off;
@@ -1050,14 +1046,26 @@ ssize_t reiser4_write_extent(struct file
                        flush_dcache_page(page);
                        kunmap_atomic(kaddr, KM_USER0);
                }
-
-               written = filemap_copy_from_user(page, page_off, buf, to_page);
+               written = filemap_copy_from_user_atomic(page, page_off, buf,
+                                                       to_page);
+               if (written != to_page)
+                       /* Do it the slow way */
+                       written = filemap_copy_from_user_nonatomic(page,
+                                                                  page_off,
+                                                                  buf,
+                                                                  to_page);
+               if (unlikely(written != to_page)) {
+                       unlock_page(page);
+                       result = RETERR(-EFAULT);
+                       break;
+               }
                flush_dcache_page(page);
                reiser4_set_page_dirty_internal(page);
                unlock_page(page);
+               nr_dirty ++;
+
                mark_page_accessed(page);
                SetPageUptodate(page);
-               page_cache_release(page);
 
                if (jnodes[i]->blocknr == 0)
                        have_to_update_extent ++;
@@ -1067,27 +1075,29 @@ ssize_t reiser4_write_extent(struct file
                left -= to_page;
                BUG_ON(get_current_context()->trans->atom != NULL);
        }
-
        if (have_to_update_extent) {
-               update_extents(file, jnodes, nr_pages, *pos);
+               update_extents(file, jnodes, nr_dirty, *pos);
        } else {
-               for (i = 0; i < nr_pages; i ++) {
+               for (i = 0; i < nr_dirty; i ++) {
+                       int ret;
                        spin_lock_jnode(jnodes[i]);
-                       result = reiser4_try_capture(jnodes[i],
+                       ret = reiser4_try_capture(jnodes[i],
                                                     ZNODE_WRITE_LOCK, 0);
-                       BUG_ON(result != 0);
+                       BUG_ON(ret != 0);
                        jnode_make_dirty_locked(jnodes[i]);
                        spin_unlock_jnode(jnodes[i]);
                }
        }
-
+ out:
        for (i = 0; i < nr_pages; i ++) {
+               page_cache_release(jnode_page(jnodes[i]));
                JF_CLR(jnodes[i], JNODE_WRITE_PREPARED);
                jput(jnodes[i]);
        }
+       /* the only errors handled so far is ENOMEM and
+          EFAULT on copy_from_user  */
 
-       /* the only error handled so far is EFAULT on copy_from_user  */
-       return (count - left) ? (count - left) : -EFAULT;
+       return (count - left) ? (count - left) : result;
 }
 
 static inline void zero_page(struct page *page)

^ permalink raw reply	[flat|nested] 50+ messages in thread
* Re: Reiser4. BEST FILESYSTEM EVER.
@ 2007-04-07 12:59 Dale Amon
  2007-04-07 15:28 ` johnrobertbanks
  0 siblings, 1 reply; 50+ messages in thread
From: Dale Amon @ 2007-04-07 12:59 UTC (permalink / raw)
  To: johnrobertbanks; +Cc: Jan Harkes, linux-kernel, linux-fsdevel

Jan does have a point about bad blocks. A couple years ago
I had a relatively new disk start to go bad on random blocks.
I detected it fairly quickly but did have some data loss.

All the compressed archives which were hit were near
total losses; most other files were at least partially
recoverable.

It is not a matter of your operating system writing
to bad blocks. It is a matter of what happens when the
blocks on which your data sit go bad underneath you.

This issue has also been discussed by people working
with revision control system. If you are archiving
data, how do you know you if your data is still good
unless you actually need it? If you do not know it
is bad, you may well get rid of good copies thinking
you do not need the extras... it does happen.

I would be quite hesitant to go with on disk compression
unless damage was limited to only the bad bits or blocks
and did not propagate through the rest of the file.

Perhaps if everyone used hardware RAID and the RAID
automatically detected a difference due to trashed
data on one disk and flagged the admin with a warning...

BTW: I'm a CMU Alum, so who are you working with Jan?





^ permalink raw reply	[flat|nested] 50+ messages in thread

end of thread, other threads:[~2007-04-09 18:36 UTC | newest]

Thread overview: 50+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-04-05 22:42 REISER4: fix for reiser4_write_extent Ignatich
2007-04-06  0:05 ` Reiser4. BEST FILESYSTEM EVER? I need help johnrobertbanks
2007-04-06  0:23   ` H. Peter Anvin
2007-04-06  0:34     ` johnrobertbanks
2007-04-06  0:39       ` H. Peter Anvin
2007-04-06  1:34         ` Reiser4. BEST FILESYSTEM EVER johnrobertbanks
2007-04-06  3:12           ` Valdis.Kletnieks
2007-04-06  4:07           ` H. Peter Anvin
2007-04-06  4:32             ` johnrobertbanks
     [not found]               ` <20070406152119.GC4228@delft.aura.cs.cmu.edu>
2007-04-07  2:47                 ` johnrobertbanks
2007-04-07  3:30                   ` Jan Harkes
2007-04-07  5:58                     ` johnrobertbanks
2007-04-07  7:15                       ` Willy Tarreau
2007-04-07 13:47                         ` johnrobertbanks
2007-04-07 14:11                       ` Krzysztof Halasa
2007-04-07 15:07                         ` johnrobertbanks
2007-04-07 16:05                           ` Pekka Enberg
2007-04-07 17:10                         ` Valdis.Kletnieks
2007-04-08 16:31                           ` Adrian Bunk
2007-04-07 17:21                         ` Valdis.Kletnieks
2007-04-08  0:41                           ` Krzysztof Halasa
2007-04-07 17:39                   ` Valdis.Kletnieks
2007-04-07 19:17               ` Lennart Sorensen
2007-04-08  0:44                 ` johnrobertbanks
2007-04-08  1:27                   ` Lennart Sorensen
2007-04-08  2:56                   ` Theodore Tso
2007-04-08  4:13                     ` johnrobertbanks
2007-04-08 12:48                       ` Jose Celestino
2007-04-08 13:21                         ` johnrobertbanks
2007-04-08 14:14                           ` Willy Tarreau
2007-04-08 17:03                       ` Theodore Tso
2007-04-08 18:18                         ` Jeff Mahoney
2007-04-08  4:32                   ` Christer Weinigel
2007-04-08 21:50                     ` Reiser4. BEST FILESYSTEM EVER - Christer Weinigel johnrobertbanks
2007-04-08 22:58                       ` Richard Knutsson
2007-04-09  5:14                         ` johnrobertbanks
2007-04-09  7:07                           ` Willy Tarreau
2007-04-09 16:10                           ` Richard Knutsson
2007-04-09 18:35                     ` Reiser4. BEST FILESYSTEM EVER Nate Diller
2007-04-07  1:26   ` COMPILING AND CONFIGURING A NEW KERNEL johnrobertbanks
2007-04-07  7:45     ` johnrobertbanks
2007-04-07 16:57       ` Valdis.Kletnieks
2007-04-08  1:11         ` johnrobertbanks
2007-04-07 16:42     ` Valdis.Kletnieks
2007-04-08  1:02       ` johnrobertbanks
2007-04-08  1:42         ` Lennart Sorensen
2007-04-07 12:51 ` REISER4: fix for reiser4_write_extent Laurent Riffard
2007-04-07 19:29   ` Edward Shishkin
  -- strict thread matches above, loose matches on Subject: below --
2007-04-07 12:59 Reiser4. BEST FILESYSTEM EVER Dale Amon
2007-04-07 15:28 ` johnrobertbanks

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox