[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Fri, 10 Oct 2008 12:34:56 -0400
From: Christoph Hellwig <hch@...radead.org>
To: "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
Cc: Christoph Hellwig <hch@...radead.org>,
Theodore Tso <tytso@....edu>,
Steven Whitehouse <steve@...gwyn.com>, npiggin@...e.de,
Andrew Morton <akpm@...ux-foundation.org>,
Mikulas Patocka <mpatocka@...hat.com>, linux-mm@...ck.org,
linux-fsdevel@...r.kernel.org, linux-ext4@...r.kernel.org
Subject: Re: [patch 4/8] mm: write_cache_pages type overflow fix
On Fri, Oct 10, 2008 at 09:24:47PM +0530, Aneesh Kumar K.V wrote:
> diff --git a/include/linux/writeback.h b/include/linux/writeback.h
> index bd91987..7599af2 100644
> --- a/include/linux/writeback.h
> +++ b/include/linux/writeback.h
> @@ -63,6 +63,8 @@ struct writeback_control {
> unsigned for_writepages:1; /* This is a writepages() call */
> unsigned range_cyclic:1; /* range_start is cyclic */
> unsigned more_io:1; /* more io to be dispatched */
> + /* flags which control the write_cache_pages behaviour */
> + int writeback_flags;
As Ted already said please follow the bitfields style already used.
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -876,11 +876,18 @@ int write_cache_pages(struct address_space *mapping,
> pgoff_t end; /* Inclusive */
> int scanned = 0;
> int range_whole = 0;
> + int flags = wbc->writeback_flags;
> + long *nr_to_write, count;
>
> if (wbc->nonblocking && bdi_write_congested(bdi)) {
> wbc->encountered_congestion = 1;
> return 0;
> }
> + if (flags & WB_NO_NRWRITE_UPDATE) {
> + count = wbc->nr_to_write;
> + nr_to_write = &count;
> + } else
> + nr_to_write = &wbc->nr_to_write;
I think we'd be better off always using a local variable and updating
wbc->nr_to_write again before the exit for the !WB_NO_NRWRITE_UPDATE
case.
> - if (wbc->range_cyclic || (range_whole && wbc->nr_to_write > 0))
> + if ((wbc->range_cyclic ||
> + (range_whole && wbc->nr_to_write > 0)) &&
> + (flags & ~WB_NO_INDEX_UPDATE)) {
> mapping->writeback_index = index;
The conditional looks rather odd, what about:
if (!wbc->no_index_update &&
(wbc->range_cyclic || (range_whole && wbc->nr_to_write > 0))
Also I wonder what this is for. Do you want what Chris did in his
original patch in ext4 code, or is there another reason to not update
the writeback_index sometimes?
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists