Re: [PATCH 2/8] readahead: state based method routines: explicitly embed class_new/class_old inside flags

Re: [PATCH 2/8] readahead: state based method routines: explicitly embed class_new/class_old inside flags

[Bodo Eggert]

Fengguang Wu <wfg@mail.ustc.edu.cn> wrote:

> The new/old ra class were implicitly stored in low bits of
> file_ra_state.flags. Now make the data structure obvious, and remove the
> coding tricks.

> +++ linux-2.6.20-rc4-mm1/include/linux/fs.h

> -     unsigned long flags;    /* RA_FLAG_xxx | ra_class_old | ra_class_new */
> +     union {
> +             unsigned long   flags;  /* RA_FLAG_xxx | class_old | class_new */
> +             struct {
> +                     u8      class_new;
> +                     u8      class_old;

This doesn't work on big endiam machines, does it?

IMO you should use a bitfield instead.
-- 
Your e-mail has been returned due to insufficient voltage. 

Friß, Spammer: aegzcH4Js@luevh-7.7eggert.dyndns.org
 ptulV7@C.7eggert.dyndns.org vkIfyB@SEcltba.7eggert.dyndns.org

Re: [PATCH 2/8] readahead: state based method routines: explicitly embed class_new/class_old inside flags

[Randy Dunlap]

On Sat, 27 Jan 2007 13:01:08 +0100 Bodo Eggert wrote:

> Fengguang Wu <wfg@mail.ustc.edu.cn> wrote:
> 
> > The new/old ra class were implicitly stored in low bits of
> > file_ra_state.flags. Now make the data structure obvious, and remove the
> > coding tricks.
> 
> > +++ linux-2.6.20-rc4-mm1/include/linux/fs.h
> 
> > -     unsigned long flags;    /* RA_FLAG_xxx | ra_class_old | ra_class_new */
> > +     union {
> > +             unsigned long   flags;  /* RA_FLAG_xxx | class_old | class_new */
> > +             struct {
> > +                     u8      class_new;
> > +                     u8      class_old;
> 
> This doesn't work on big endiam machines, does it?
> 
> IMO you should use a bitfield instead.

Or "use bit masks instead" since

"The order of allocation of bit-fields within a unit (high-order to
low-order or low-order to high-order) is implementation-defined."

---
~Randy

[PATCH 0/8] readahead updates

[Fengguang Wu]

Andrew,

Here are more updates for adaptive readahead.

It's not likely to have more big changes, so the next plan is to convert some
accounting items to the statistics infrastructure by Martin Peschke.
[Martin: Sorry for the long delay.]

I'm also playing with the current readahead code, trying to make it more clean
and reliable. I guess it would also be a long journey. It may be better to
wait and confirm if the adaptive readahead works in the wild, before touching
the stock readahead.

Regards,
Fengguang Wu
---

--- broken-out/series	2007-01-12 13:19:44.000000000 +0800
+++ patches/series	2007-01-27 14:53:27.000000000 +0800
@@ -1089,9 +1089,13 @@
 readahead-sysctl-parameters-set-readahead_hit_rate=1.patch
 readahead-min-max-sizes.patch
 readahead-min-max-sizes-remove-get_readahead_bounds.patch
+readahead-min-max-sizes-increase-VM_MIN_READAHEAD-to-32KB.patch
 readahead-state-based-method-aging-accounting.patch
 readahead-state-based-method-routines.patch
+readahead-state-based-method-routines-explicitly-embed-class_new-class_old-inside-flags.patch
 readahead-state-based-method.patch
+readahead-state-based-method-prevent-tiny-size.patch
+readahead-state-based-method-move-readahead_ratio-out-of-compute_thrashing_threshold.patch
 readahead-context-based-method.patch
 readahead-context-based-method-locking-fix.patch
 readahead-context-based-method-locking-fix-2.patch
@@ -1100,14 +1104,18 @@
 readahead-initial-method-guiding-sizes.patch
 readahead-initial-method-thrashing-guard-size.patch
 readahead-initial-method-user-recommended-size.patch
+readahead-initial-method-user-recommended-size-rename-to-read_ahead_initial_kb.patch
 readahead-initial-method.patch
 readahead-backward-prefetching-method.patch
 readahead-thrashing-recovery-method.patch
+readahead-thrashing-recovery-method-fix.patch
 readahead-call-scheme.patch
 readahead-call-scheme-ifdef-fix.patch
 readahead-call-scheme-build-fix.patch
 readahead-call-scheme-remove-get_readahead_bounds.patch
+readahead-call-scheme-fix-thrashed-unaligned-read.patch
 readahead-laptop-mode.patch
+readahead-laptop-mode-fix.patch
 readahead-loop-case.patch
 readahead-nfsd-case.patch
 readahead-nfsd-case-fix.patch

[PATCH 2/8] readahead: state based method routines: explicitly embed class_new/class_old inside flags

[Fengguang Wu]

[-- Attachment #1: readahead-state-based-method-routines-explicitly-embed-class_new-class_old-inside-flags.patch --]
[-- Type: text/plain, Size: 2155 bytes --]

The new/old ra class were implicitly stored in low bits of file_ra_state.flags.
Now make the data structure obvious, and remove the coding tricks.

Signed-off-by: Fengguang Wu <wfg@mail.ustc.edu.cn>
---
 include/linux/fs.h |    9 ++++++++-
 mm/readahead.c     |   18 ++++--------------
 2 files changed, 12 insertions(+), 15 deletions(-)

--- linux-2.6.20-rc4-mm1.orig/include/linux/fs.h
+++ linux-2.6.20-rc4-mm1/include/linux/fs.h
@@ -738,7 +738,14 @@ struct file_ra_state {
 	unsigned long mmap_hit;		/* Cache hit stat for mmap accesses */
 	unsigned long mmap_miss;	/* Cache miss stat for mmap accesses */
 
-	unsigned long flags;	/* RA_FLAG_xxx | ra_class_old | ra_class_new */
+	union {
+		unsigned long	flags;	/* RA_FLAG_xxx | class_old | class_new */
+		struct {
+			u8	class_new;
+			u8	class_old;
+		};
+	};
+
 	unsigned long prev_page;	/* Cache last read() position */
 	unsigned long ra_pages;		/* Maximum readahead window */
 };
--- linux-2.6.20-rc4-mm1.orig/mm/readahead.c
+++ linux-2.6.20-rc4-mm1/mm/readahead.c
@@ -48,8 +48,6 @@ int readahead_hit_rate = 1;
 /*
  * Detailed classification of read-ahead behaviors.
  */
-#define RA_CLASS_SHIFT 4
-#define RA_CLASS_MASK  ((1 << RA_CLASS_SHIFT) - 1)
 enum ra_class {
 	RA_CLASS_ALL,
 	RA_CLASS_INITIAL,
@@ -795,12 +793,12 @@ out:
 
 static enum ra_class ra_class_new(struct file_ra_state *ra)
 {
-	return ra->flags & RA_CLASS_MASK;
+	return ra->class_new;
 }
 
 static inline enum ra_class ra_class_old(struct file_ra_state *ra)
 {
-	return (ra->flags >> RA_CLASS_SHIFT) & RA_CLASS_MASK;
+	return ra->class_old;
 }
 
 static unsigned long ra_readahead_size(struct file_ra_state *ra)
@@ -832,16 +830,8 @@ static int ra_has_index(struct file_ra_s
  */
 static void ra_set_class(struct file_ra_state *ra, enum ra_class ra_class)
 {
-	unsigned long flags_mask;
-	unsigned long flags;
-	unsigned long old_ra_class;
-
-	flags_mask = ~(RA_CLASS_MASK | (RA_CLASS_MASK << RA_CLASS_SHIFT));
-	flags = ra->flags & flags_mask;
-
-	old_ra_class = ra_class_new(ra) << RA_CLASS_SHIFT;
-
-	ra->flags = flags | old_ra_class | ra_class;
+	ra->class_old = ra->class_new;
+	ra->class_new = ra_class;
 }
 
 /*

--