Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Tue, 31 May 2022 21:56:37 +0100
From:   Matthew Wilcox <willy@infradead.org>
To:     Andrew Morton <akpm@linux-foundation.org>
Cc:     Hsin-Yi Wang <hsinyi@chromium.org>,
        Phillip Lougher <phillip@squashfs.org.uk>,
        Xiongwei Song <Xiongwei.Song@windriver.com>,
        Zheng Liang <zhengliang6@huawei.com>,
        Zhang Yi <yi.zhang@huawei.com>, Hou Tao <houtao1@huawei.com>,
        Miao Xie <miaoxie@huawei.com>,
        "linux-mm @ kvack . org" <linux-mm@kvack.org>,
        "squashfs-devel @ lists . sourceforge . net" 
        <squashfs-devel@lists.sourceforge.net>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 3/3] squashfs: implement readahead
Message-ID: <YpaBBXEEtv4ZjFMA@casper.infradead.org>
References: <20220523065909.883444-1-hsinyi@chromium.org>
 <20220523065909.883444-4-hsinyi@chromium.org>
 <20220531134740.91ae4dcea1e06640ba1bfc12@linux-foundation.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220531134740.91ae4dcea1e06640ba1bfc12@linux-foundation.org>
Precedence: bulk

On Tue, May 31, 2022 at 01:47:40PM -0700, Andrew Morton wrote:
> > +	for (;;) {
> > +		nr_pages = __readahead_batch(ractl, pages, max_pages);
> > +		if (!nr_pages)
> > +			break;
> > +
> > +		if (readahead_pos(ractl) >= i_size_read(inode) ||
> > +		    nr_pages < max_pages)
> > +			goto skip_pages;
> > +
> > +		index = pages[0]->index >> shift;
> > +		if ((pages[nr_pages - 1]->index >> shift) != index)
> > +			goto skip_pages;
> > +
> > +		expected = index == file_end ?
> > +			   (i_size_read(inode) & (msblk->block_size - 1)) :
> > +			    msblk->block_size;
> > +
> > +		bsize = read_blocklist(inode, index, &block);
> > +		if (bsize == 0)
> > +			goto skip_pages;
> > +
> > +		res = squashfs_read_data(inode->i_sb, block, bsize, NULL,
> > +					 actor);
> > +
> > +		if (res == expected) {
> > +			/* Last page may have trailing bytes not filled */
> > +			bytes = res % PAGE_SIZE;
> > +			if (bytes) {
> > +				pageaddr = kmap_atomic(pages[nr_pages - 1]);
> > +				memset(pageaddr + bytes, 0, PAGE_SIZE - bytes);
> > +				kunmap_atomic(pageaddr);
> > +			}
> > +
> > +			for (i = 0; i < nr_pages; i++)
> > +				SetPageUptodate(pages[i]);
> > +		}
> 
> res == -EIO is unhandled?

No it isn't ... this is readahead, which means there's nobody to care
about the error.  The pages are left !Uptodate, which means that they'll
be retried with a call to ->read_folio later.  At that point, somebody
actually wants the data in those pages, and they'll see the error.