Re: [f2fs-dev] [PATCH 1/2] add cotnfigure option --with-root-libdir

2018-08-29 Thread Theodore Y. Ts'o
On Wed, Aug 29, 2018 at 02:04:55PM -0700, Jaegeuk Kim wrote:
> Thank you so much for kind explanation. This is really awesome especially for
> newbies like me. This gives answers to me for all my own stupid questions like
> why I need to investigate all the lib changes in order to determine bumping
> current/revision/age numbers or not.
> 
> I'd definitely like to follow this rule to avoid any verion changes later, as
> I expect the next release will be the last major update. For dependency parts,
> I'll study and practice them for a while.

If you are going to be making one less major update, this is the
perfect time to make sure that data structures are allocated by the
library, and are (ideally) opaque to the calling application (so they
only manipulate structure poitners).  That is, the structure
definition is not exposed in the public header file, and you use
accessor functions to set and get fields in the structure.

If you can't do that for all data structures, if you can do that with
your primary data structure that's going to make your life much easier
in the long term.  For ext2fs, that's the file systme handle.  It's
created by ext2fs_open(), and it's passed to all other library
functions as the first argument.

The other thing you might want to consider doing is adding a magic
number to the beginning of each structure.  That way you can tell if
the wrong structure gets passed to a library.  It's also helpful for
doing the equivalent of subclassing in C.

This is how we do it in libext2fs --- we use com_err to define the
magic numbers:

error_table ext2

ec  EXT2_ET_BASE,
"EXT2FS Library version @E2FSPROGS_VERSION@"

ec  EXT2_ET_MAGIC_EXT2FS_FILSYS,
"Wrong magic number for ext2_filsys structure"

ec  EXT2_ET_MAGIC_BADBLOCKS_LIST,
"Wrong magic number for badblocks_list structure"
...

And then every single structure starts like so:

struct struct_ext2_filsys {
errcode_t   magic;
...

struct ext2_struct_inode_scan {
errcode_t   magic;
...

And then before we use any pointer we do this:

if (file->magic != EXT2_ET_MAGIC_EXT2_FILE)
return EXT2_ET_MAGIC_EXT2_FILE;

Anyway, you don't have to do any of this, but if you're thinking about
making a big major version number bump, this is a great opportunity to
do some of these things.

> BTW, can I archive this like in f2fs-tools/VERSIONING?

Sure, feel free!

- Ted

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH v3] f2fs: avoid wrong decrypted data from disk

2018-08-29 Thread Jaegeuk Kim
1. Create a file in an encrypted directory
2. Do GC & drop caches
3. Read stale data before its bio for metapage was not issued yet

Signed-off-by: Jaegeuk Kim 
---
Change log from v2:
 - make it globally consistent
 - should use f2fs_post_read_required()

 fs/f2fs/data.c| 18 ++
 fs/f2fs/f2fs.h|  2 +-
 fs/f2fs/file.c|  3 +--
 fs/f2fs/segment.c |  6 +-
 4 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 382c1ef9a9e4..8c204f896c22 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -565,9 +565,6 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, 
block_t blkaddr,
ctx->bio = bio;
ctx->enabled_steps = post_read_steps;
bio->bi_private = ctx;
-
-   /* wait the page to be moved by cleaning */
-   f2fs_wait_on_block_writeback(sbi, blkaddr);
}
 
return bio;
@@ -582,6 +579,9 @@ static int f2fs_submit_page_read(struct inode *inode, 
struct page *page,
if (IS_ERR(bio))
return PTR_ERR(bio);
 
+   /* wait for GCed page writeback via META_MAPPING */
+   f2fs_wait_on_block_writeback(inode, blkaddr);
+
if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE) {
bio_put(bio);
return -EFAULT;
@@ -1558,6 +1558,12 @@ static int f2fs_mpage_readpages(struct address_space 
*mapping,
}
}
 
+   /*
+* If the page is under writeback, we need to wait for
+* its completion to see the correct decrypted data.
+*/
+   f2fs_wait_on_block_writeback(inode, block_nr);
+
if (bio_add_page(bio, page, blocksize, 0) < blocksize)
goto submit_and_realloc;
 
@@ -1625,7 +1631,7 @@ static int encrypt_one_page(struct f2fs_io_info *fio)
return 0;
 
/* wait for GCed page writeback via META_MAPPING */
-   f2fs_wait_on_block_writeback(fio->sbi, fio->old_blkaddr);
+   f2fs_wait_on_block_writeback(inode, fio->old_blkaddr);
 
 retry_encrypt:
fio->encrypted_page = fscrypt_encrypt_page(inode, fio->page,
@@ -2382,10 +2388,6 @@ static int f2fs_write_begin(struct file *file, struct 
address_space *mapping,
 
f2fs_wait_on_page_writeback(page, DATA, false);
 
-   /* wait for GCed page writeback via META_MAPPING */
-   if (f2fs_post_read_required(inode))
-   f2fs_wait_on_block_writeback(sbi, blkaddr);
-
if (len == PAGE_SIZE || PageUptodate(page))
return 0;
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 4e1120f20041..42950d87e91d 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2942,7 +2942,7 @@ void f2fs_allocate_data_block(struct f2fs_sb_info *sbi, 
struct page *page,
struct f2fs_io_info *fio, bool add_list);
 void f2fs_wait_on_page_writeback(struct page *page,
enum page_type type, bool ordered);
-void f2fs_wait_on_block_writeback(struct f2fs_sb_info *sbi, block_t blkaddr);
+void f2fs_wait_on_block_writeback(struct inode *inode, block_t blkaddr);
 void f2fs_write_data_summaries(struct f2fs_sb_info *sbi, block_t start_blk);
 void f2fs_write_node_summaries(struct f2fs_sb_info *sbi, block_t start_blk);
 int f2fs_lookup_journal_in_cursum(struct f2fs_journal *journal, int type,
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 5474aaa274b9..98bb26be3f6f 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -112,8 +112,7 @@ static vm_fault_t f2fs_vm_page_mkwrite(struct vm_fault *vmf)
f2fs_wait_on_page_writeback(page, DATA, false);
 
/* wait for GCed page writeback via META_MAPPING */
-   if (f2fs_post_read_required(inode))
-   f2fs_wait_on_block_writeback(sbi, dn.data_blkaddr);
+   f2fs_wait_on_block_writeback(inode, dn.data_blkaddr);
 
 out_sem:
up_read(_I(inode)->i_mmap_sem);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 29a1ee21f396..056ac120305a 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -3191,10 +3191,14 @@ void f2fs_wait_on_page_writeback(struct page *page,
}
 }
 
-void f2fs_wait_on_block_writeback(struct f2fs_sb_info *sbi, block_t blkaddr)
+void f2fs_wait_on_block_writeback(struct inode *inode, block_t blkaddr)
 {
+   struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
struct page *cpage;
 
+   if (!f2fs_post_read_required(inode))
+   return;
+
if (!is_valid_data_blkaddr(sbi, blkaddr))
return;
 
-- 
2.17.0.441.gb46fe60e1d-goog


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net

Re: [f2fs-dev] [PATCH v2] f2fs: avoid wrong decrypted data from disk

2018-08-29 Thread Jaegeuk Kim
On 08/30, Chao Yu wrote:
> On 2018/8/30 9:48, Jaegeuk Kim wrote:
> > 1. Create a file in an encrypted directory
> > 2. Do GC & drop caches
> > 3. Read stale data before its bio for metapage was not issued yet
> > 
> > Signed-off-by: Jaegeuk Kim 
> > ---
> > Change log from v1:
> >  - move to bio != NULL
> > 
> >  fs/f2fs/data.c | 6 ++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 382c1ef9a9e4..159e411785e9 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1556,6 +1556,12 @@ static int f2fs_mpage_readpages(struct address_space 
> > *mapping,
> > bio = NULL;
> > goto set_error_page;
> > }
> > +   } else if (unlikely(f2fs_encrypted_file(inode))) {
> 
> For android scenario, encryption becomes a common case now, so I think we can
> remove unlikely here.
> 
> > +   /*
> > +* If the page is under writeback, we need to wait for
> > +* its completion to see the correct decrypted data.
> > +*/
> > +   f2fs_wait_on_block_writeback(F2FS_I_SB(inode), 
> > block_nr);
> > }
> 
> Look at this again, it seems f2fs_wait_on_block_writeback() is not related to
> f2fs_grab_read_bio(), so we can move it here to make code logic more clear:
> 
> if (f2fs_encrypted_file(inode)) {
>   /* */
>   f2fs_wait_on_block_writeback(F2FS_I_SB(inode), block_nr);
> }
> 
> Also, in f2fs_submit_page_read(), we need to adjust to:
> 
> - f2fs_grab_read_bio()
> - f2fs_wait_on_block_writeback()
> - bio_add_page()
> 
> How do you think?

Oh, yeah!

> 
> Thanks,
> 
> >  
> > if (bio_add_page(bio, page, blocksize, 0) < blocksize)
> > 

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [RFC PATCH RESEND 4/5] f2fs-tools: introduce sb checksum

2018-08-29 Thread Junling Zheng
This patch introduced crc for superblock.

Reviewed-by: Chao Yu 
Signed-off-by: Junling Zheng 
---
 fsck/mount.c   | 23 +++
 fsck/resize.c  | 12 ++--
 include/f2fs_fs.h  | 16 +++-
 mkfs/f2fs_format.c |  3 +++
 4 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/fsck/mount.c b/fsck/mount.c
index 2bc44ce..71af6b2 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -340,6 +340,7 @@ void print_raw_sb_info(struct f2fs_super_block *sb)
DISP_u32(sb, node_ino);
DISP_u32(sb, meta_ino);
DISP_u32(sb, cp_payload);
+   DISP_u32(sb, crc);
DISP("%-.256s", sb, version);
printf("\n");
 }
@@ -467,6 +468,9 @@ void print_sb_state(struct f2fs_super_block *sb)
if (f & cpu_to_le32(F2FS_FEATURE_LOST_FOUND)) {
MSG(0, "%s", " lost_found");
}
+   if (f & cpu_to_le32(F2FS_FEATURE_SB_CHKSUM)) {
+   MSG(0, "%s", " sb_checksum");
+   }
MSG(0, "\n");
MSG(0, "Info: superblock encrypt level = %d, salt = ",
sb->encryption_level);
@@ -555,10 +559,29 @@ static inline int sanity_check_area_boundary(struct 
f2fs_super_block *sb,
return 0;
 }
 
+static int verify_sb_chksum(struct f2fs_super_block *sb)
+{
+   if (SB_CHKSUM_OFFSET != get_sb(checksum_offset)) {
+   MSG(0, "\tInvalid SB CRC offset: %u\n",
+   get_sb(checksum_offset));
+   return -1;
+   }
+   if (f2fs_crc_valid(get_sb(crc), sb,
+   get_sb(checksum_offset))) {
+   MSG(0, "\tInvalid SB CRC: 0x%x\n", get_sb(crc));
+   return -1;
+   }
+   return 0;
+}
+
 int sanity_check_raw_super(struct f2fs_super_block *sb, enum SB_ADDR sb_addr)
 {
unsigned int blocksize;
 
+   if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) &&
+   verify_sb_chksum(sb))
+   return -1;
+
if (F2FS_SUPER_MAGIC != get_sb(magic))
return -1;
 
diff --git a/fsck/resize.c b/fsck/resize.c
index 5161a1f..3462165 100644
--- a/fsck/resize.c
+++ b/fsck/resize.c
@@ -603,9 +603,6 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
}
}
 
-   print_raw_sb_info(sb);
-   print_raw_sb_info(new_sb);
-
old_main_blkaddr = get_sb(main_blkaddr);
new_main_blkaddr = get_newsb(main_blkaddr);
offset = new_main_blkaddr - old_main_blkaddr;
@@ -629,6 +626,9 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
migrate_sit(sbi, new_sb, offset_seg);
rebuild_checkpoint(sbi, new_sb, offset_seg);
update_superblock(new_sb, SB_ALL);
+   print_raw_sb_info(sb);
+   print_raw_sb_info(new_sb);
+
return 0;
 }
 
@@ -658,9 +658,6 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
}
}
 
-   print_raw_sb_info(sb);
-   print_raw_sb_info(new_sb);
-
old_main_blkaddr = get_sb(main_blkaddr);
new_main_blkaddr = get_newsb(main_blkaddr);
offset = old_main_blkaddr - new_main_blkaddr;
@@ -690,6 +687,9 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
/* move whole data region */
//if (err)
//  migrate_main(sbi, offset);
+   print_raw_sb_info(sb);
+   print_raw_sb_info(new_sb);
+
return 0;
 }
 
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index df46950..9396c78 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -279,6 +279,7 @@ static inline uint64_t bswap_64(uint64_t val)
 #define BITS_PER_BYTE  8
 #define F2FS_SUPER_MAGIC   0xF2F52010  /* F2FS Magic Number */
 #define CP_CHKSUM_OFFSET   4092
+#define SB_CHKSUM_OFFSET   3068
 #define MAX_PATH_LEN   64
 #define MAX_DEVICES8
 
@@ -579,6 +580,7 @@ enum {
 #define F2FS_FEATURE_INODE_CRTIME  0x0100
 #define F2FS_FEATURE_LOST_FOUND0x0200
 #define F2FS_FEATURE_VERITY0x0400  /* reserved */
+#define F2FS_FEATURE_SB_CHKSUM 0x0800
 
 #define MAX_VOLUME_NAME512
 
@@ -632,7 +634,8 @@ struct f2fs_super_block {
struct f2fs_device devs[MAX_DEVICES];   /* device list */
__le32 qf_ino[F2FS_MAX_QUOTAS]; /* quota inode numbers */
__u8 hot_ext_count; /* # of hot file extension */
-   __u8 reserved[314]; /* valid reserved region */
+   __u8 reserved[310]; /* valid reserved region */
+   __le32 crc; /* checksum of superblock */
 } __attribute__((packed));
 
 /*
@@ -1338,6 +1341,7 @@ struct feature feature_table[] = {
\
{ "inode_crtime",   F2FS_FEATURE_INODE_CRTIME },\
{ "lost_found", F2FS_FEATURE_LOST_FOUND },  \
{ "verity", F2FS_FEATURE_VERITY },  /* reserved */ \
+   { "sb_checksum",   

[f2fs-dev] [RFC PATCH RESEND 2/5] f2fs-tools: rename CHECKSUM_OFFSET to CP_CHKSUM_OFFSET

2018-08-29 Thread Junling Zheng
This patch renamed CHECKSUM_OFFSET to CP_CHKSUM_OFFSET.

Reviewed-by: Chao Yu 
Signed-off-by: Junling Zheng 
---
 fsck/fsck.c|  4 ++--
 fsck/mount.c   |  4 ++--
 fsck/resize.c  |  8 
 include/f2fs_fs.h  |  6 +++---
 mkfs/f2fs_format.c | 14 +++---
 5 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index d550403..f080d3c 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -2010,8 +2010,8 @@ static void fix_checkpoint(struct f2fs_sb_info *sbi)
set_cp(valid_node_count, fsck->chk.valid_node_cnt);
set_cp(valid_inode_count, fsck->chk.valid_inode_cnt);
 
-   crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CHECKSUM_OFFSET);
-   *((__le32 *)((unsigned char *)cp + CHECKSUM_OFFSET)) = cpu_to_le32(crc);
+   crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CP_CHKSUM_OFFSET);
+   *((__le32 *)((unsigned char *)cp + CP_CHKSUM_OFFSET)) = 
cpu_to_le32(crc);
 
cp_blk_no = get_sb(cp_blkaddr);
if (sbi->cur_cp == 2)
diff --git a/fsck/mount.c b/fsck/mount.c
index 2c2473d..1daef75 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -2252,8 +2252,8 @@ void write_checkpoint(struct f2fs_sb_info *sbi)
flags = update_nat_bits_flags(sb, cp, flags);
set_cp(ckpt_flags, flags);
 
-   crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CHECKSUM_OFFSET);
-   *((__le32 *)((unsigned char *)cp + CHECKSUM_OFFSET)) = cpu_to_le32(crc);
+   crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CP_CHKSUM_OFFSET);
+   *((__le32 *)((unsigned char *)cp + CP_CHKSUM_OFFSET)) = 
cpu_to_le32(crc);
 
cp_blk_no = get_sb(cp_blkaddr);
if (sbi->cur_cp == 2)
diff --git a/fsck/resize.c b/fsck/resize.c
index fe8a61a..e9612b3 100644
--- a/fsck/resize.c
+++ b/fsck/resize.c
@@ -90,10 +90,10 @@ static int get_new_sb(struct f2fs_super_block *sb)
 * It requires more pages for cp.
 */
if (max_sit_bitmap_size > MAX_SIT_BITMAP_SIZE_IN_CKPT) {
-   max_nat_bitmap_size = CHECKSUM_OFFSET - sizeof(struct 
f2fs_checkpoint) + 1;
+   max_nat_bitmap_size = CP_CHKSUM_OFFSET - sizeof(struct 
f2fs_checkpoint) + 1;
set_sb(cp_payload, F2FS_BLK_ALIGN(max_sit_bitmap_size));
} else {
-   max_nat_bitmap_size = CHECKSUM_OFFSET - sizeof(struct 
f2fs_checkpoint) + 1
+   max_nat_bitmap_size = CP_CHKSUM_OFFSET - sizeof(struct 
f2fs_checkpoint) + 1
- max_sit_bitmap_size;
set_sb(cp_payload, 0);
}
@@ -520,8 +520,8 @@ static void rebuild_checkpoint(struct f2fs_sb_info *sbi,
(unsigned char *)cp);
new_cp->checkpoint_ver = cpu_to_le64(cp_ver + 1);
 
-   crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, new_cp, CHECKSUM_OFFSET);
-   *((__le32 *)((unsigned char *)new_cp + CHECKSUM_OFFSET)) =
+   crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, new_cp, CP_CHKSUM_OFFSET);
+   *((__le32 *)((unsigned char *)new_cp + CP_CHKSUM_OFFSET)) =
cpu_to_le32(crc);
 
/* Write a new checkpoint in the other set */
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 2c086a9..38a0da4 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -278,7 +278,7 @@ static inline uint64_t bswap_64(uint64_t val)
 #define PAGE_CACHE_SIZE4096
 #define BITS_PER_BYTE  8
 #define F2FS_SUPER_MAGIC   0xF2F52010  /* F2FS Magic Number */
-#define CHECKSUM_OFFSET4092
+#define CP_CHKSUM_OFFSET   4092
 #define MAX_PATH_LEN   64
 #define MAX_DEVICES8
 
@@ -682,9 +682,9 @@ struct f2fs_checkpoint {
 } __attribute__((packed));
 
 #define MAX_SIT_BITMAP_SIZE_IN_CKPT\
-   (CHECKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1 - 64)
+   (CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1 - 64)
 #define MAX_BITMAP_SIZE_IN_CKPT\
-   (CHECKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1)
+   (CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1)
 
 /*
  * For orphan inode management
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 4b88d93..621126c 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -342,12 +342,12 @@ static int f2fs_prepare_super_block(void)
 * It requires more pages for cp.
 */
if (max_sit_bitmap_size > MAX_SIT_BITMAP_SIZE_IN_CKPT) {
-   max_nat_bitmap_size = CHECKSUM_OFFSET -
+   max_nat_bitmap_size = CP_CHKSUM_OFFSET -
sizeof(struct f2fs_checkpoint) + 1;
set_sb(cp_payload, F2FS_BLK_ALIGN(max_sit_bitmap_size));
} else {
max_nat_bitmap_size =
-   CHECKSUM_OFFSET - sizeof(struct 
f2fs_checkpoint) + 1
+   

[f2fs-dev] [RFC PATCH RESEND 5/5] fsck.f2fs: try to recover cp_payload from valid cp pack

2018-08-29 Thread Junling Zheng
From: Chao Yu 

If sb checksum is not enabled, and cp pack is valid due to no
crc inconsistence, let's try to recover cp_payload based on
cp_pack_start_sum in cp pack.

Signed-off-by: Chao Yu 
---
 fsck/f2fs.h  |  5 +
 fsck/mount.c | 10 +++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/fsck/f2fs.h b/fsck/f2fs.h
index d216444..0d0d5e2 100644
--- a/fsck/f2fs.h
+++ b/fsck/f2fs.h
@@ -259,6 +259,11 @@ static inline unsigned long __bitmap_size(struct 
f2fs_sb_info *sbi, int flag)
return 0;
 }
 
+static inline block_t __cp_payload(struct f2fs_sb_info *sbi)
+{
+   return le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_payload);
+}
+
 static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
 {
struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi);
diff --git a/fsck/mount.c b/fsck/mount.c
index 71af6b2..8a7d6e4 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -945,12 +945,16 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
}
 
cp_pack_start_sum = __start_sum_addr(sbi);
-   cp_payload = get_sb(cp_payload);
+   cp_payload = __cp_payload(sbi);
if (cp_pack_start_sum < cp_payload + 1 ||
cp_pack_start_sum > blocks_per_seg - 1 -
NR_CURSEG_TYPE) {
-   MSG(0, "\tWrong cp_pack_start_sum(%u)\n", cp_pack_start_sum);
-   return 1;
+   MSG(0, "\tWrong cp_pack_start_sum(%u) or cp_payload(%u)\n",
+   cp_pack_start_sum, cp_payload);
+   if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM))
+   return 1;
+   set_sb(cp_payload, cp_pack_start_sum - 1);
+   update_superblock(sb, SB_ALL);
}
 
return 0;
-- 
2.18.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [RFC PATCH v3 3/5] f2fs-tools: unify the writeback of superblock

2018-08-29 Thread Junling Zheng
Introduce __write_superblock() to support updating specified one
superblock or both, thus we can wrapper it in update_superblock() and
f2fs_write_super_block to unify all places where sb needs to be updated.

Signed-off-by: Junling Zheng 
---
v2 -> v3:
 - fix wrong condition of ASSERT in update_superblock.
v1 -> v2:
 - if dev_write_block failed, add some notes and free buf to avoid memory leak.
 fsck/fsck.h|  2 +-
 fsck/mount.c   | 74 +++---
 fsck/resize.c  | 20 ++---
 include/f2fs_fs.h  | 35 ++
 mkfs/f2fs_format.c | 19 +---
 5 files changed, 56 insertions(+), 94 deletions(-)

diff --git a/fsck/fsck.h b/fsck/fsck.h
index 6042e68..e3490e6 100644
--- a/fsck/fsck.h
+++ b/fsck/fsck.h
@@ -178,7 +178,7 @@ extern void move_curseg_info(struct f2fs_sb_info *, u64, 
int);
 extern void write_curseg_info(struct f2fs_sb_info *);
 extern int find_next_free_block(struct f2fs_sb_info *, u64 *, int, int);
 extern void write_checkpoint(struct f2fs_sb_info *);
-extern void write_superblock(struct f2fs_super_block *);
+extern void update_superblock(struct f2fs_super_block *, int);
 extern void update_data_blkaddr(struct f2fs_sb_info *, nid_t, u16, block_t);
 extern void update_nat_blkaddr(struct f2fs_sb_info *, nid_t, nid_t, block_t);
 
diff --git a/fsck/mount.c b/fsck/mount.c
index 1daef75..2bc44ce 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -476,7 +476,7 @@ void print_sb_state(struct f2fs_super_block *sb)
 }
 
 static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
-   u64 offset)
+   enum SB_ADDR sb_addr)
 {
u32 segment0_blkaddr = get_sb(segment0_blkaddr);
u32 cp_blkaddr = get_sb(cp_blkaddr);
@@ -542,14 +542,11 @@ static inline int sanity_check_area_boundary(struct 
f2fs_super_block *sb,
segment_count_main << log_blocks_per_seg);
return -1;
} else if (main_end_blkaddr < seg_end_blkaddr) {
-   int err;
-
set_sb(segment_count, (main_end_blkaddr -
segment0_blkaddr) >> log_blocks_per_seg);
 
-   err = dev_write(sb, offset, sizeof(struct f2fs_super_block));
-   MSG(0, "Info: Fix alignment: %s, start(%u) end(%u) block(%u)\n",
-   err ? "failed": "done",
+   update_superblock(sb, 1 << sb_addr);
+   MSG(0, "Info: Fix alignment: start(%u) end(%u) block(%u)\n",
main_blkaddr,
segment0_blkaddr +
(segment_count << log_blocks_per_seg),
@@ -558,7 +555,7 @@ static inline int sanity_check_area_boundary(struct 
f2fs_super_block *sb,
return 0;
 }
 
-int sanity_check_raw_super(struct f2fs_super_block *sb, u64 offset)
+int sanity_check_raw_super(struct f2fs_super_block *sb, enum SB_ADDR sb_addr)
 {
unsigned int blocksize;
 
@@ -600,30 +597,24 @@ int sanity_check_raw_super(struct f2fs_super_block *sb, 
u64 offset)
if (get_sb(segment_count) > F2FS_MAX_SEGMENT)
return -1;
 
-   if (sanity_check_area_boundary(sb, offset))
+   if (sanity_check_area_boundary(sb, sb_addr))
return -1;
return 0;
 }
 
-int validate_super_block(struct f2fs_sb_info *sbi, int block)
+int validate_super_block(struct f2fs_sb_info *sbi, enum SB_ADDR sb_addr)
 {
-   u64 offset;
char buf[F2FS_BLKSIZE];
 
sbi->raw_super = malloc(sizeof(struct f2fs_super_block));
 
-   if (block == 0)
-   offset = F2FS_SUPER_OFFSET;
-   else
-   offset = F2FS_BLKSIZE + F2FS_SUPER_OFFSET;
-
-   if (dev_read_block(buf, block))
+   if (dev_read_block(buf, sb_addr))
return -1;
 
memcpy(sbi->raw_super, buf + F2FS_SUPER_OFFSET,
sizeof(struct f2fs_super_block));
 
-   if (!sanity_check_raw_super(sbi->raw_super, offset)) {
+   if (!sanity_check_raw_super(sbi->raw_super, sb_addr)) {
/* get kernel version */
if (c.kd >= 0) {
dev_read_version(c.version, 0, VERSION_LEN);
@@ -642,13 +633,9 @@ int validate_super_block(struct f2fs_sb_info *sbi, int 
block)
MSG(0, "Info: FSCK version\n  from \"%s\"\nto \"%s\"\n",
c.sb_version, c.version);
if (memcmp(c.sb_version, c.version, VERSION_LEN)) {
-   int ret;
-
memcpy(sbi->raw_super->version,
c.version, VERSION_LEN);
-   ret = dev_write(sbi->raw_super, offset,
-   sizeof(struct f2fs_super_block));
-   ASSERT(ret >= 0);
+   update_superblock(sbi->raw_super, 1 << sb_addr);
 

[f2fs-dev] [RFC PATCH RESEND 1/5] f2fs: support superblock checksum

2018-08-29 Thread Junling Zheng
Now we support crc32 checksum for superblock.

Reviewed-by: Chao Yu 
Signed-off-by: Junling Zheng 
---
 fs/f2fs/f2fs.h  |  2 ++
 fs/f2fs/super.c | 29 +
 fs/f2fs/sysfs.c |  7 +++
 include/linux/f2fs_fs.h |  3 ++-
 4 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 4525f4f82af0..d50d6efda96b 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -147,6 +147,7 @@ struct f2fs_mount_info {
 #define F2FS_FEATURE_INODE_CRTIME  0x0100
 #define F2FS_FEATURE_LOST_FOUND0x0200
 #define F2FS_FEATURE_VERITY0x0400  /* reserved */
+#define F2FS_FEATURE_SB_CHKSUM 0x0800
 
 #define F2FS_HAS_FEATURE(sb, mask) \
((F2FS_SB(sb)->raw_super->feature & cpu_to_le32(mask)) != 0)
@@ -3376,6 +3377,7 @@ F2FS_FEATURE_FUNCS(flexible_inline_xattr, 
FLEXIBLE_INLINE_XATTR);
 F2FS_FEATURE_FUNCS(quota_ino, QUOTA_INO);
 F2FS_FEATURE_FUNCS(inode_crtime, INODE_CRTIME);
 F2FS_FEATURE_FUNCS(lost_found, LOST_FOUND);
+F2FS_FEATURE_FUNCS(sb_chksum, SB_CHKSUM);
 
 #ifdef CONFIG_BLK_DEV_ZONED
 static inline int get_blkz_type(struct f2fs_sb_info *sbi,
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index bd57be470e23..3ffc336caea8 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2149,6 +2149,26 @@ static int sanity_check_raw_super(struct f2fs_sb_info 
*sbi,
(bh->b_data + F2FS_SUPER_OFFSET);
struct super_block *sb = sbi->sb;
unsigned int blocksize;
+   size_t crc_offset = 0;
+   __u32 crc = 0;
+
+   /* Check checksum_offset and crc in superblock */
+   if (le32_to_cpu(raw_super->feature) & F2FS_FEATURE_SB_CHKSUM) {
+   crc_offset = le32_to_cpu(raw_super->checksum_offset);
+   if (crc_offset !=
+   offsetof(struct f2fs_super_block, crc)) {
+   f2fs_msg(sb, KERN_INFO,
+   "Invalid SB checksum offset: %zu",
+   crc_offset);
+   return 1;
+   }
+   crc = le32_to_cpu(raw_super->crc);
+   if (!f2fs_crc_valid(sbi, crc, raw_super, crc_offset)) {
+   f2fs_msg(sb, KERN_INFO,
+   "Invalid SB checksum value: %u", crc);
+   return 1;
+   }
+   }
 
if (F2FS_SUPER_MAGIC != le32_to_cpu(raw_super->magic)) {
f2fs_msg(sb, KERN_INFO,
@@ -2568,6 +2588,7 @@ static int read_raw_super_block(struct f2fs_sb_info *sbi,
 int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
 {
struct buffer_head *bh;
+   __u32 crc = 0;
int err;
 
if ((recover && f2fs_readonly(sbi->sb)) ||
@@ -2576,6 +2597,13 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool 
recover)
return -EROFS;
}
 
+   /* we should update superblock crc here */
+   if (!recover && f2fs_sb_has_sb_chksum(sbi->sb)) {
+   crc = f2fs_crc32(sbi, F2FS_RAW_SUPER(sbi),
+   offsetof(struct f2fs_super_block, crc));
+   F2FS_RAW_SUPER(sbi)->crc = cpu_to_le32(crc);
+   }
+
/* write back-up superblock first */
bh = sb_bread(sbi->sb, sbi->valid_super_block ? 0 : 1);
if (!bh)
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index cd2e030e47b8..c86d91be6c48 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -120,6 +120,9 @@ static ssize_t features_show(struct f2fs_attr *a,
if (f2fs_sb_has_lost_found(sb))
len += snprintf(buf + len, PAGE_SIZE - len, "%s%s",
len ? ", " : "", "lost_found");
+   if (f2fs_sb_has_sb_chksum(sb))
+   len += snprintf(buf + len, PAGE_SIZE - len, "%s%s",
+   len ? ", " : "", "sb_checksum");
len += snprintf(buf + len, PAGE_SIZE - len, "\n");
return len;
 }
@@ -337,6 +340,7 @@ enum feat_id {
FEAT_QUOTA_INO,
FEAT_INODE_CRTIME,
FEAT_LOST_FOUND,
+   FEAT_SB_CHECKSUM,
 };
 
 static ssize_t f2fs_feature_show(struct f2fs_attr *a,
@@ -353,6 +357,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
case FEAT_QUOTA_INO:
case FEAT_INODE_CRTIME:
case FEAT_LOST_FOUND:
+   case FEAT_SB_CHECKSUM:
return snprintf(buf, PAGE_SIZE, "supported\n");
}
return 0;
@@ -433,6 +438,7 @@ F2FS_FEATURE_RO_ATTR(flexible_inline_xattr, 
FEAT_FLEXIBLE_INLINE_XATTR);
 F2FS_FEATURE_RO_ATTR(quota_ino, FEAT_QUOTA_INO);
 F2FS_FEATURE_RO_ATTR(inode_crtime, FEAT_INODE_CRTIME);
 F2FS_FEATURE_RO_ATTR(lost_found, FEAT_LOST_FOUND);
+F2FS_FEATURE_RO_ATTR(sb_checksum, FEAT_SB_CHECKSUM);
 
 #define ATTR_LIST(name) (_attr_##name.attr)
 static struct attribute *f2fs_attrs[] = {
@@ -489,6 +495,7 @@ static struct attribute *f2fs_feat_attrs[] = {
ATTR_LIST(quota_ino),

Re: [f2fs-dev] [PATCH v2] f2fs: avoid wrong decrypted data from disk

2018-08-29 Thread Chao Yu
On 2018/8/30 9:48, Jaegeuk Kim wrote:
> 1. Create a file in an encrypted directory
> 2. Do GC & drop caches
> 3. Read stale data before its bio for metapage was not issued yet
> 
> Signed-off-by: Jaegeuk Kim 
> ---
> Change log from v1:
>  - move to bio != NULL
> 
>  fs/f2fs/data.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 382c1ef9a9e4..159e411785e9 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1556,6 +1556,12 @@ static int f2fs_mpage_readpages(struct address_space 
> *mapping,
>   bio = NULL;
>   goto set_error_page;
>   }
> + } else if (unlikely(f2fs_encrypted_file(inode))) {

For android scenario, encryption becomes a common case now, so I think we can
remove unlikely here.

> + /*
> +  * If the page is under writeback, we need to wait for
> +  * its completion to see the correct decrypted data.
> +  */
> + f2fs_wait_on_block_writeback(F2FS_I_SB(inode), 
> block_nr);
>   }

Look at this again, it seems f2fs_wait_on_block_writeback() is not related to
f2fs_grab_read_bio(), so we can move it here to make code logic more clear:

if (f2fs_encrypted_file(inode)) {
/* */
f2fs_wait_on_block_writeback(F2FS_I_SB(inode), block_nr);
}

Also, in f2fs_submit_page_read(), we need to adjust to:

- f2fs_grab_read_bio()
- f2fs_wait_on_block_writeback()
- bio_add_page()

How do you think?

Thanks,

>  
>   if (bio_add_page(bio, page, blocksize, 0) < blocksize)
> 


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH] fsck.f2fs: try to recover cp_payload from valid cp pack

2018-08-29 Thread Jaegeuk Kim
On 08/30, Junling Zheng wrote:
> Hi, Jaegeuk
> 
> On 2018/8/30 10:10, Jaegeuk Kim wrote:
> > Hi Chao,
> > 
> > Could you please add this into Junling's patch series?
> > A little bit confusing between patches and reviews on them.
> > 
> 
> I'll send an new version patch series with this patch as soon as possible :)

Cool! Thank you. :)

> 
> > Thanks,
> > 
> > On 08/28, Chao Yu wrote:
> >> If sb checksum is not enabled, and cp pack is valid due to no
> >> crc inconsistence, let's try to recover cp_payload based on
> >> cp_pack_start_sum in cp pack.
> >>
> >> Signed-off-by: Chao Yu 
> >> ---
> >>  fsck/f2fs.h  |  5 +
> >>  fsck/mount.c | 10 +++---
> >>  2 files changed, 12 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/fsck/f2fs.h b/fsck/f2fs.h
> >> index d2164449db15..0d0d5e2cc399 100644
> >> --- a/fsck/f2fs.h
> >> +++ b/fsck/f2fs.h
> >> @@ -259,6 +259,11 @@ static inline unsigned long __bitmap_size(struct 
> >> f2fs_sb_info *sbi, int flag)
> >>return 0;
> >>  }
> >>  
> >> +static inline block_t __cp_payload(struct f2fs_sb_info *sbi)
> >> +{
> >> +  return le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_payload);
> >> +}
> >> +
> >>  static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
> >>  {
> >>struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi);
> >> diff --git a/fsck/mount.c b/fsck/mount.c
> >> index 4f0c3fc0db50..9421f5953cd8 100644
> >> --- a/fsck/mount.c
> >> +++ b/fsck/mount.c
> >> @@ -945,12 +945,16 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
> >>}
> >>  
> >>cp_pack_start_sum = __start_sum_addr(sbi);
> >> -  cp_payload = get_sb(cp_payload);
> >> +  cp_payload = __cp_payload(sbi);
> >>if (cp_pack_start_sum < cp_payload + 1 ||
> >>cp_pack_start_sum > blocks_per_seg - 1 -
> >>NR_CURSEG_TYPE) {
> >> -  MSG(0, "\tWrong cp_pack_start_sum(%u)\n", cp_pack_start_sum);
> >> -  return 1;
> >> +  MSG(0, "\tWrong cp_pack_start_sum(%u) or cp_payload(%u)\n",
> >> +  cp_pack_start_sum, cp_payload);
> >> +  if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM))
> >> +  return 1;
> >> +  set_sb(cp_payload, cp_pack_start_sum - 1);
> >> +  update_superblock(sb, SB_ALL);
> >>}
> >>  
> >>return 0;
> >> -- 
> >> 2.18.0.rc1
> > 
> > --
> > Check out the vibrant tech community on one of the world's most
> > engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> > ___
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > 
> > 
> 

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH] fsck.f2fs: try to recover cp_payload from valid cp pack

2018-08-29 Thread Junling Zheng
Hi, Jaegeuk

On 2018/8/30 10:10, Jaegeuk Kim wrote:
> Hi Chao,
> 
> Could you please add this into Junling's patch series?
> A little bit confusing between patches and reviews on them.
> 

I'll send an new version patch series with this patch as soon as possible :)

> Thanks,
> 
> On 08/28, Chao Yu wrote:
>> If sb checksum is not enabled, and cp pack is valid due to no
>> crc inconsistence, let's try to recover cp_payload based on
>> cp_pack_start_sum in cp pack.
>>
>> Signed-off-by: Chao Yu 
>> ---
>>  fsck/f2fs.h  |  5 +
>>  fsck/mount.c | 10 +++---
>>  2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/fsck/f2fs.h b/fsck/f2fs.h
>> index d2164449db15..0d0d5e2cc399 100644
>> --- a/fsck/f2fs.h
>> +++ b/fsck/f2fs.h
>> @@ -259,6 +259,11 @@ static inline unsigned long __bitmap_size(struct 
>> f2fs_sb_info *sbi, int flag)
>>  return 0;
>>  }
>>  
>> +static inline block_t __cp_payload(struct f2fs_sb_info *sbi)
>> +{
>> +return le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_payload);
>> +}
>> +
>>  static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
>>  {
>>  struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi);
>> diff --git a/fsck/mount.c b/fsck/mount.c
>> index 4f0c3fc0db50..9421f5953cd8 100644
>> --- a/fsck/mount.c
>> +++ b/fsck/mount.c
>> @@ -945,12 +945,16 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
>>  }
>>  
>>  cp_pack_start_sum = __start_sum_addr(sbi);
>> -cp_payload = get_sb(cp_payload);
>> +cp_payload = __cp_payload(sbi);
>>  if (cp_pack_start_sum < cp_payload + 1 ||
>>  cp_pack_start_sum > blocks_per_seg - 1 -
>>  NR_CURSEG_TYPE) {
>> -MSG(0, "\tWrong cp_pack_start_sum(%u)\n", cp_pack_start_sum);
>> -return 1;
>> +MSG(0, "\tWrong cp_pack_start_sum(%u) or cp_payload(%u)\n",
>> +cp_pack_start_sum, cp_payload);
>> +if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM))
>> +return 1;
>> +set_sb(cp_payload, cp_pack_start_sum - 1);
>> +update_superblock(sb, SB_ALL);
>>  }
>>  
>>  return 0;
>> -- 
>> 2.18.0.rc1
> 
> --
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> ___
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 
> 



--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH] fsck.f2fs: try to recover cp_payload from valid cp pack

2018-08-29 Thread Jaegeuk Kim
Hi Chao,

Could you please add this into Junling's patch series?
A little bit confusing between patches and reviews on them.

Thanks,

On 08/28, Chao Yu wrote:
> If sb checksum is not enabled, and cp pack is valid due to no
> crc inconsistence, let's try to recover cp_payload based on
> cp_pack_start_sum in cp pack.
> 
> Signed-off-by: Chao Yu 
> ---
>  fsck/f2fs.h  |  5 +
>  fsck/mount.c | 10 +++---
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/fsck/f2fs.h b/fsck/f2fs.h
> index d2164449db15..0d0d5e2cc399 100644
> --- a/fsck/f2fs.h
> +++ b/fsck/f2fs.h
> @@ -259,6 +259,11 @@ static inline unsigned long __bitmap_size(struct 
> f2fs_sb_info *sbi, int flag)
>   return 0;
>  }
>  
> +static inline block_t __cp_payload(struct f2fs_sb_info *sbi)
> +{
> + return le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_payload);
> +}
> +
>  static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
>  {
>   struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi);
> diff --git a/fsck/mount.c b/fsck/mount.c
> index 4f0c3fc0db50..9421f5953cd8 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -945,12 +945,16 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
>   }
>  
>   cp_pack_start_sum = __start_sum_addr(sbi);
> - cp_payload = get_sb(cp_payload);
> + cp_payload = __cp_payload(sbi);
>   if (cp_pack_start_sum < cp_payload + 1 ||
>   cp_pack_start_sum > blocks_per_seg - 1 -
>   NR_CURSEG_TYPE) {
> - MSG(0, "\tWrong cp_pack_start_sum(%u)\n", cp_pack_start_sum);
> - return 1;
> + MSG(0, "\tWrong cp_pack_start_sum(%u) or cp_payload(%u)\n",
> + cp_pack_start_sum, cp_payload);
> + if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM))
> + return 1;
> + set_sb(cp_payload, cp_pack_start_sum - 1);
> + update_superblock(sb, SB_ALL);
>   }
>  
>   return 0;
> -- 
> 2.18.0.rc1

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH v2] f2fs: avoid wrong decrypted data from disk

2018-08-29 Thread Jaegeuk Kim
1. Create a file in an encrypted directory
2. Do GC & drop caches
3. Read stale data before its bio for metapage was not issued yet

Signed-off-by: Jaegeuk Kim 
---
Change log from v1:
 - move to bio != NULL

 fs/f2fs/data.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 382c1ef9a9e4..159e411785e9 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1556,6 +1556,12 @@ static int f2fs_mpage_readpages(struct address_space 
*mapping,
bio = NULL;
goto set_error_page;
}
+   } else if (unlikely(f2fs_encrypted_file(inode))) {
+   /*
+* If the page is under writeback, we need to wait for
+* its completion to see the correct decrypted data.
+*/
+   f2fs_wait_on_block_writeback(F2FS_I_SB(inode), 
block_nr);
}
 
if (bio_add_page(bio, page, blocksize, 0) < blocksize)
-- 
2.17.0.441.gb46fe60e1d-goog


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH 1/2] add cotnfigure option --with-root-libdir

2018-08-29 Thread Jaegeuk Kim
Thank you so much for kind explanation. This is really awesome especially for
newbies like me. This gives answers to me for all my own stupid questions like
why I need to investigate all the lib changes in order to determine bumping
current/revision/age numbers or not.

I'd definitely like to follow this rule to avoid any verion changes later, as
I expect the next release will be the last major update. For dependency parts,
I'll study and practice them for a while.

BTW, can I archive this like in f2fs-tools/VERSIONING?

Thanks,

On 08/28, Theodore Y. Ts'o wrote:
> On Tue, Aug 28, 2018 at 09:59:44AM -0700, Jaegeuk Kim wrote:
> > 
> > https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html
> > 
> > I understood that, if there is no interface change but some implementation
> > changes, I need to bump revision. If new interface is added, for example, I
> > need to bump current while revision=0 and age++.
> 
> So part of the problem here is that libtool is doing something really
> strange because they are trying to use some abstract concept that is
> OS-independent.  I don't use libtool because I find it horribly
> complex and doesn't add enough value to be worth the complexity.
> 
> So I'll tell you how things work with respect to Linux's ELF version
> numbering system.  Translating this to libtool's wierd "current,
> revision, age" terminology is left as an exercise to the reader.  I've
> looked at the libtool documentation, and it confuses me horribly.
> Reading it, I suspect it's wrong, but I don't have the time to
> experiment to confirm that the documentation is wrong and how it
> diverges from the libtool implementation.
> 
> So let me explain things using the ELF shared library terminology,
> which is "major version, minor version, patchlevel".  This shows up in
> the library name:
> 
>   libudev.so.1.6.11
> 
> So in this example, the major version number is 1, the minor version
> is 6, and the patchlevel is 11.  The patchlevel is entirely optional,
> and many packages don't use it at all.  The minor number is also
> mostly useless on Linux, but it's still there for historical reasons.
> The patchlevel and minor version numbers were useful back for SunOS
> (and Linux a.out shared library), back when there weren't rpm and dpkg
> as package managers.
> 
> So many modern Linux shared libraries will only use the major and
> minor version numbers, e.g:
> 
>   libext2fs.so.2.4
> 
> The only thing you really need to worry about is the major version
> number, really.  The minor version is *supposed* to change when new
> interfaces has changed (but I and most other people don't do that any
> more).  But the big deal is that the major number *must* get bumped if
> an existing interface has *changed*.
> 
> So let's talk about the major version number, and then we'll talk
> about why the minor version number isn't really a big deal for Linux.
> 
> So if you change any of the library's function signatures --- and this
> includes changing a type from a 32-bit integer to a 64-bit integer,
> that's an ABI breakage, and so you must bump the major version number
> so that a program that was linked against libfoo.so.4 doesn't try to
> use libfoo.so.5.  That's really the key --- will a program linked
> against the previous version library break if it links against the
> newer version.  If it does, then you need to bump the version number.
> 
> So for structures, if you change any of the existing fields, or if the
> application program allocates the structure --- either by declaring it
> on the stack, or via malloc() --- and you expand the structure,
> obviously that will cause problem, and so that's an ABI break.
> 
> If however, you arrange to have structures allocated by the library,
> and struct members are always added at the end, then an older program
> won't have any problems.  You can guarantee this by simply only using
> a pointer to the struct in your public header files, and defining the
> struct in a private header file that is not available to userspace
> programs.
> 
> Similarly, adding new functions never breaks the ABI.  That's because
> older program won't try to use the newer interfaces.  So if I need to
> change an interface to a function, what I'll generally do is to define
> a new function, and then implement the older function in terms of the
> newer one.  For example:
> 
> extern errcode_t ext2fs_open(const char *name, int flags, int superblock,
>unsigned int block_size, io_manager manager,
>ext2_filsys *ret_fs);
>
> extern errcode_t ext2fs_open2(const char *name, const char *io_options,
> int flags, int superblock,
> unsigned int block_size, io_manager manager,
> ext2_filsys *hret_fs);
> 
> As far as the minor version numbers are concerned, the dynamic linker
> doesn't use it.  In SunOS 4, if you 

Re: [f2fs-dev] [PATCH] fsck.f2fs: detect and recover corrupted quota file

2018-08-29 Thread Chao Yu
Hi Sheng,

On 2018/8/29 23:06, Sheng Yong wrote:
> Hi, Chao
> 
> On 2018/8/29 20:09, Chao Yu wrote:
>> Once quota file is corrupted, kernel will set CP_QUOTA_NEED_FSCK_FLAG
>> into checkpoint pack, this patch makes fsck supporting to detect the flag
>> and try to rebuild corrupted quota file.
>>
> Do we need to drop recovery data? Recovery data is never checked by fsck,
> if fsck tries to rebuild quota, it needs to allocate blocks from cursegs,
> this may overwrite data blocks which may be recovered latter.

IMO, we'd better not to drop those data, so I prefer to support recovering
fsynced data in fsck, and do recovery in the condition of:
- needs to allocate block from log header.
- through defined option.

How do you think?

Thanks,

> 
> thanks
>> Signed-off-by: Chao Yu 
>> ---
>>   fsck/fsck.c   | 3 ++-
>>   fsck/mount.c  | 6 --
>>   include/f2fs_fs.h | 2 ++
>>   lib/libf2fs.c | 2 ++
>>   4 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/fsck/fsck.c b/fsck/fsck.c
>> index f080d3c8741c..10b69ef403ef 100644
>> --- a/fsck/fsck.c
>> +++ b/fsck/fsck.c
>> @@ -2668,7 +2668,8 @@ int fsck_verify(struct f2fs_sb_info *sbi)
>>   flush_curseg_sit_entries(sbi);
>>   }
>>   fix_checkpoint(sbi);
>> -    } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG)) {
>> +    } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG) ||
>> +    is_set_ckpt_flags(cp, CP_QUOTA_NEED_FSCK_FLAG)) {
>>   write_checkpoint(sbi);
>>   }
>>   }
>> diff --git a/fsck/mount.c b/fsck/mount.c
>> index a2448e370c0b..168cf387991f 100644
>> --- a/fsck/mount.c
>> +++ b/fsck/mount.c
>> @@ -405,6 +405,8 @@ void print_ckpt_info(struct f2fs_sb_info *sbi)
>>   void print_cp_state(u32 flag)
>>   {
>>   MSG(0, "Info: checkpoint state = %x : ", flag);
>> +    if (flag & CP_QUOTA_NEED_FSCK_FLAG)
>> +    MSG(0, "%s", " quota_need_fsck");
>>   if (flag & CP_LARGE_NAT_BITMAP_FLAG)
>>   MSG(0, "%s", " large_nat_bitmap");
>>   if (flag & CP_NOCRC_RECOVERY_FLAG)
>> @@ -2532,8 +2534,8 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
>>   u32 flag = get_cp(ckpt_flags);
>>     if (flag & CP_FSCK_FLAG ||
>> -    (exist_qf_ino(sb) && (!(flag & CP_UMOUNT_FLAG) ||
>> -    flag & CP_ERROR_FLAG))) {
>> +    flag & CP_QUOTA_NEED_FSCK_FLAG ||
>> +    (exist_qf_ino(sb) && (flag & CP_ERROR_FLAG))) {
>>   c.fix_on = 1;
>>   } else if (!c.preen_mode) {
>>   print_cp_state(flag);
>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
>> index 9396c785a254..160eaf72f0b6 100644
>> --- a/include/f2fs_fs.h
>> +++ b/include/f2fs_fs.h
>> @@ -372,6 +372,7 @@ struct f2fs_configuration {
>>   int defset;
>>   int bug_on;
>>   int auto_fix;
>> +    int quota_fix;
>>   int preen_mode;
>>   int ro;
>>   int preserve_limits;    /* preserve quota limits */
>> @@ -641,6 +642,7 @@ struct f2fs_super_block {
>>   /*
>>    * For checkpoint
>>    */
>> +#define CP_QUOTA_NEED_FSCK_FLAG    0x0800
>>   #define CP_LARGE_NAT_BITMAP_FLAG    0x0400
>>   #define CP_NOCRC_RECOVERY_FLAG    0x0200
>>   #define CP_TRIMMED_FLAG    0x0100
>> diff --git a/lib/libf2fs.c b/lib/libf2fs.c
>> index a1f8beb1f78d..192b8125de36 100644
>> --- a/lib/libf2fs.c
>> +++ b/lib/libf2fs.c
>> @@ -619,6 +619,8 @@ void f2fs_init_configuration(void)
>>   /* default root owner */
>>   c.root_uid = getuid();
>>   c.root_gid = getgid();
>> +
>> +    c.quota_fix = 1;
>>   }
>>     #ifdef HAVE_SETMNTENT
>>
> 
> 
> --
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> ___
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH] fsck.f2fs: detect and recover corrupted quota file

2018-08-29 Thread Sheng Yong

Hi, Chao

On 2018/8/29 20:09, Chao Yu wrote:

Once quota file is corrupted, kernel will set CP_QUOTA_NEED_FSCK_FLAG
into checkpoint pack, this patch makes fsck supporting to detect the flag
and try to rebuild corrupted quota file.


Do we need to drop recovery data? Recovery data is never checked by fsck,
if fsck tries to rebuild quota, it needs to allocate blocks from cursegs,
this may overwrite data blocks which may be recovered latter.

thanks

Signed-off-by: Chao Yu 
---
  fsck/fsck.c   | 3 ++-
  fsck/mount.c  | 6 --
  include/f2fs_fs.h | 2 ++
  lib/libf2fs.c | 2 ++
  4 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index f080d3c8741c..10b69ef403ef 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -2668,7 +2668,8 @@ int fsck_verify(struct f2fs_sb_info *sbi)
flush_curseg_sit_entries(sbi);
}
fix_checkpoint(sbi);
-   } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG)) {
+   } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG) ||
+   is_set_ckpt_flags(cp, CP_QUOTA_NEED_FSCK_FLAG)) {
write_checkpoint(sbi);
}
}
diff --git a/fsck/mount.c b/fsck/mount.c
index a2448e370c0b..168cf387991f 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -405,6 +405,8 @@ void print_ckpt_info(struct f2fs_sb_info *sbi)
  void print_cp_state(u32 flag)
  {
MSG(0, "Info: checkpoint state = %x : ", flag);
+   if (flag & CP_QUOTA_NEED_FSCK_FLAG)
+   MSG(0, "%s", " quota_need_fsck");
if (flag & CP_LARGE_NAT_BITMAP_FLAG)
MSG(0, "%s", " large_nat_bitmap");
if (flag & CP_NOCRC_RECOVERY_FLAG)
@@ -2532,8 +2534,8 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
u32 flag = get_cp(ckpt_flags);
  
  		if (flag & CP_FSCK_FLAG ||

-   (exist_qf_ino(sb) && (!(flag & CP_UMOUNT_FLAG) ||
-   flag & CP_ERROR_FLAG))) {
+   flag & CP_QUOTA_NEED_FSCK_FLAG ||
+   (exist_qf_ino(sb) && (flag & CP_ERROR_FLAG))) {
c.fix_on = 1;
} else if (!c.preen_mode) {
print_cp_state(flag);
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 9396c785a254..160eaf72f0b6 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -372,6 +372,7 @@ struct f2fs_configuration {
int defset;
int bug_on;
int auto_fix;
+   int quota_fix;
int preen_mode;
int ro;
int preserve_limits;/* preserve quota limits */
@@ -641,6 +642,7 @@ struct f2fs_super_block {
  /*
   * For checkpoint
   */
+#define CP_QUOTA_NEED_FSCK_FLAG0x0800
  #define CP_LARGE_NAT_BITMAP_FLAG  0x0400
  #define CP_NOCRC_RECOVERY_FLAG0x0200
  #define CP_TRIMMED_FLAG   0x0100
diff --git a/lib/libf2fs.c b/lib/libf2fs.c
index a1f8beb1f78d..192b8125de36 100644
--- a/lib/libf2fs.c
+++ b/lib/libf2fs.c
@@ -619,6 +619,8 @@ void f2fs_init_configuration(void)
/* default root owner */
c.root_uid = getuid();
c.root_gid = getgid();
+
+   c.quota_fix = 1;
  }
  
  #ifdef HAVE_SETMNTENT





--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH 1/5] ext2: cache NULL when both default_acl and acl are NULL

2018-08-29 Thread Jan Kara
On Tue 14-08-18 22:16:30, Chengguang Xu wrote:
> default_acl and acl of newly created inode will be initiated
> as ACL_NOT_CACHED in vfs function inode_init_always() and later
> will be updated by calling xxx_init_acl() in specific filesystems.
> Howerver, when default_acl and acl are NULL then they keep the value
> of ACL_NOT_CACHED, this patch tries to cache NULL for acl/default_acl
> in this case.
> 
> Signed-off-by: Chengguang Xu 
> ---
>  fs/ext2/acl.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/ext2/acl.c b/fs/ext2/acl.c
> index 224c04abb2e5..74411e8ea507 100644
> --- a/fs/ext2/acl.c
> +++ b/fs/ext2/acl.c
> @@ -262,5 +262,8 @@ ext2_init_acl(struct inode *inode, struct inode *dir)
>   error = __ext2_set_acl(inode, acl, ACL_TYPE_ACCESS);
>   posix_acl_release(acl);
>   }
> + if (!default_acl && !acl)
> + cache_no_acl(inode);
> +

Thanks for the patch. I'd somewhat prefer something equivalent to:

if (!acl)
inode->i_acl = NULL;
if (!default_acl)
inode->i_default_acl = NULL;

given the code in ext2_init_acl(). It would be most natural like:

if (default_acl) {
do foo
} else {
inode->i_default_acl = NULL;
}

And similarly for 'acl'.

Honza
-- 
Jan Kara 
SUSE Labs, CR

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [PATCH] fsck.f2fs: detect and recover corrupted quota file

2018-08-29 Thread Chao Yu
Once quota file is corrupted, kernel will set CP_QUOTA_NEED_FSCK_FLAG
into checkpoint pack, this patch makes fsck supporting to detect the flag
and try to rebuild corrupted quota file.

Signed-off-by: Chao Yu 
---
 fsck/fsck.c   | 3 ++-
 fsck/mount.c  | 6 --
 include/f2fs_fs.h | 2 ++
 lib/libf2fs.c | 2 ++
 4 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index f080d3c8741c..10b69ef403ef 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -2668,7 +2668,8 @@ int fsck_verify(struct f2fs_sb_info *sbi)
flush_curseg_sit_entries(sbi);
}
fix_checkpoint(sbi);
-   } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG)) {
+   } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG) ||
+   is_set_ckpt_flags(cp, CP_QUOTA_NEED_FSCK_FLAG)) {
write_checkpoint(sbi);
}
}
diff --git a/fsck/mount.c b/fsck/mount.c
index a2448e370c0b..168cf387991f 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -405,6 +405,8 @@ void print_ckpt_info(struct f2fs_sb_info *sbi)
 void print_cp_state(u32 flag)
 {
MSG(0, "Info: checkpoint state = %x : ", flag);
+   if (flag & CP_QUOTA_NEED_FSCK_FLAG)
+   MSG(0, "%s", " quota_need_fsck");
if (flag & CP_LARGE_NAT_BITMAP_FLAG)
MSG(0, "%s", " large_nat_bitmap");
if (flag & CP_NOCRC_RECOVERY_FLAG)
@@ -2532,8 +2534,8 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
u32 flag = get_cp(ckpt_flags);
 
if (flag & CP_FSCK_FLAG ||
-   (exist_qf_ino(sb) && (!(flag & CP_UMOUNT_FLAG) ||
-   flag & CP_ERROR_FLAG))) {
+   flag & CP_QUOTA_NEED_FSCK_FLAG ||
+   (exist_qf_ino(sb) && (flag & CP_ERROR_FLAG))) {
c.fix_on = 1;
} else if (!c.preen_mode) {
print_cp_state(flag);
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 9396c785a254..160eaf72f0b6 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -372,6 +372,7 @@ struct f2fs_configuration {
int defset;
int bug_on;
int auto_fix;
+   int quota_fix;
int preen_mode;
int ro;
int preserve_limits;/* preserve quota limits */
@@ -641,6 +642,7 @@ struct f2fs_super_block {
 /*
  * For checkpoint
  */
+#define CP_QUOTA_NEED_FSCK_FLAG0x0800
 #define CP_LARGE_NAT_BITMAP_FLAG   0x0400
 #define CP_NOCRC_RECOVERY_FLAG 0x0200
 #define CP_TRIMMED_FLAG0x0100
diff --git a/lib/libf2fs.c b/lib/libf2fs.c
index a1f8beb1f78d..192b8125de36 100644
--- a/lib/libf2fs.c
+++ b/lib/libf2fs.c
@@ -619,6 +619,8 @@ void f2fs_init_configuration(void)
/* default root owner */
c.root_uid = getuid();
c.root_gid = getgid();
+
+   c.quota_fix = 1;
 }
 
 #ifdef HAVE_SETMNTENT
-- 
2.18.0.rc1


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel