Re: [f2fs-dev] [PATCH 03/18] f2fs crypto: declare some definitions for f2fs encryption feature
Please keep in mind that I'm also working on transparent compression. I'm watching this thread closely so that I can implement a compression library alongside the crypto library. If there is any interest or benefit, I would be glad to work together so that the two can be done cooperatively at the same time. On 05/13/2015 06:56 PM, Jaegeuk Kim wrote: On Thu, May 14, 2015 at 10:37:21AM +1000, Dave Chinner wrote: On Tue, May 12, 2015 at 11:48:02PM -0700, Jaegeuk Kim wrote: On Wed, May 13, 2015 at 12:02:08PM +1000, Dave Chinner wrote: On Fri, May 08, 2015 at 09:20:38PM -0700, Jaegeuk Kim wrote: This definitions will be used by inode and superblock for encyption. How much of this crypto stuff is common with or only slightly modified from the ext4 code? Is the behaviour and features the same? Is the user API and management tools the same? IMO, if there is any amount of overlap, then we should be implementing this stuff as generic code, not propagating the same code through multiple filesystems via copy-n-paste-n-modify. This will simply end up with diverging code, different bugs and feature sets, and none of the implementations will get the review and maintenance they really require... And, FWIW, this is the reason why I originally asked for the ext4 encryption code to be pulled up to the VFS: precisely so we didn't end up with a rapid proliferation of individual in-filesystem encryption implementations that are all slightly different... Totally agreed! AFAIK, Ted wants to push the codes as a crypto library into fs/ finally, so I believe most part of crypto codes are common. Can I suggest fs/crypto/ if there are going to be multiple files? No problem at all. I'll do. But, in order to realize that quickly, Ted implemented the feature to finalize on-disk and in-memory design in EXT4 as a first step. Then, I've been catching up and validating its design by implementing it in F2FS, which also intends to figure out what crypto codes can be exactly common. Excellent. That will make it easier and less error prone for other filesystems to implement it, too! As Ted mentioned before, since next android version tries to use per-file encryption, F2FS also needs to support it as quick as possible likewise EXT4. Fair enough. Meanwhile, surely I've been working on writing patches to push them into fs/; currenlty, I did for cryto.c and will do for crypto_key.c and crypto_fname.c. But, it needs to think about crypto_policy.c differently, since it may depend on how each filesystem stores the policy information respectively; we cannot push all the filesystems should use xattrs, right? All filesystems likely to implement per-file crypto support xattrs, and this is exactly what xattrs are designed for. e.g. we already require xattrs for generic security labels, ACLs, etc. Hence per-file crypto information should also use a common, shared xattr format. That way it only needs to be implemented once in the generic code and there's very little (hopefully nothing!) each filesystem has to customise to store the crypto information for each file. Ok, I see. Let me take a look at that too. Thank you for sharing your thoughts. :) Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- One dashboard for servers and applications across Physical-Virtual-Cloud Widest out-of-the-box monitoring support with 50+ applications Performance metrics, stats and reports that give you Actionable Insights Deep dive visibility with transaction tracing using APM Insight. http://ad.doubleclick.net/ddm/clk/290420510;117567292;y ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] Space leak in f2fs
Hi Hu,
I've been rethinking about whole this issue differently.
And, now I'm starting to test with the below patch instead of previous one.
Thanks,
Signed-off-by: Jaegeuk Kim jaeg...@kernel.org
---
fs/f2fs/checkpoint.c | 19 +++
fs/f2fs/data.c | 4
fs/f2fs/f2fs.h | 1 +
fs/f2fs/super.c | 15 ---
4 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 7b7a9d8..74875fb 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -378,6 +378,20 @@ static void __remove_ino_entry(struct f2fs_sb_info *sbi,
nid_t ino, int type)
spin_unlock(im-ino_lock);
}
+static bool __exist_ino_entry(struct f2fs_sb_info *sbi, nid_t ino, int type)
+{
+ struct inode_management *im = sbi-im[type];
+ struct ino_entry *e;
+ bool exist = false;
+
+ spin_lock(im-ino_lock);
+ e = radix_tree_lookup(im-ino_root, ino);
+ if (e)
+ exist = true;
+ spin_unlock(im-ino_lock);
+ return exist;
+}
+
void add_dirty_inode(struct f2fs_sb_info *sbi, nid_t ino, int type)
{
/* add new dirty ino entry into list */
@@ -458,6 +472,11 @@ void remove_orphan_inode(struct f2fs_sb_info *sbi, nid_t
ino)
__remove_ino_entry(sbi, ino, ORPHAN_INO);
}
+bool is_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
+{
+ return __exist_ino_entry(sbi, ino, ORPHAN_INO);
+}
+
static void recover_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
{
struct inode *inode = f2fs_iget(sbi-sb, ino);
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index b0cc2aa..1988f5f 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1749,6 +1749,10 @@ write:
goto out;
}
+ /* if orphan inode, we don't need to write its data */
+ if (is_orphan_inode(sbi, inode-i_ino))
+ goto out;
+
if (!wbc-for_reclaim)
need_balance_fs = true;
else if (has_not_enough_free_secs(sbi, 0))
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 8f1f21a..697346a 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1726,6 +1726,7 @@ int acquire_orphan_inode(struct f2fs_sb_info *);
void release_orphan_inode(struct f2fs_sb_info *);
void add_orphan_inode(struct f2fs_sb_info *, nid_t);
void remove_orphan_inode(struct f2fs_sb_info *, nid_t);
+bool is_orphan_inode(struct f2fs_sb_info *, nid_t);
void recover_orphan_inodes(struct f2fs_sb_info *);
int get_valid_checkpoint(struct f2fs_sb_info *);
void update_dirty_page(struct inode *, struct page *);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 19438f2..1d0973a 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -422,20 +422,6 @@ static struct inode *f2fs_alloc_inode(struct super_block
*sb)
return fi-vfs_inode;
}
-static int f2fs_drop_inode(struct inode *inode)
-{
- /*
-* This is to avoid a deadlock condition like below.
-* writeback_single_inode(inode)
-* - f2fs_write_data_page
-*- f2fs_gc - iput - evict
-* - inode_wait_for_writeback(inode)
-*/
- if (!inode_unhashed(inode) inode-i_state I_SYNC)
- return 0;
- return generic_drop_inode(inode);
-}
-
/*
* f2fs_dirty_inode() is called from __mark_inode_dirty()
*
@@ -759,7 +745,6 @@ restore_opts:
static struct super_operations f2fs_sops = {
.alloc_inode= f2fs_alloc_inode,
- .drop_inode = f2fs_drop_inode,
.destroy_inode = f2fs_destroy_inode,
.write_inode= f2fs_write_inode,
.dirty_inode= f2fs_dirty_inode,
--
2.1.1
--
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs crypto: fix incorrect release for crypto ctx
When encryption feature is enable, if we rmmod f2fs module,
we will encounter a stack backtrace reported in syslog:
BUG: Bad page state in process rmmod pfn:aaf8a
page:f0f4f148 count:0 mapcount:129 mapping:ee2f4104 index:0x80
flags: 0xee2830a4(referenced|lru|slab|private_2|writeback|swapbacked|mlocked)
page dumped because: PAGE_FLAGS_CHECK_AT_FREE flag(s) set
bad because of flags:
flags: 0x2030a0(lru|slab|private_2|writeback|mlocked)
Modules linked in: f2fs(O-) fuse bnep rfcomm bluetooth dm_crypt binfmt_misc
snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm
snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq snd_timer snd_seq_device
joydev ppdev mac_hid lp hid_generic i2c_piix4
parport_pc psmouse snd serio_raw parport soundcore ext4 jbd2 mbcache usbhid hid
e1000 [last unloaded: f2fs]
CPU: 1 PID: 3049 Comm: rmmod Tainted: GB O4.1.0-rc3+ #10
Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
c0021eb4 c15b7518 f0f4f148 c0021ed8 c112e0b7 c1779174
c9b75674 000aaf8a 01b13ce1 c17791a4 f0f4f148 ee2830a4 c0021ef8 c112e3c3
f0f4f148 c0021f34 f0f4f148 ee2830a4 ef9f c0021f20 c112fdf8
Call Trace:
[c15b7518] dump_stack+0x41/0x52
[c112e0b7] bad_page.part.72+0xa7/0x100
[c112e3c3] free_pages_prepare+0x213/0x220
[c112fdf8] free_hot_cold_page+0x28/0x120
[c1073380] ? try_to_wake_up+0x2b0/0x2b0
[c112ff15] __free_pages+0x25/0x30
[c112c4fd] mempool_free_pages+0xd/0x10
[c112c5f1] mempool_free+0x31/0x90
[f0f441cf] f2fs_exit_crypto+0x6f/0xf0 [f2fs]
[f0f456c4] exit_f2fs_fs+0x23/0x95f [f2fs]
[c10c30e0] SyS_delete_module+0x130/0x180
[c11556d6] ? vm_munmap+0x46/0x60
[c15bd888] sysenter_do_call+0x12/0x12
The reason is that:
since commit 0827e645fd35
(f2fs crypto: shrink size of the f2fs_crypto_ctx structure) is merged,
some fields in f2fs_crypto_ctx structure are merged into a union as they
will never be used simultaneously in write path, read path or on free list.
In f2fs_exit_crypto, we traverse each crypto ctx from free list, in this
moment, our free_list field in union is valid, but still we will try to
release memory space which is pointed by other invalid field in union
structure for each ctx.
Then the error occurs, let's fix it with this patch.
Signed-off-by: Chao Yu chao2...@samsung.com
---
fs/f2fs/crypto.c | 8
1 file changed, 8 deletions(-)
diff --git a/fs/f2fs/crypto.c b/fs/f2fs/crypto.c
index e36eddd..0a66c9b 100644
--- a/fs/f2fs/crypto.c
+++ b/fs/f2fs/crypto.c
@@ -233,14 +233,6 @@ void f2fs_exit_crypto(void)
struct f2fs_crypto_ctx *pos, *n;
list_for_each_entry_safe(pos, n, f2fs_free_crypto_ctxs, free_list) {
- if (pos-w.bounce_page) {
- if (pos-flags
- F2FS_BOUNCE_PAGE_REQUIRES_FREE_ENCRYPT_FL)
- __free_page(pos-w.bounce_page);
- else
- mempool_free(pos-w.bounce_page,
- f2fs_bounce_page_pool);
- }
if (pos-tfm)
crypto_free_tfm(pos-tfm);
kmem_cache_free(f2fs_crypto_ctx_cachep, pos);
--
2.3.3
--
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [patch] f2fs: cleanup a confusing indent
The return was not indented far enough so it looked like it was supposed
to go with the other if statement.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 17e89ba..741db3a 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -554,8 +554,8 @@ static void add_discard_addrs(struct f2fs_sb_info *sbi,
struct cp_control *cpc)
if (!force) {
if (!test_opt(sbi, DISCARD) || !se-valid_blocks ||
- SM_I(sbi)-nr_discards = SM_I(sbi)-max_discards)
- return;
+ SM_I(sbi)-nr_discards = SM_I(sbi)-max_discards)
+ return;
}
/* SIT_VBLOCK_MAP_SIZE should be multiple of sizeof(unsigned long) */
--
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
