linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: linux-ext4@vger.kernel.org, Jan Kara <jack@suse.cz>,
	Ojaswin Mujoo <ojaswin@linux.ibm.com>,
	Disha Goel <disgoel@linux.ibm.com>
Subject: Re: [RFCv1 3/4] ext4: Make mpage_journal_page_buffers use folio
Date: Mon, 17 Apr 2023 05:52:12 +0530	[thread overview]
Message-ID: <87zg77ici3.fsf@doe.com> (raw)
In-Reply-To: <ZDxQghSIxhX+YUME@casper.infradead.org>

Matthew Wilcox <willy@infradead.org> writes:

> On Mon, Apr 17, 2023 at 12:01:52AM +0530, Ritesh Harjani (IBM) wrote:
>> This patch converts mpage_journal_page_buffers() to use folio and also
>> removes the PAGE_SIZE assumption.
>
> Bit of an oversight on my part.  I neglected to do this after Jan added
> it.  Perils of parallel development ...
>

Yes, these got left overs because of the parallel series.

>> -static int ext4_journal_page_buffers(handle_t *handle, struct page *page,
>> -				     int len)
>> +static int ext4_journal_page_buffers(handle_t *handle, struct folio *folio,
>> +				     size_t len)
>
> Should this be called ext4_journal_folio_buffers?

Sure. Will make the change. Otherwise this patch looks good to you?
I also had a query regarding setting "len = size - folio_pos(folio)" in this patch.
Details of which I had pasted in the cover letter. Let me copy-paste
it here from the cover letter. Could you please take a look at it?


<copy-paste>
Also had a query w.r.t your change [1]. I couldn't understand this change diff
from [1]. Given if we are making the conversion to folio, then shouldn't we do
len = size - folio_pos(pos), instead of len = size & ~PAGE_MASK
Could you please tell if the current change in [1] is kept deliberately?
At other places you did make len as size - folio_pos(pos) which removes the
PAGE_SIZE assumption.

-static int mpage_submit_page(struct mpage_da_data *mpd, struct page *page)
+static int mpage_submit_folio(struct mpage_da_data *mpd, struct folio *folio)
 {
-	int len;
+	size_t len;

	<...>

	size = i_size_read(mpd->inode);
-	if (page->index == size >> PAGE_SHIFT &&
+	len = folio_size(folio);
+	if (folio_pos(folio) + len > size &&
 	    !ext4_verity_in_progress(mpd->inode))
 		len = size & ~PAGE_MASK;
-	else
-		len = PAGE_SIZE;
-	err = ext4_bio_write_page(&mpd->io_submit, page, len);
+	err = ext4_bio_write_page(&mpd->io_submit, &folio->page, len);
 	if (!err)
 		mpd->wbc->nr_to_write--;

[1]: https://lore.kernel.org/linux-ext4/20230324180129.1220691-7-willy@infradead.org/


Thanks for the quick review!

-ritesh

  reply	other threads:[~2023-04-17  0:22 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-16 18:31 [RFCv1 0/4] ext4: misc left over folio changes Ritesh Harjani (IBM)
2023-04-16 18:31 ` [RFCv1 1/4] ext4: kill unused function ext4_journalled_write_inline_data Ritesh Harjani (IBM)
2023-04-16 19:32   ` Matthew Wilcox
2023-04-16 18:31 ` [RFCv1 2/4] ext4: Change remaining tracepoints to use folio Ritesh Harjani (IBM)
2023-04-16 19:39   ` Matthew Wilcox
2023-04-16 18:31 ` [RFCv1 3/4] ext4: Make mpage_journal_page_buffers " Ritesh Harjani (IBM)
2023-04-16 19:46   ` Matthew Wilcox
2023-04-17  0:22     ` Ritesh Harjani [this message]
2023-04-17  5:16       ` Ritesh Harjani
2023-04-16 18:31 ` [RFCv1 4/4] ext4: Make ext4_write_inline_data_end() " Ritesh Harjani (IBM)
2023-04-16 19:58   ` Matthew Wilcox
2023-05-13 21:55 ` [RFCv1 0/4] ext4: misc left over folio changes Theodore Ts'o
2023-05-14  4:22   ` Ritesh Harjani

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87zg77ici3.fsf@doe.com \
    --to=ritesh.list@gmail.com \
    --cc=disgoel@linux.ibm.com \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=ojaswin@linux.ibm.com \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).