Received-SPF: pass (google.com: domain of linux-kernel+bounces-8282-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Thu, 21 Dec 2023 12:26:44 +0100
From: Jan Kara <jack@suse.cz>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-mm@kvack.org, "Matthew Wilcox (Oracle)" <willy@infradead.org>,
	Jan Kara <jack@suse.com>, David Howells <dhowells@redhat.com>,
	Brian Foster <bfoster@redhat.com>, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 10/17] pagevec: Add ability to iterate a queue
Message-ID: <20231221112644.2ijv6c5zooclcqtm@quack3>
References: <20231218153553.807799-1-hch@lst.de>
 <20231218153553.807799-11-hch@lst.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231218153553.807799-11-hch@lst.de>

On Mon 18-12-23 16:35:46, Christoph Hellwig wrote:
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
> 
> Add a loop counter inside the folio_batch to let us iterate from 0-nr
> instead of decrementing nr and treating the batch as a stack.  It would
> generate some very weird and suboptimal I/O patterns for page writeback
> to iterate over the batch as a stack.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  include/linux/pagevec.h | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/include/linux/pagevec.h b/include/linux/pagevec.h
> index 87cc678adc850b..fcc06c300a72c3 100644
> --- a/include/linux/pagevec.h
> +++ b/include/linux/pagevec.h
> @@ -27,6 +27,7 @@ struct folio;
>   */
>  struct folio_batch {
>  	unsigned char nr;
> +	unsigned char i;
>  	bool percpu_pvec_drained;
>  	struct folio *folios[PAGEVEC_SIZE];
>  };
> @@ -40,12 +41,14 @@ struct folio_batch {
>  static inline void folio_batch_init(struct folio_batch *fbatch)
>  {
>  	fbatch->nr = 0;
> +	fbatch->i = 0;
>  	fbatch->percpu_pvec_drained = false;
>  }
>  
>  static inline void folio_batch_reinit(struct folio_batch *fbatch)
>  {
>  	fbatch->nr = 0;
> +	fbatch->i = 0;
>  }
>  
>  static inline unsigned int folio_batch_count(struct folio_batch *fbatch)
> @@ -75,6 +78,21 @@ static inline unsigned folio_batch_add(struct folio_batch *fbatch,
>  	return folio_batch_space(fbatch);
>  }
>  
> +/**
> + * folio_batch_next - Return the next folio to process.
> + * @fbatch: The folio batch being processed.
> + *
> + * Use this function to implement a queue of folios.
> + *
> + * Return: The next folio in the queue, or NULL if the queue is empty.
> + */
> +static inline struct folio *folio_batch_next(struct folio_batch *fbatch)
> +{
> +	if (fbatch->i == fbatch->nr)
> +		return NULL;
> +	return fbatch->folios[fbatch->i++];
> +}
> +
>  void __folio_batch_release(struct folio_batch *pvec);
>  
>  static inline void folio_batch_release(struct folio_batch *fbatch)
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR