| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <51F9D407.8040703@cn.fujitsu.com>
Date: Thu, 01 Aug 2013 11:20:39 +0800
From: Gu Zheng <guz.fnst@...fujitsu.com>
To: Namjae Jeon <linkinjeon@...il.com>
CC: jaegeuk.kim@...sung.com, linux-f2fs-devel@...ts.sourceforge.net,
linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
Namjae Jeon <namjae.jeon@...sung.com>,
Pankaj Kumar <pankaj.km@...sung.com>
Subject: Re: [PATCH 2/2] f2fs: add sysfs entries to select the gc policy
Hi Jeon,
On 07/31/2013 10:33 PM, Namjae Jeon wrote:
> From: Namjae Jeon <namjae.jeon@...sung.com>
>
> Add sysfs entries namely gc_long_idle and gc_short_idle to control the
> gc policy. Where long idle corresponds to selecting a cost benefit approach,
> while short idle corresponds to selecting a greedy approach to garbage
> collection. The selection is mutually exclusive one approach will work at
> any point.
>
> Signed-off-by: Namjae Jeon <namjae.jeon@...sung.com>
> Signed-off-by: Pankaj Kumar <pankaj.km@...sung.com>
> ---
> Documentation/ABI/testing/sysfs-fs-f2fs | 12 +++++++
> Documentation/filesystems/f2fs.txt | 8 +++++
> fs/f2fs/gc.c | 22 ++++++++++--
> fs/f2fs/gc.h | 4 +++
> fs/f2fs/super.c | 59 +++++++++++++++++++++++++++++--
> 5 files changed, 99 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> index 5f44095..96b62ea 100644
> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> @@ -19,4 +19,16 @@ Description:
> Controls the default sleep time for gc_thread. Time
> is in milliseconds.
>
> +What: /sys/fs/f2fs/<disk>/gc_long_idle
> +Date: July 2013
> +Contact: "Namjae Jeon" <namjae.jeon@...sung.com>
> +Description:
> + Controls the selection of gc policy. long_idle is used
> + to select the cost benefit approach for garbage collection.
>
> +What: /sys/fs/f2fs/<disk>/gc_short_idle
> +Date: July 2013
> +Contact: "Namjae Jeon" <namjae.jeon@...sung.com>
> +Description:
> + Controls the selection of gc policy. short_idle is used
> + to select the greedy approach for garbage collection.
> diff --git a/Documentation/filesystems/f2fs.txt b/Documentation/filesystems/f2fs.txt
> index 2e9e873..06dd5d7 100644
> --- a/Documentation/filesystems/f2fs.txt
> +++ b/Documentation/filesystems/f2fs.txt
> @@ -158,6 +158,14 @@ Files in /sys/fs/f2fs/<devname>
> time for the garbage collection thread. Time is
> in milliseconds.
>
> + gc_long_idle This parameter controls the selection of cost
> + benefit approach for garbage collectoin. Writing
> + 1 to this file will select the cost benefit policy.
> +
> + gc_short_idle This parameter controls the selection of greedy
> + approach for the garbage collection. Writing 1
> + to this file will select the greedy policy.
Why introduce two opposite attributes? It'll cause some confusion condition if we
double enable/disable them.
> +
> ================================================================================
> USAGE
> ================================================================================
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 60d4f67..af2d9d7 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -106,6 +106,8 @@ int start_gc_thread(struct f2fs_sb_info *sbi)
> gc_th->max_sleep_time = DEF_GC_THREAD_MAX_SLEEP_TIME;
> gc_th->no_gc_sleep_time = DEF_GC_THREAD_NOGC_SLEEP_TIME;
>
> + gc_th->long_idle = gc_th->short_idle = 0;
> +
> sbi->gc_thread = gc_th;
> init_waitqueue_head(&sbi->gc_thread->gc_wait_queue_head);
> sbi->gc_thread->f2fs_gc_task = kthread_run(gc_thread_func, sbi,
> @@ -130,9 +132,23 @@ void stop_gc_thread(struct f2fs_sb_info *sbi)
> sbi->gc_thread = NULL;
> }
>
> -static int select_gc_type(int gc_type)
> +static int select_gc_type(struct f2fs_gc_kthread *gc_th, int gc_type)
> {
> - return (gc_type == BG_GC) ? GC_CB : GC_GREEDY;
> + int gc_mode;
> +
> + if (gc_th) {
> + if (gc_th->long_idle) {
> + gc_mode = GC_CB;
> + goto out;
> + } else if (gc_th->short_idle) {
> + gc_mode = GC_GREEDY;
> + goto out;
> + }
> + }
> +
> + gc_mode = (gc_type == BG_GC) ? GC_CB : GC_GREEDY;
> +out:
> + return gc_mode;
> }
>
> static void select_policy(struct f2fs_sb_info *sbi, int gc_type,
> @@ -145,7 +161,7 @@ static void select_policy(struct f2fs_sb_info *sbi, int gc_type,
> p->dirty_segmap = dirty_i->dirty_segmap[type];
> p->ofs_unit = 1;
> } else {
> - p->gc_mode = select_gc_type(gc_type);
> + p->gc_mode = select_gc_type(sbi->gc_thread, gc_type);
> p->dirty_segmap = dirty_i->dirty_segmap[DIRTY];
> p->ofs_unit = sbi->segs_per_sec;
> }
> diff --git a/fs/f2fs/gc.h b/fs/f2fs/gc.h
> index f4bf44c..b2faae5 100644
> --- a/fs/f2fs/gc.h
> +++ b/fs/f2fs/gc.h
> @@ -30,6 +30,10 @@ struct f2fs_gc_kthread {
> unsigned int min_sleep_time;
> unsigned int max_sleep_time;
> unsigned int no_gc_sleep_time;
> +
> + /* for changing gc mode */
> + unsigned int long_idle;
> + unsigned int short_idle;
The same issue I mentioned in the front patch, they're per f2fs sb infos.
Regards,
Gu
> };
>
> struct inode_entry {
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 30de280..3db806b 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -107,6 +107,56 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
> return count;
> }
>
> +static ssize_t f2fs_gc_long_idle_store(struct f2fs_attr *a,
> + struct f2fs_sb_info *sbi,
> + const char *buf, size_t count)
> +{
> + struct f2fs_gc_kthread *gc_kth = sbi->gc_thread;
> + unsigned long t;
> + ssize_t ret;
> +
> + if (!gc_kth)
> + return -EFAULT;
> +
> + ret = kstrtoul(skip_spaces(buf), 0, &t);
> + if (ret < 0)
> + return ret;
> +
> + if (t == 0)
> + gc_kth->long_idle = 0;
> + else if (t == 1) {
> + gc_kth->long_idle = 1;
> + gc_kth->short_idle = 0;
> + }
> +
> + return count;
> +}
> +
> +static ssize_t f2fs_gc_short_idle_store(struct f2fs_attr *a,
> + struct f2fs_sb_info *sbi,
> + const char *buf, size_t count)
> +{
> + struct f2fs_gc_kthread *gc_kth = sbi->gc_thread;
> + unsigned long t;
> + ssize_t ret;
> +
> + if (!gc_kth)
> + return -EFAULT;
> +
> + ret = kstrtoul(skip_spaces(buf), 0, &t);
> + if (ret < 0)
> + return ret;
> +
> + if (t == 0)
> + gc_kth->short_idle = 0;
> + else if (t == 1) {
> + gc_kth->short_idle = 1;
> + gc_kth->long_idle = 0;
> + }
> +
> + return count;
> +}
> +
> static ssize_t f2fs_attr_show(struct kobject *kobj,
> struct attribute *attr, char *buf)
> {
> @@ -142,21 +192,24 @@ static struct f2fs_attr f2fs_attr_##_name = { \
> .offset = offsetof(struct f2fs_gc_kthread, _elname), \
> }
>
> -#define F2FS_ATTR(name, mode, show, store) \
> -static struct f2fs_attr f2fs_attr_##name = __ATTR(name, mode, show, store)
> -
> #define F2FS_RW_ATTR(name, elname) \
> F2FS_ATTR_OFFSET(name, 0644, f2fs_sbi_show, f2fs_sbi_store, elname)
>
> F2FS_RW_ATTR(gc_min_sleep_time, min_sleep_time);
> F2FS_RW_ATTR(gc_max_sleep_time, max_sleep_time);
> F2FS_RW_ATTR(gc_no_gc_sleep_time, no_gc_sleep_time);
> +F2FS_ATTR_OFFSET(gc_long_idle, 0644, f2fs_sbi_show,
> + f2fs_gc_long_idle_store, long_idle);
> +F2FS_ATTR_OFFSET(gc_short_idle, 0644, f2fs_sbi_show,
> + f2fs_gc_short_idle_store, short_idle);
>
> #define ATTR_LIST(name) (&f2fs_attr_##name.attr)
> static struct attribute *f2fs_attrs[] = {
> ATTR_LIST(gc_min_sleep_time),
> ATTR_LIST(gc_max_sleep_time),
> ATTR_LIST(gc_no_gc_sleep_time),
> + ATTR_LIST(gc_long_idle),
> + ATTR_LIST(gc_short_idle),
> NULL,
> };
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists