Re: [PATCH] Writeback page range hint

Re: [PATCH] Writeback page range hint

[Dave Jones]

On Mon, Aug 23, 2004 at 07:59:27PM +0000, Linux Kernel wrote:
 > ChangeSet 1.2034, 2004/08/23 12:59:27-07:00, akpm@osdl.org
 > 
 > 	[PATCH] Writeback page range hint
 > 	
 > 	Modify mpage_writepages to optionally only write back dirty pages within a
 > 	specified range in a file (as in the case of O_SYNC).  Cheat a little to avoid
 > 	changes to prototypes of aops - just put the <start, end> hint into the
 > 	writeback_control struct instead.  If <start, end> are not set, then default
 > 	to writing back all the mapping's dirty pages.

FYI, this chunk...

 >  	long pages_skipped;		/* Pages which were not written */
 > -	int nonblocking;		/* Don't get stuck on request queues */
 > -	int encountered_congestion;	/* An output: a queue is full */
 > -	int for_kupdate;		/* A kupdate writeback */
 > -	int for_reclaim;		/* Invoked from the page allocator */
 > +
 > +	/*
 > +	 * For a_ops->writepages(): is start or end are non-zero then this is
 > +	 * a hint that the filesystem need only write out the pages inside that
 > +	 * byterange.  The byte at `end' is included in the writeout request.
 > +	 */
 > +	loff_t start;
 > +	loff_t end;
 > +
 > +	int nonblocking:1;		/* Don't get stuck on request queues */
 > +	int encountered_congestion:1;	/* An output: a queue is full */
 > +	int for_kupdate:1;		/* A kupdate writeback */
 > +	int for_reclaim:1;		/* Invoked from the page allocator */

Causes sparse spew..

include/linux/writeback.h:54:19: warning: dubious one-bit signed bitfield
include/linux/writeback.h:55:30: warning: dubious one-bit signed bitfield
include/linux/writeback.h:56:19: warning: dubious one-bit signed bitfield
include/linux/writeback.h:57:19: warning: dubious one-bit signed bitfield

		Dave

Re: [PATCH] Writeback page range hint

[Andrew Morton]

Dave Jones <davej@redhat.com> wrote:
>
> FYI, this chunk...
> 
>   >  	long pages_skipped;		/* Pages which were not written */
>   > -	int nonblocking;		/* Don't get stuck on request queues */
>   > -	int encountered_congestion;	/* An output: a queue is full */
>   > -	int for_kupdate;		/* A kupdate writeback */
>   > -	int for_reclaim;		/* Invoked from the page allocator */
>   > +
>   > +	/*
>   > +	 * For a_ops->writepages(): is start or end are non-zero then this is
>   > +	 * a hint that the filesystem need only write out the pages inside that
>   > +	 * byterange.  The byte at `end' is included in the writeout request.
>   > +	 */
>   > +	loff_t start;
>   > +	loff_t end;
>   > +
>   > +	int nonblocking:1;		/* Don't get stuck on request queues */
>   > +	int encountered_congestion:1;	/* An output: a queue is full */
>   > +	int for_kupdate:1;		/* A kupdate writeback */
>   > +	int for_reclaim:1;		/* Invoked from the page allocator */
> 
>  Causes sparse spew..
> 
>  include/linux/writeback.h:54:19: warning: dubious one-bit signed bitfield
>  include/linux/writeback.h:55:30: warning: dubious one-bit signed bitfield
>  include/linux/writeback.h:56:19: warning: dubious one-bit signed bitfield
>  include/linux/writeback.h:57:19: warning: dubious one-bit signed bitfield

That's fussy of it.  I assume this shuts it up?

--- 25/include/linux/writeback.h~writeback-sparse-warning-fix	2004-08-23 18:12:09.669385176 -0700
+++ 25-akpm/include/linux/writeback.h	2004-08-23 18:12:44.064156376 -0700
@@ -51,10 +51,10 @@ struct writeback_control {
 	loff_t start;
 	loff_t end;
 
-	int nonblocking:1;		/* Don't get stuck on request queues */
-	int encountered_congestion:1;	/* An output: a queue is full */
-	int for_kupdate:1;		/* A kupdate writeback */
-	int for_reclaim:1;		/* Invoked from the page allocator */
+	unsigned int nonblocking:1;	/* Don't get stuck on request queues */
+	unsigned int encountered_congestion:1;	/* An output: a queue is full */
+	unsigned int for_kupdate:1;	/* A kupdate writeback */
+	unsigned int for_reclaim:1;	/* Invoked from the page allocator */
 };
 
 /*
_

Re: [PATCH] Writeback page range hint

[Dave Jones]

On Mon, Aug 23, 2004 at 06:14:00PM -0700, Andrew Morton wrote:

 > >   > +	int nonblocking:1;		/* Don't get stuck on request queues */
 > >   > +	int encountered_congestion:1;	/* An output: a queue is full */
 > >   > +	int for_kupdate:1;		/* A kupdate writeback */
 > >   > +	int for_reclaim:1;		/* Invoked from the page allocator */
 > > 
 > >  Causes sparse spew..
 > > 
 > >  include/linux/writeback.h:54:19: warning: dubious one-bit signed bitfield
 > >  include/linux/writeback.h:55:30: warning: dubious one-bit signed bitfield
 > >  include/linux/writeback.h:56:19: warning: dubious one-bit signed bitfield
 > >  include/linux/writeback.h:57:19: warning: dubious one-bit signed bitfield
 > 
 > That's fussy of it.  I assume this shuts it up?

very likely (its 2am, and I'm feeling too lazy to check).
though, is there any real reason why they are bitfields at all ?

I'll double check it in the morning when the nightly sparse cron jobs mail me 8-)

		Dave

Re: [PATCH] Writeback page range hint

[Andrew Morton]

Dave Jones <davej@redhat.com> wrote:
>
> On Mon, Aug 23, 2004 at 06:14:00PM -0700, Andrew Morton wrote:
> 
>  > >   > +	int nonblocking:1;		/* Don't get stuck on request queues */
>  > >   > +	int encountered_congestion:1;	/* An output: a queue is full */
>  > >   > +	int for_kupdate:1;		/* A kupdate writeback */
>  > >   > +	int for_reclaim:1;		/* Invoked from the page allocator */
>  > > 
>  > >  Causes sparse spew..
>  > > 
>  > >  include/linux/writeback.h:54:19: warning: dubious one-bit signed bitfield
>  > >  include/linux/writeback.h:55:30: warning: dubious one-bit signed bitfield
>  > >  include/linux/writeback.h:56:19: warning: dubious one-bit signed bitfield
>  > >  include/linux/writeback.h:57:19: warning: dubious one-bit signed bitfield
>  > 
>  > That's fussy of it.  I assume this shuts it up?
> 
> very likely (its 2am, and I'm feeling too lazy to check).

That's the middle of the working day.

> though, is there any real reason why they are bitfields at all ?

That should have been in the changelog.  Bad Suparna.

These structures are allocated on the stack in the (deep) writeback paths. 
Saving a few bytes there does seem to be worth the increased code size.

Re: [PATCH] Writeback page range hint

[wli]

On Tue, Aug 24, 2004 at 02:07:23AM +0100, Dave Jones wrote:
> Causes sparse spew..
> include/linux/writeback.h:54:19: warning: dubious one-bit signed bitfield
> include/linux/writeback.h:55:30: warning: dubious one-bit signed bitfield
> include/linux/writeback.h:56:19: warning: dubious one-bit signed bitfield
> include/linux/writeback.h:57:19: warning: dubious one-bit signed bitfield

Does this help?


Index: mm4-2.6.8.1/include/linux/writeback.h
===================================================================
--- mm4-2.6.8.1.orig/include/linux/writeback.h	2004-08-23 16:11:10.318739856 -0700
+++ mm4-2.6.8.1/include/linux/writeback.h	2004-08-23 18:26:45.790960088 -0700
@@ -51,10 +51,10 @@
 	loff_t start;
 	loff_t end;
 
-	int nonblocking:1;		/* Don't get stuck on request queues */
-	int encountered_congestion:1;	/* An output: a queue is full */
-	int for_kupdate:1;		/* A kupdate writeback */
-	int for_reclaim:1;		/* Invoked from the page allocator */
+	unsigned nonblocking:1,		/* Don't get stuck on request queues */
+	    encountered_congestion:1,	/* An output: a queue is full */
+	    for_kupdate:1,		/* A kupdate writeback */
+	    for_reclaim:1;		/* Invoked from the page allocator */
 };
 
 /*