From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Eric Biggers <ebiggers@kernel.org>
Cc: linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-ext4@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net,
Matthew Wilcox <willy@infradead.org>, Chao Yu <chao@kernel.org>
Subject: Re: [PATCH v4] fsverity: stop using PG_error to track error status
Date: Tue, 29 Nov 2022 08:51:05 -0800 [thread overview]
Message-ID: <Y4Y4eaMvCBZMgig0@google.com> (raw)
In-Reply-To: <Y4WxUZesKJ79mI9e@sol.localdomain>
On 11/28, Eric Biggers wrote:
> On Mon, Nov 28, 2022 at 10:48:41PM -0800, Jaegeuk Kim wrote:
> > > static void f2fs_finish_read_bio(struct bio *bio, bool in_task)
> > > {
> > > struct bio_vec *bv;
> > > struct bvec_iter_all iter_all;
> > > + struct bio_post_read_ctx *ctx = bio->bi_private;
> > >
> > > - /*
> > > - * Update and unlock the bio's pagecache pages, and put the
> > > - * decompression context for any compressed pages.
> > > - */
> > > bio_for_each_segment_all(bv, bio, iter_all) {
> > > struct page *page = bv->bv_page;
> > >
> > > if (f2fs_is_compressed_page(page)) {
> > > - if (bio->bi_status)
> > > + if (!ctx->decompression_attempted)
> >
> > If seems this causes a panic due to the ctx nullified by f2fs_verify_bio.
> >
>
> Thanks for catching that! I've sent out v5 that checks for 'ctx &&
> !ctx->decompression_attempted' here. That's the right thing to do, since if ctx
> is NULL then decompression must have been attempted.
>
> I'd like to get rid of freeing the bio_post_read_ctx in f2fs_verify_bio().
> But I believe it's still needed, at least in theory.
>
> Do you have a suggestion for testing f2fs compression + verity with xfstests?
> I missed this because compression isn't covered by the "verity" group tests.
> Maybe there should be an "f2fs/compress" config in xfstests-bld that uses mkfs
> and mount options that cause all files to be automatically compressed, similar
> to how f2fs/encrypt automatically encrypts all files with test_dummy_encryption.
I used for fsstress+fault_injection+shutdown loop with compressed and
non-compressed directories with:
# mkfs.f2fs -f -O extra_attr -O project_quota -O compression -g android /dev/$DEV
# mount -t f2fs -o discard,fsync_mode=nobarrier,reserve_root=32768,checkpoint_merge,atgc,compress_cache /dev/$DEV $TESTDIR
# mkdir $TESTDIR/comp
# f2fs_io setflags compression $TESTDIR/comp
# fsstress [options] -d $TESTDIR/comp
I think you can simply mount with "-o compress_extension=*" to compress
everything.
>
> - Eric
WARNING: multiple messages have this Message-ID (diff)
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Eric Biggers <ebiggers@kernel.org>
Cc: Matthew Wilcox <willy@infradead.org>,
linux-f2fs-devel@lists.sourceforge.net,
linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-ext4@vger.kernel.org
Subject: Re: [f2fs-dev] [PATCH v4] fsverity: stop using PG_error to track error status
Date: Tue, 29 Nov 2022 08:51:05 -0800 [thread overview]
Message-ID: <Y4Y4eaMvCBZMgig0@google.com> (raw)
In-Reply-To: <Y4WxUZesKJ79mI9e@sol.localdomain>
On 11/28, Eric Biggers wrote:
> On Mon, Nov 28, 2022 at 10:48:41PM -0800, Jaegeuk Kim wrote:
> > > static void f2fs_finish_read_bio(struct bio *bio, bool in_task)
> > > {
> > > struct bio_vec *bv;
> > > struct bvec_iter_all iter_all;
> > > + struct bio_post_read_ctx *ctx = bio->bi_private;
> > >
> > > - /*
> > > - * Update and unlock the bio's pagecache pages, and put the
> > > - * decompression context for any compressed pages.
> > > - */
> > > bio_for_each_segment_all(bv, bio, iter_all) {
> > > struct page *page = bv->bv_page;
> > >
> > > if (f2fs_is_compressed_page(page)) {
> > > - if (bio->bi_status)
> > > + if (!ctx->decompression_attempted)
> >
> > If seems this causes a panic due to the ctx nullified by f2fs_verify_bio.
> >
>
> Thanks for catching that! I've sent out v5 that checks for 'ctx &&
> !ctx->decompression_attempted' here. That's the right thing to do, since if ctx
> is NULL then decompression must have been attempted.
>
> I'd like to get rid of freeing the bio_post_read_ctx in f2fs_verify_bio().
> But I believe it's still needed, at least in theory.
>
> Do you have a suggestion for testing f2fs compression + verity with xfstests?
> I missed this because compression isn't covered by the "verity" group tests.
> Maybe there should be an "f2fs/compress" config in xfstests-bld that uses mkfs
> and mount options that cause all files to be automatically compressed, similar
> to how f2fs/encrypt automatically encrypts all files with test_dummy_encryption.
I used for fsstress+fault_injection+shutdown loop with compressed and
non-compressed directories with:
# mkfs.f2fs -f -O extra_attr -O project_quota -O compression -g android /dev/$DEV
# mount -t f2fs -o discard,fsync_mode=nobarrier,reserve_root=32768,checkpoint_merge,atgc,compress_cache /dev/$DEV $TESTDIR
# mkdir $TESTDIR/comp
# f2fs_io setflags compression $TESTDIR/comp
# fsstress [options] -d $TESTDIR/comp
I think you can simply mount with "-o compress_extension=*" to compress
everything.
>
> - Eric
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2022-11-29 16:58 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-25 19:06 [PATCH v4] fsverity: stop using PG_error to track error status Eric Biggers
2022-11-25 19:06 ` [f2fs-dev] " Eric Biggers
2022-11-28 19:18 ` Jaegeuk Kim
2022-11-28 19:18 ` [f2fs-dev] " Jaegeuk Kim
2022-11-29 6:48 ` Jaegeuk Kim
2022-11-29 6:48 ` [f2fs-dev] " Jaegeuk Kim
2022-11-29 7:14 ` Eric Biggers
2022-11-29 7:14 ` [f2fs-dev] " Eric Biggers
2022-11-29 16:51 ` Jaegeuk Kim [this message]
2022-11-29 16:51 ` Jaegeuk Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y4Y4eaMvCBZMgig0@google.com \
--to=jaegeuk@kernel.org \
--cc=chao@kernel.org \
--cc=ebiggers@kernel.org \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-fscrypt@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.