lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 1 Dec 2022 12:50:19 +0100 From: Jan Kara <jack@...e.cz> To: "Ritesh Harjani (IBM)" <ritesh.list@...il.com> Cc: Jan Kara <jack@...e.cz>, Ted Tso <tytso@....edu>, linux-ext4@...r.kernel.org, Christoph Hellwig <hch@...radead.org> Subject: Re: [PATCH 5/9] ext4: Add support for writepages calls that cannot map blocks On Thu 01-12-22 16:43:59, Ritesh Harjani (IBM) wrote: > On 22/11/30 05:35PM, Jan Kara wrote: > > Add support for calls to ext4_writepages() than cannot map blocks. These > > will be issued from jbd2 transaction commit code. > > I guess we should expand the description of mpage_prepare_extent_to_map() > function now. Other than that the patch looks good to me. > > Please feel free to add: > Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@...il.com> Thanks for review! > > /* > > * mpage_prepare_extent_to_map - find & lock contiguous range of dirty pages > > * and underlying extent to map > > Since we are overloading this function. this can be also called with can_map > as 0. Maybe good to add some description around that? Well, it was somewhat overloaded already before but you're right some documentation update is in order :) I'll do that. > > @@ -2651,14 +2665,30 @@ static int mpage_prepare_extent_to_map(struct mpage_da_data *mpd) > > > adding context of code so that it doesn't get missed in the discussion. > > <...> > /* If we can't merge this page, we are done. */ > if (mpd->map.m_len > 0 && mpd->next_page != page->index) > goto out; > > I guess this also will not hold for us given we will always have m_len to be 0. > <...> Correct. > > + /* > > + * Writeout for transaction commit where we cannot > > + * modify metadata is simple. Just submit the page. > > + */ > > + if (!mpd->can_map) { > > + if (ext4_page_nomap_can_writeout(page)) { > > + err = mpage_submit_page(mpd, page); > > + if (err < 0) > > + goto out; > > + } else { > > + unlock_page(page); > > + mpd->first_page++; > > We anyway should always have mpd->map.m_len = 0. > That means, we always set mpd->first_page = page->index above. > So this might not be useful. But I guess for consistency of the code, > or to avoid any future bugs, this isn't harmful to keep. Yes, it is mostly for consistency but it is also needed so that once we exit the loop, mpage_release_unused_pages() starts working from a correct page. Honza -- Jan Kara <jack@...e.com> SUSE Labs, CR
Powered by blists - more mailing lists