* [PATCH 1/2] ubifs: drop softlimit and delta fields from struct ubifs_wbuf
@ 2016-09-14 10:24 Rafał Miłecki
2016-09-14 10:24 ` [PATCH 2/2] ubifs: use dirty_writeback_interval value for wbuf timer Rafał Miłecki
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Rafał Miłecki @ 2016-09-14 10:24 UTC (permalink / raw)
To: Richard Weinberger, Artem Bityutskiy, Adrian Hunter
Cc: linux-mtd, Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
Values of these fields are set during init and never modified. They are
used (read) in a single function only. There isn't really any reason to
keep them in a struct. It only makes struct just a bit bigger without
any clear gain.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
fs/ubifs/io.c | 18 ++++++++++--------
fs/ubifs/ubifs.h | 5 -----
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index 97be412..4d6ce4a 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -452,16 +452,22 @@ static enum hrtimer_restart wbuf_timer_callback_nolock(struct hrtimer *timer)
*/
static void new_wbuf_timer_nolock(struct ubifs_wbuf *wbuf)
{
+ ktime_t softlimit = ktime_set(WBUF_TIMEOUT_SOFTLIMIT, 0);
+ unsigned long long delta;
+
+ delta = WBUF_TIMEOUT_HARDLIMIT - WBUF_TIMEOUT_SOFTLIMIT;
+ delta *= 1000000000ULL;
+
ubifs_assert(!hrtimer_active(&wbuf->timer));
+ ubifs_assert(delta <= ULONG_MAX);
if (wbuf->no_timer)
return;
dbg_io("set timer for jhead %s, %llu-%llu millisecs",
dbg_jhead(wbuf->jhead),
- div_u64(ktime_to_ns(wbuf->softlimit), USEC_PER_SEC),
- div_u64(ktime_to_ns(wbuf->softlimit) + wbuf->delta,
- USEC_PER_SEC));
- hrtimer_start_range_ns(&wbuf->timer, wbuf->softlimit, wbuf->delta,
+ div_u64(ktime_to_ns(softlimit), USEC_PER_SEC),
+ div_u64(ktime_to_ns(softlimit) + delta, USEC_PER_SEC));
+ hrtimer_start_range_ns(&wbuf->timer, softlimit, delta,
HRTIMER_MODE_REL);
}
@@ -1059,10 +1065,6 @@ int ubifs_wbuf_init(struct ubifs_info *c, struct ubifs_wbuf *wbuf)
hrtimer_init(&wbuf->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
wbuf->timer.function = wbuf_timer_callback_nolock;
- wbuf->softlimit = ktime_set(WBUF_TIMEOUT_SOFTLIMIT, 0);
- wbuf->delta = WBUF_TIMEOUT_HARDLIMIT - WBUF_TIMEOUT_SOFTLIMIT;
- wbuf->delta *= 1000000000ULL;
- ubifs_assert(wbuf->delta <= ULONG_MAX);
return 0;
}
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 4617d45..11bc8fa 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -644,9 +644,6 @@ typedef int (*ubifs_lpt_scan_callback)(struct ubifs_info *c,
* @io_mutex: serializes write-buffer I/O
* @lock: serializes @buf, @lnum, @offs, @avail, @used, @next_ino and @inodes
* fields
- * @softlimit: soft write-buffer timeout interval
- * @delta: hard and soft timeouts delta (the timer expire interval is @softlimit
- * and @softlimit + @delta)
* @timer: write-buffer timer
* @no_timer: non-zero if this write-buffer does not have a timer
* @need_sync: non-zero if the timer expired and the wbuf needs sync'ing
@@ -675,8 +672,6 @@ struct ubifs_wbuf {
int (*sync_callback)(struct ubifs_info *c, int lnum, int free, int pad);
struct mutex io_mutex;
spinlock_t lock;
- ktime_t softlimit;
- unsigned long long delta;
struct hrtimer timer;
unsigned int no_timer:1;
unsigned int need_sync:1;
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] ubifs: use dirty_writeback_interval value for wbuf timer
2016-09-14 10:24 [PATCH 1/2] ubifs: drop softlimit and delta fields from struct ubifs_wbuf Rafał Miłecki
@ 2016-09-14 10:24 ` Rafał Miłecki
2016-09-14 15:48 ` Boris Brezillon
2016-09-14 15:24 ` [PATCH 1/2] ubifs: drop softlimit and delta fields from struct ubifs_wbuf Boris Brezillon
2016-09-14 15:40 ` Boris Brezillon
2 siblings, 1 reply; 6+ messages in thread
From: Rafał Miłecki @ 2016-09-14 10:24 UTC (permalink / raw)
To: Richard Weinberger, Artem Bityutskiy, Adrian Hunter
Cc: linux-mtd, Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
Right now wbuf timer has hardcoded timeouts and there is no place for
manual adjustments. Some projects / cases many need that though. Few
file systems allow doing that by respecting dirty_writeback_interval
that can be set using sysctl (dirty_writeback_centisecs).
Lowering dirty_writeback_interval could be some way of dealing with user
space apps lacking proper fsyncs. This is definitely *not* a perfect
solution but we don't have ideal (user space) world. There were already
advanced discussions on this matter, mostly when ext4 was introduced and
it wasn't behaving as ext3. Anyway, the final decision was to add some
hacks to the ext4, as trying to fix whole user space or adding new API
was pointless.
We can't (and shouldn't?) just follow ext4. We can't e.g. sync on close
as this would cause too many commits and flash wearing. On the other
hand we still should allow some trade-off between -o sync and default
wbuf timeout. Respecting dirty_writeback_interval should allow some sane
cutomizations if used warily.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
fs/ubifs/io.c | 7 ++-----
fs/ubifs/ubifs.h | 4 ----
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index 4d6ce4a..aced6ee 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -452,11 +452,8 @@ static enum hrtimer_restart wbuf_timer_callback_nolock(struct hrtimer *timer)
*/
static void new_wbuf_timer_nolock(struct ubifs_wbuf *wbuf)
{
- ktime_t softlimit = ktime_set(WBUF_TIMEOUT_SOFTLIMIT, 0);
- unsigned long long delta;
-
- delta = WBUF_TIMEOUT_HARDLIMIT - WBUF_TIMEOUT_SOFTLIMIT;
- delta *= 1000000000ULL;
+ ktime_t softlimit = ms_to_ktime(dirty_writeback_interval * 10);
+ unsigned long long delta = dirty_writeback_interval; /* 10% delta */
ubifs_assert(!hrtimer_active(&wbuf->timer));
ubifs_assert(delta <= ULONG_MAX);
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 11bc8fa..26e6340 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -83,10 +83,6 @@
*/
#define BGT_NAME_PATTERN "ubifs_bgt%d_%d"
-/* Write-buffer synchronization timeout interval in seconds */
-#define WBUF_TIMEOUT_SOFTLIMIT 3
-#define WBUF_TIMEOUT_HARDLIMIT 5
-
/* Maximum possible inode number (only 32-bit inodes are supported now) */
#define MAX_INUM 0xFFFFFFFF
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] ubifs: drop softlimit and delta fields from struct ubifs_wbuf
2016-09-14 10:24 [PATCH 1/2] ubifs: drop softlimit and delta fields from struct ubifs_wbuf Rafał Miłecki
2016-09-14 10:24 ` [PATCH 2/2] ubifs: use dirty_writeback_interval value for wbuf timer Rafał Miłecki
@ 2016-09-14 15:24 ` Boris Brezillon
2016-09-14 15:28 ` Boris Brezillon
2016-09-14 15:40 ` Boris Brezillon
2 siblings, 1 reply; 6+ messages in thread
From: Boris Brezillon @ 2016-09-14 15:24 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Richard Weinberger, Artem Bityutskiy, Adrian Hunter,
Rafał Miłecki, linux-mtd
On Wed, 14 Sep 2016 12:24:34 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> Values of these fields are set during init and never modified. They are
> used (read) in a single function only. There isn't really any reason to
> keep them in a struct. It only makes struct just a bit bigger without
> any clear gain.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> fs/ubifs/io.c | 18 ++++++++++--------
> fs/ubifs/ubifs.h | 5 -----
> 2 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
> index 97be412..4d6ce4a 100644
> --- a/fs/ubifs/io.c
> +++ b/fs/ubifs/io.c
> @@ -452,16 +452,22 @@ static enum hrtimer_restart wbuf_timer_callback_nolock(struct hrtimer *timer)
> */
> static void new_wbuf_timer_nolock(struct ubifs_wbuf *wbuf)
> {
> + ktime_t softlimit = ktime_set(WBUF_TIMEOUT_SOFTLIMIT, 0);
> + unsigned long long delta;
> +
> + delta = WBUF_TIMEOUT_HARDLIMIT - WBUF_TIMEOUT_SOFTLIMIT;
> + delta *= 1000000000ULL;
> +
> ubifs_assert(!hrtimer_active(&wbuf->timer));
> + ubifs_assert(delta <= ULONG_MAX);
Hm, do we really want to recalculate delta each time we reschedule the
wbuf timer?
How about making the softlimit and delta variables static and
initializing them once?
>
> if (wbuf->no_timer)
> return;
> dbg_io("set timer for jhead %s, %llu-%llu millisecs",
> dbg_jhead(wbuf->jhead),
> - div_u64(ktime_to_ns(wbuf->softlimit), USEC_PER_SEC),
> - div_u64(ktime_to_ns(wbuf->softlimit) + wbuf->delta,
> - USEC_PER_SEC));
> - hrtimer_start_range_ns(&wbuf->timer, wbuf->softlimit, wbuf->delta,
> + div_u64(ktime_to_ns(softlimit), USEC_PER_SEC),
> + div_u64(ktime_to_ns(softlimit) + delta, USEC_PER_SEC));
> + hrtimer_start_range_ns(&wbuf->timer, softlimit, delta,
> HRTIMER_MODE_REL);
> }
>
> @@ -1059,10 +1065,6 @@ int ubifs_wbuf_init(struct ubifs_info *c, struct ubifs_wbuf *wbuf)
>
> hrtimer_init(&wbuf->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> wbuf->timer.function = wbuf_timer_callback_nolock;
> - wbuf->softlimit = ktime_set(WBUF_TIMEOUT_SOFTLIMIT, 0);
> - wbuf->delta = WBUF_TIMEOUT_HARDLIMIT - WBUF_TIMEOUT_SOFTLIMIT;
> - wbuf->delta *= 1000000000ULL;
> - ubifs_assert(wbuf->delta <= ULONG_MAX);
> return 0;
> }
>
> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> index 4617d45..11bc8fa 100644
> --- a/fs/ubifs/ubifs.h
> +++ b/fs/ubifs/ubifs.h
> @@ -644,9 +644,6 @@ typedef int (*ubifs_lpt_scan_callback)(struct ubifs_info *c,
> * @io_mutex: serializes write-buffer I/O
> * @lock: serializes @buf, @lnum, @offs, @avail, @used, @next_ino and @inodes
> * fields
> - * @softlimit: soft write-buffer timeout interval
> - * @delta: hard and soft timeouts delta (the timer expire interval is @softlimit
> - * and @softlimit + @delta)
> * @timer: write-buffer timer
> * @no_timer: non-zero if this write-buffer does not have a timer
> * @need_sync: non-zero if the timer expired and the wbuf needs sync'ing
> @@ -675,8 +672,6 @@ struct ubifs_wbuf {
> int (*sync_callback)(struct ubifs_info *c, int lnum, int free, int pad);
> struct mutex io_mutex;
> spinlock_t lock;
> - ktime_t softlimit;
> - unsigned long long delta;
> struct hrtimer timer;
> unsigned int no_timer:1;
> unsigned int need_sync:1;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] ubifs: drop softlimit and delta fields from struct ubifs_wbuf
2016-09-14 15:24 ` [PATCH 1/2] ubifs: drop softlimit and delta fields from struct ubifs_wbuf Boris Brezillon
@ 2016-09-14 15:28 ` Boris Brezillon
0 siblings, 0 replies; 6+ messages in thread
From: Boris Brezillon @ 2016-09-14 15:28 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Rafał Miłecki, Richard Weinberger, linux-mtd,
Adrian Hunter, Artem Bityutskiy
On Wed, 14 Sep 2016 17:24:28 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> On Wed, 14 Sep 2016 12:24:34 +0200
> Rafał Miłecki <zajec5@gmail.com> wrote:
>
> > From: Rafał Miłecki <rafal@milecki.pl>
> >
> > Values of these fields are set during init and never modified. They are
> > used (read) in a single function only. There isn't really any reason to
> > keep them in a struct. It only makes struct just a bit bigger without
> > any clear gain.
> >
> > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> > ---
> > fs/ubifs/io.c | 18 ++++++++++--------
> > fs/ubifs/ubifs.h | 5 -----
> > 2 files changed, 10 insertions(+), 13 deletions(-)
> >
> > diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
> > index 97be412..4d6ce4a 100644
> > --- a/fs/ubifs/io.c
> > +++ b/fs/ubifs/io.c
> > @@ -452,16 +452,22 @@ static enum hrtimer_restart wbuf_timer_callback_nolock(struct hrtimer *timer)
> > */
> > static void new_wbuf_timer_nolock(struct ubifs_wbuf *wbuf)
> > {
> > + ktime_t softlimit = ktime_set(WBUF_TIMEOUT_SOFTLIMIT, 0);
> > + unsigned long long delta;
> > +
> > + delta = WBUF_TIMEOUT_HARDLIMIT - WBUF_TIMEOUT_SOFTLIMIT;
> > + delta *= 1000000000ULL;
> > +
> > ubifs_assert(!hrtimer_active(&wbuf->timer));
> > + ubifs_assert(delta <= ULONG_MAX);
>
> Hm, do we really want to recalculate delta each time we reschedule the
> wbuf timer?
>
> How about making the softlimit and delta variables static and
> initializing them once?
Nevermind. I just read patch 2, and these delta and softlimit values
have to be recalculated to account for dirty_writeback_interval changes.
>
> >
> > if (wbuf->no_timer)
> > return;
> > dbg_io("set timer for jhead %s, %llu-%llu millisecs",
> > dbg_jhead(wbuf->jhead),
> > - div_u64(ktime_to_ns(wbuf->softlimit), USEC_PER_SEC),
> > - div_u64(ktime_to_ns(wbuf->softlimit) + wbuf->delta,
> > - USEC_PER_SEC));
> > - hrtimer_start_range_ns(&wbuf->timer, wbuf->softlimit, wbuf->delta,
> > + div_u64(ktime_to_ns(softlimit), USEC_PER_SEC),
> > + div_u64(ktime_to_ns(softlimit) + delta, USEC_PER_SEC));
> > + hrtimer_start_range_ns(&wbuf->timer, softlimit, delta,
> > HRTIMER_MODE_REL);
> > }
> >
> > @@ -1059,10 +1065,6 @@ int ubifs_wbuf_init(struct ubifs_info *c, struct ubifs_wbuf *wbuf)
> >
> > hrtimer_init(&wbuf->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> > wbuf->timer.function = wbuf_timer_callback_nolock;
> > - wbuf->softlimit = ktime_set(WBUF_TIMEOUT_SOFTLIMIT, 0);
> > - wbuf->delta = WBUF_TIMEOUT_HARDLIMIT - WBUF_TIMEOUT_SOFTLIMIT;
> > - wbuf->delta *= 1000000000ULL;
> > - ubifs_assert(wbuf->delta <= ULONG_MAX);
> > return 0;
> > }
> >
> > diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> > index 4617d45..11bc8fa 100644
> > --- a/fs/ubifs/ubifs.h
> > +++ b/fs/ubifs/ubifs.h
> > @@ -644,9 +644,6 @@ typedef int (*ubifs_lpt_scan_callback)(struct ubifs_info *c,
> > * @io_mutex: serializes write-buffer I/O
> > * @lock: serializes @buf, @lnum, @offs, @avail, @used, @next_ino and @inodes
> > * fields
> > - * @softlimit: soft write-buffer timeout interval
> > - * @delta: hard and soft timeouts delta (the timer expire interval is @softlimit
> > - * and @softlimit + @delta)
> > * @timer: write-buffer timer
> > * @no_timer: non-zero if this write-buffer does not have a timer
> > * @need_sync: non-zero if the timer expired and the wbuf needs sync'ing
> > @@ -675,8 +672,6 @@ struct ubifs_wbuf {
> > int (*sync_callback)(struct ubifs_info *c, int lnum, int free, int pad);
> > struct mutex io_mutex;
> > spinlock_t lock;
> > - ktime_t softlimit;
> > - unsigned long long delta;
> > struct hrtimer timer;
> > unsigned int no_timer:1;
> > unsigned int need_sync:1;
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] ubifs: drop softlimit and delta fields from struct ubifs_wbuf
2016-09-14 10:24 [PATCH 1/2] ubifs: drop softlimit and delta fields from struct ubifs_wbuf Rafał Miłecki
2016-09-14 10:24 ` [PATCH 2/2] ubifs: use dirty_writeback_interval value for wbuf timer Rafał Miłecki
2016-09-14 15:24 ` [PATCH 1/2] ubifs: drop softlimit and delta fields from struct ubifs_wbuf Boris Brezillon
@ 2016-09-14 15:40 ` Boris Brezillon
2 siblings, 0 replies; 6+ messages in thread
From: Boris Brezillon @ 2016-09-14 15:40 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Richard Weinberger, Artem Bityutskiy, Adrian Hunter,
Rafał Miłecki, linux-mtd
On Wed, 14 Sep 2016 12:24:34 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> Values of these fields are set during init and never modified. They are
> used (read) in a single function only. There isn't really any reason to
> keep them in a struct. It only makes struct just a bit bigger without
> any clear gain.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> fs/ubifs/io.c | 18 ++++++++++--------
> fs/ubifs/ubifs.h | 5 -----
> 2 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
> index 97be412..4d6ce4a 100644
> --- a/fs/ubifs/io.c
> +++ b/fs/ubifs/io.c
> @@ -452,16 +452,22 @@ static enum hrtimer_restart wbuf_timer_callback_nolock(struct hrtimer *timer)
> */
> static void new_wbuf_timer_nolock(struct ubifs_wbuf *wbuf)
> {
> + ktime_t softlimit = ktime_set(WBUF_TIMEOUT_SOFTLIMIT, 0);
> + unsigned long long delta;
> +
> + delta = WBUF_TIMEOUT_HARDLIMIT - WBUF_TIMEOUT_SOFTLIMIT;
> + delta *= 1000000000ULL;
> +
> ubifs_assert(!hrtimer_active(&wbuf->timer));
> + ubifs_assert(delta <= ULONG_MAX);
>
> if (wbuf->no_timer)
> return;
> dbg_io("set timer for jhead %s, %llu-%llu millisecs",
> dbg_jhead(wbuf->jhead),
> - div_u64(ktime_to_ns(wbuf->softlimit), USEC_PER_SEC),
> - div_u64(ktime_to_ns(wbuf->softlimit) + wbuf->delta,
> - USEC_PER_SEC));
> - hrtimer_start_range_ns(&wbuf->timer, wbuf->softlimit, wbuf->delta,
> + div_u64(ktime_to_ns(softlimit), USEC_PER_SEC),
> + div_u64(ktime_to_ns(softlimit) + delta, USEC_PER_SEC));
> + hrtimer_start_range_ns(&wbuf->timer, softlimit, delta,
> HRTIMER_MODE_REL);
> }
>
> @@ -1059,10 +1065,6 @@ int ubifs_wbuf_init(struct ubifs_info *c, struct ubifs_wbuf *wbuf)
>
> hrtimer_init(&wbuf->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> wbuf->timer.function = wbuf_timer_callback_nolock;
> - wbuf->softlimit = ktime_set(WBUF_TIMEOUT_SOFTLIMIT, 0);
> - wbuf->delta = WBUF_TIMEOUT_HARDLIMIT - WBUF_TIMEOUT_SOFTLIMIT;
> - wbuf->delta *= 1000000000ULL;
> - ubifs_assert(wbuf->delta <= ULONG_MAX);
> return 0;
> }
>
> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> index 4617d45..11bc8fa 100644
> --- a/fs/ubifs/ubifs.h
> +++ b/fs/ubifs/ubifs.h
> @@ -644,9 +644,6 @@ typedef int (*ubifs_lpt_scan_callback)(struct ubifs_info *c,
> * @io_mutex: serializes write-buffer I/O
> * @lock: serializes @buf, @lnum, @offs, @avail, @used, @next_ino and @inodes
> * fields
> - * @softlimit: soft write-buffer timeout interval
> - * @delta: hard and soft timeouts delta (the timer expire interval is @softlimit
> - * and @softlimit + @delta)
> * @timer: write-buffer timer
> * @no_timer: non-zero if this write-buffer does not have a timer
> * @need_sync: non-zero if the timer expired and the wbuf needs sync'ing
> @@ -675,8 +672,6 @@ struct ubifs_wbuf {
> int (*sync_callback)(struct ubifs_info *c, int lnum, int free, int pad);
> struct mutex io_mutex;
> spinlock_t lock;
> - ktime_t softlimit;
> - unsigned long long delta;
> struct hrtimer timer;
> unsigned int no_timer:1;
> unsigned int need_sync:1;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] ubifs: use dirty_writeback_interval value for wbuf timer
2016-09-14 10:24 ` [PATCH 2/2] ubifs: use dirty_writeback_interval value for wbuf timer Rafał Miłecki
@ 2016-09-14 15:48 ` Boris Brezillon
0 siblings, 0 replies; 6+ messages in thread
From: Boris Brezillon @ 2016-09-14 15:48 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Richard Weinberger, Artem Bityutskiy, Adrian Hunter,
Rafał Miłecki, linux-mtd
On Wed, 14 Sep 2016 12:24:35 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> Right now wbuf timer has hardcoded timeouts and there is no place for
> manual adjustments. Some projects / cases many need that though. Few
> file systems allow doing that by respecting dirty_writeback_interval
> that can be set using sysctl (dirty_writeback_centisecs).
>
> Lowering dirty_writeback_interval could be some way of dealing with user
> space apps lacking proper fsyncs. This is definitely *not* a perfect
> solution but we don't have ideal (user space) world. There were already
> advanced discussions on this matter, mostly when ext4 was introduced and
> it wasn't behaving as ext3. Anyway, the final decision was to add some
> hacks to the ext4, as trying to fix whole user space or adding new API
> was pointless.
>
> We can't (and shouldn't?) just follow ext4. We can't e.g. sync on close
> as this would cause too many commits and flash wearing. On the other
> hand we still should allow some trade-off between -o sync and default
> wbuf timeout. Respecting dirty_writeback_interval should allow some sane
> cutomizations if used warily.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> fs/ubifs/io.c | 7 ++-----
> fs/ubifs/ubifs.h | 4 ----
> 2 files changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
> index 4d6ce4a..aced6ee 100644
> --- a/fs/ubifs/io.c
> +++ b/fs/ubifs/io.c
> @@ -452,11 +452,8 @@ static enum hrtimer_restart wbuf_timer_callback_nolock(struct hrtimer *timer)
> */
> static void new_wbuf_timer_nolock(struct ubifs_wbuf *wbuf)
> {
> - ktime_t softlimit = ktime_set(WBUF_TIMEOUT_SOFTLIMIT, 0);
> - unsigned long long delta;
> -
> - delta = WBUF_TIMEOUT_HARDLIMIT - WBUF_TIMEOUT_SOFTLIMIT;
> - delta *= 1000000000ULL;
> + ktime_t softlimit = ms_to_ktime(dirty_writeback_interval * 10);
> + unsigned long long delta = dirty_writeback_interval; /* 10% delta */
dirty_writeback_interval is expressed in centiseconds, and delta is in
nanoseconds.
I think you miss
delta *= 10000000ULL;
>
> ubifs_assert(!hrtimer_active(&wbuf->timer));
> ubifs_assert(delta <= ULONG_MAX);
> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> index 11bc8fa..26e6340 100644
> --- a/fs/ubifs/ubifs.h
> +++ b/fs/ubifs/ubifs.h
> @@ -83,10 +83,6 @@
> */
> #define BGT_NAME_PATTERN "ubifs_bgt%d_%d"
>
> -/* Write-buffer synchronization timeout interval in seconds */
> -#define WBUF_TIMEOUT_SOFTLIMIT 3
> -#define WBUF_TIMEOUT_HARDLIMIT 5
> -
> /* Maximum possible inode number (only 32-bit inodes are supported now) */
> #define MAX_INUM 0xFFFFFFFF
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-14 15:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-14 10:24 [PATCH 1/2] ubifs: drop softlimit and delta fields from struct ubifs_wbuf Rafał Miłecki
2016-09-14 10:24 ` [PATCH 2/2] ubifs: use dirty_writeback_interval value for wbuf timer Rafał Miłecki
2016-09-14 15:48 ` Boris Brezillon
2016-09-14 15:24 ` [PATCH 1/2] ubifs: drop softlimit and delta fields from struct ubifs_wbuf Boris Brezillon
2016-09-14 15:28 ` Boris Brezillon
2016-09-14 15:40 ` Boris Brezillon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.