Re: [PATCHv2 3/3] reiser4: mark pages created during tail2extent conversion as dirty.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11/12/2015 12:31 PM, Ivan Shapovalov wrote:
On 2015-11-12 at 11:00 +0100, Edward Shishkin wrote:
On 11/11/2015 11:30 AM, Edward Shishkin wrote:

On 11/09/2015 01:18 PM, Edward Shishkin wrote:
On 10/25/2015 01:02 AM, Ivan Shapovalov wrote:
This is responsible for an oops in v4.2 in
write_jnodes_to_disk_extent() -> set_page_writeback().

As to me, this patch doesn't prevent the oops,
so someone else forgets to set pages dirty...

False alarm: I applied to the old stuff..
Everything woks without the hack in
write_jnodes_to_disk_extent().
Good work! Merged to reiser4-for-4.2.3.
Hi Edward,

Glad to hear that. I was already halfway recovering my debug patches
for collecting stacktraces and related information and was going to ask
you to reproduce :)

The pages needs to be marked dirty before marked writeback.

  From a similar problem in f2fs:
"The cgroup attaches inode->i_wb via mark_inode_dirty and when
set_page_writeback is called, __inc_wb_stat() updates i_wb's
stat.

So, we need to explicitly call set_page_dirty-
__mark_inode_dirty in
prior to any writebacking pages."

Signed-off-by: Ivan Shapovalov <intelfx100@xxxxxxxxx>
---
   fs/reiser4/plugin/file/tail_conversion.c | 1 +
   1 file changed, 1 insertion(+)

diff --git a/fs/reiser4/plugin/file/tail_conversion.c
b/fs/reiser4/plugin/file/tail_conversion.c
index 7542c03..c856b73 100644
--- a/fs/reiser4/plugin/file/tail_conversion.c
+++ b/fs/reiser4/plugin/file/tail_conversion.c
@@ -175,6 +175,7 @@ static int replace(struct inode *inode,
struct
page **pages, unsigned nr_pages,
                                   i_mapping));
           if (result)
               break;
+        set_page_dirty_notag(pages[i]);

So, at this point the page is dirty but not uptodate.
I am confused with this. Why not to set the page
uptodate right before setting it dirty? At this point
everything has been copied already, so the page is
in fact uptodate. Could you please try this?
Have you made this correction in the merged patch?

Yes I have.

Anyway, could you
please explain why it should be done one way and not another? What will
happen if the ordering is left as is?

Is it about marking page uptodate _before unlocking_, so that no one will see it dirty && !uptodate?

Just let's set all the flags in timely fashion.
Otherwise it would mean a bad style.

Thanks,
Edward.
--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux File System Development]     [Linux BTRFS]     [Linux NFS]     [Linux Filesystems]     [Ext4 Filesystem]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Resources]

  Powered by Linux