From: Philippe Liard <pliard@google.com>
To: phillip@squashfs.org.uk, hch@lst.de
Cc: linux-kernel@vger.kernel.org, groeck@chromium.org, pliard@google.com
Subject: Re: [PATCH] squashfs: Migrate from ll_rw_block usage to BIO
Date: Wed, 30 Oct 2019 10:19:54 +0900 [thread overview]
Message-ID: <20191030011954.60006-1-pliard@google.com> (raw)
In-Reply-To: <20191029074939.GA18999@lst.de>
> FYI, the mail you quoted never made it to me..
Sorry about that. That was my first time replying on the LKML and I
must have made a mistake when I invoked git send-email.
> On Tue, Oct 29, 2019 at 01:10:13PM +0900, Philippe Liard wrote:
> > > My admittedly limited understanding is that using BIO indirectly
> > > requires buffer_head or an alternative including some
> > > synchronization mechanism at least.
>
> What access do you need to synchronize? If you read data into the
> page cache the page lock provides all synchronization needed. If
> you just read into decrompression buffers there probably is no need
> for synchronization at all as each buffer is only accessed by one
> thread at a time.
My main concern here was waiting for the BIO request to complete but
submit_bio_wait() that you pointed out below should address that.
> > > It's true that the bio_{alloc,add_page,submit}() functions don't
> > > require passing a buffer_head. However because bio_submit() is
> > > asynchronous AFAICT the client needs to use a synchronization
> > > mechanism to wait for and notify the completion of the request
> > > which buffer heads provide. This is achieved respectively by
> > > wait_on_buffer() and {set,clear}_buffer_uptodate().
>
> submit_bio_wait() is synchronous and takes care of that for you.
Thanks, I should have noticed that.
> > > Another dependency on buffer heads is the fact that
> > > squashfs_read_data() calls into other squashfs functions operating
> > > on buffer heads outside this file. For example
> > > squashfs_decompress() operates on a buffer_head array.
>
> All the decompressors do is accessing the, and then eventually doing
> a bh_put. So as a prep patch you can just pass them bio_vecs and
> keep all the buffer head handling in data.c. Initially that will be
> a little less efficient as it requires two allocations, but as soon
> as you kill off the buffer heads it actually becomes much better.
I will try that, possibily all as a single patch if it looks simple
enough so that there is no need to convert from buffer heads to
bio_vecs. Let me know though if you feel strongly about having this as
two patches.
>
> > > Given that bio_submit() is asynchronous I'm also not seeing how
> > > the squashfs_bio_request allocation can be removed? There can be
> > > multiple BIO requests in flight each needing to carry some context
> > > used on completion of the request.
> >
> > Christoph, do you still think this could be simplified as you
> > suggested?
>
> Yes.
Thanks for explaining all of this. I will make the changes you
suggested and will report back with a new patch.
next prev parent reply other threads:[~2019-10-30 1:20 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-18 1:08 [PATCH] squashfs: Migrate from ll_rw_block usage to BIO Philippe Liard
2019-10-18 16:32 ` Christoph Hellwig
2019-10-24 1:23 ` Philippe Liard
2019-10-24 5:41 ` Gao Xiang
2019-10-25 0:45 ` Philippe Liard
2019-10-25 2:53 ` Gao Xiang
[not found] ` <CABXOdTeQTapfvKqGrqZME8JACeJhaHram_ZWk7ZZX2VWvYORaw@mail.gmail.com>
2019-10-25 3:12 ` Gao Xiang
2019-10-29 4:10 ` Philippe Liard
2019-10-29 7:49 ` Christoph Hellwig
2019-10-30 1:19 ` Philippe Liard [this message]
2019-10-30 14:01 ` Christoph Hellwig
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=20191030011954.60006-1-pliard@google.com \
--to=pliard@google.com \
--cc=groeck@chromium.org \
--cc=hch@lst.de \
--cc=linux-kernel@vger.kernel.org \
--cc=phillip@squashfs.org.uk \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).