On Fri, Nov 10, 2023 at 3:37 PM Ryusuke Konishi wrote:
>
> On Fri, Nov 10, 2023 at 6:06 AM Matthew Wilcox (Oracle) wrote:
> >
> > We must not shift by a negative number so work in terms of a byte
> > offset to avoid the awkward shift left-or-right-depending-on-sign
> > option. This means we need to use check_mul_overflow() to ensure
> > that a large block number does not result in a wrap.
> >
> > Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> > ---
> > fs/buffer.c | 17 ++++++-----------
> > 1 file changed, 6 insertions(+), 11 deletions(-)
> >
> > diff --git a/fs/buffer.c b/fs/buffer.c
> > index 44e0c0b7f71f..9c3f49cf8d28 100644
> > --- a/fs/buffer.c
> > +++ b/fs/buffer.c
> > @@ -1085,26 +1085,21 @@ static bool grow_dev_folio(struct block_device *bdev, sector_t block,
> > static bool grow_buffers(struct block_device *bdev, sector_t block,
> > unsigned size, gfp_t gfp)
> > {
> > - pgoff_t index;
> > - int sizebits;
> > -
> > - sizebits = PAGE_SHIFT - __ffs(size);
> > - index = block >> sizebits;
> > + loff_t pos;
> >
> > /*
> > - * Check for a block which wants to lie outside our maximum possible
> > - * pagecache index. (this comparison is done using sector_t types).
> > + * Check for a block which lies outside our maximum possible
> > + * pagecache index.
> > */
> > - if (unlikely(index != block >> sizebits)) {
> > - printk(KERN_ERR "%s: requested out-of-range block %llu for "
> > - "device %pg\n",
> > + if (check_mul_overflow(block, size, &pos) || pos > MAX_LFS_FILESIZE) {
> > + printk(KERN_ERR "%s: requested out-of-range block %llu for device %pg\n",
> > __func__, (unsigned long long)block,
> > bdev);
> > return false;
> > }
> >
> > /* Create a folio with the proper size buffers */
> > - return grow_dev_folio(bdev, block, index, size, gfp);
>
> > + return grow_dev_folio(bdev, block, pos / PAGE_SIZE, size, gfp);
>
> "pos" has a loff_t type (= long long type).
> Was it okay to do C division directly on 32-bit architectures?
>
> Regards,
> Ryusuke Konishi
Similar to the comment for patch 5/7, can we safely use the generally
less expensive shift operation "pos >> PAGE_SHIFT" here ?
Regards,
Ryusuke Konishi
