Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <37f918e1-91a8-0d32-8ef5-3c8441a2cbd6@alu.unizg.hr>
Date:   Tue, 18 Oct 2022 23:34:58 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: BISECT result: 6.0.0-RC kernels trigger Firefox snap bug with
 6.0.0-rc3 through 6.0.0-rc7
Content-Language: en-US
From:   Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
To:     Phillip Lougher <phillip@squashfs.org.uk>
Cc:     bagasdotme@gmail.com, hsinyi@chromium.org,
        linux-kernel@vger.kernel.org, marcmiltenberger@gmail.com,
        nicememory@gmail.com, regressions@leemhuis.info,
        regressions@lists.linux.dev, srw@sladewatkins.net
References: <cfe5dd3a-5820-98cf-ae31-b6a3ff3f0578@squashfs.org.uk>
 <20221018174157.1897-1-phillip@squashfs.org.uk>
 <1b41bf99-754e-8b90-cc2c-67f50642e2dc@alu.unizg.hr>
In-Reply-To: <1b41bf99-754e-8b90-cc2c-67f50642e2dc@alu.unizg.hr>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 18. 10. 2022. 19:41, Phillip Lougher wrote:
> This diff has fixed the problem with my repoducer.
> Please test and report your results.
>
> Phillip
>
> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
> index e56510964b22..e526eb7a1658 100644
> --- a/fs/squashfs/file.c
> +++ b/fs/squashfs/file.c
> @@ -557,6 +557,7 @@ static void squashfs_readahead(struct readahead_control *ractl)
>   		int res, bsize;
>   		u64 block = 0;
>   		unsigned int expected;
> +		struct page *last_page;
>   
>   		nr_pages = __readahead_batch(ractl, pages, max_pages);
>   		if (!nr_pages)
> @@ -593,15 +594,15 @@ static void squashfs_readahead(struct readahead_control *ractl)
>   
>   		res = squashfs_read_data(inode->i_sb, block, bsize, NULL, actor);
>   
> -		squashfs_page_actor_free(actor);
> +		last_page = squashfs_page_actor_free(actor);
>   
>   		if (res == expected) {
>   			int bytes;
>   
>   			/* Last page (if present) may have trailing bytes not filled */
>   			bytes = res % PAGE_SIZE;
> -			if (pages[nr_pages - 1]->index == file_end && bytes)
> -				memzero_page(pages[nr_pages - 1], bytes,
> +			if (index == file_end && bytes && last_page)
> +				memzero_page(last_page, bytes,
>   					     PAGE_SIZE - bytes);
>   
>   			for (i = 0; i < nr_pages; i++) {
> diff --git a/fs/squashfs/page_actor.c b/fs/squashfs/page_actor.c
> index 54b93bf4a25c..6aa38f88e31c 100644
> --- a/fs/squashfs/page_actor.c
> +++ b/fs/squashfs/page_actor.c
> @@ -53,6 +53,7 @@ struct squashfs_page_actor *squashfs_page_actor_init(void **buffer,
>   	actor->pages = pages;
>   	actor->next_page = 0;
>   	actor->tmp_buffer = NULL;
> +	actor->last_page = NULL;
>   	actor->squashfs_first_page = cache_first_page;
>   	actor->squashfs_next_page = cache_next_page;
>   	actor->squashfs_finish_page = cache_finish_page;
> @@ -71,11 +72,13 @@ static void *handle_next_page(struct squashfs_page_actor *actor)
>   			(actor->next_index != actor->page[actor->next_page]->index)) {
>   		actor->next_index++;
>   		actor->returned_pages++;
> +		actor->last_page = NULL;
>   		return actor->alloc_buffer ? actor->tmp_buffer : ERR_PTR(-ENOMEM);
>   	}
>   
>   	actor->next_index++;
>   	actor->returned_pages++;
> +	actor->last_page = actor->page[actor->next_page];
>   	return actor->pageaddr = kmap_local_page(actor->page[actor->next_page++]);
>   }
>   
> diff --git a/fs/squashfs/page_actor.h b/fs/squashfs/page_actor.h
> index 95ffbb543d91..97d4983559b1 100644
> --- a/fs/squashfs/page_actor.h
> +++ b/fs/squashfs/page_actor.h
> @@ -16,6 +16,7 @@ struct squashfs_page_actor {
>   	void    *(*squashfs_first_page)(struct squashfs_page_actor *);
>   	void    *(*squashfs_next_page)(struct squashfs_page_actor *);
>   	void    (*squashfs_finish_page)(struct squashfs_page_actor *);
> +	struct page *last_page;
>   	int	pages;
>   	int	length;
>   	int	next_page;
> @@ -29,10 +30,13 @@ extern struct squashfs_page_actor *squashfs_page_actor_init(void **buffer,
>   extern struct squashfs_page_actor *squashfs_page_actor_init_special(
>   				struct squashfs_sb_info *msblk,
>   				struct page **page, int pages, int length);
> -static inline void squashfs_page_actor_free(struct squashfs_page_actor *actor)
> +static inline struct page *squashfs_page_actor_free(struct squashfs_page_actor *actor)
>   {
> +	struct page *last_page = actor->last_page;
> +
>   	kfree(actor->tmp_buffer);
>   	kfree(actor);
> +	return last_page;
>   }
>   static inline void *squashfs_first_page(struct squashfs_page_actor *actor)
>   {
Dear Mr. Phillip,

Congratulations!

The Firefox "Gah: tab crashed" went away with the same windows and tabs 
(I still had the problems
with OOM killer at that load, but I hope it will be improved with MGLRU 
in 6.1 ...).

I thank the Heavens!

It was really worth it and I feel proud to be a part of the open source 
community and of this speed of
fixing bugs.

Once again I have to apologise, for I failed to regard your great 
contributions to the Linux kernel tree
and treated you as if you didn't know Ubuntu and snaps. My sincere 
apologies. I should have done more
research before posting, but I am very new to bug reporting and alas the 
damage is done :(

Thank you again.

Mirsad

P.S.
My laptop locked up in Wayland soon after finishing the last email which 
didn't happen in the other
builds. The syslog did not reveal much but an OOM kill. I cannot say 
that it is squashfs-related, but
it surely needs more testing.

I will leave it running a couple of days before making a definitive 
conclusion, as I am certain you will
agree.

Thank you
Mirsad

--
Mirsad Goran Todorovac
Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu
-- 
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb, Republic of Croatia
The European Union