linux-kernel - Re: [RFC v2 02/14] filemap: align the index to mapping_min

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 13 Feb 2024 22:13:55 +0100
From: "Pankaj Raghav (Samsung)" <kernel@...kajraghav.com>
To: Hannes Reinecke <hare@...e.de>
Cc: linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	mcgrof@...nel.org, gost.dev@...sung.com, akpm@...ux-foundation.org, 
	kbusch@...nel.org, djwong@...nel.org, chandan.babu@...cle.com, p.raghav@...sung.com, 
	linux-kernel@...r.kernel.org, willy@...radead.org, linux-mm@...ck.org, david@...morbit.com
Subject: Re: [RFC v2 02/14] filemap: align the index to mapping_min_order in
 the page cache

> > diff --git a/mm/filemap.c b/mm/filemap.c
> > index 750e779c23db..323a8e169581 100644
> > --- a/mm/filemap.c
> > +++ b/mm/filemap.c
> > @@ -2479,14 +2479,16 @@ static int filemap_get_pages(struct kiocb *iocb, size_t count,
> >   {
> >   	struct file *filp = iocb->ki_filp;
> >   	struct address_space *mapping = filp->f_mapping;
> > +	unsigned int min_nrpages = mapping_min_folio_nrpages(mapping);
> >   	struct file_ra_state *ra = &filp->f_ra;
> > -	pgoff_t index = iocb->ki_pos >> PAGE_SHIFT;
> > +	pgoff_t index = round_down(iocb->ki_pos >> PAGE_SHIFT, min_nrpages);
> >   	pgoff_t last_index;
> >   	struct folio *folio;
> >   	int err = 0;
> >   	/* "last_index" is the index of the page beyond the end of the read */
> >   	last_index = DIV_ROUND_UP(iocb->ki_pos + count, PAGE_SIZE);
> > +	last_index = round_up(last_index, min_nrpages);
> 
> Huh? 'last_index' is unset here; sure you mean 'iocb->ki_pos + count' ?
> 
> And what's the rationale to replace 'DIV_ROUND_UP' with 'round_up' ?

Actually we are not replacing DIV_ROUND_UP, we are just adding another
round_up operation to min_nrpages pages after we get the last_index by
converting the bytes to page count. The main idea is to align the
last_index to min_order, if it is set. AFAIK, there is no one helper
that can do both lines in one shot.

> 
> >   retry:
> >   	if (fatal_signal_pending(current))
> >   		return -EINTR;