linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Philippe Liard <pliard@google.com>
Cc: phillip@squashfs.org.uk, hch@lst.de,
	linux-kernel@vger.kernel.org, groeck@chromium.org
Subject: Re: [PATCH] squashfs: Migrate from ll_rw_block usage to BIO
Date: Tue, 29 Oct 2019 08:49:39 +0100	[thread overview]
Message-ID: <20191029074939.GA18999@lst.de> (raw)
In-Reply-To: <20191029041013.175636-1-pliard@google.com>

FYI, the mail you quoted never made it to me..

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.

> > 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.

> > 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.

> > 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.

  reply	other threads:[~2019-10-29  7:49 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 [this message]
2019-10-30  1:19     ` Philippe Liard
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=20191029074939.GA18999@lst.de \
    --to=hch@lst.de \
    --cc=groeck@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=phillip@squashfs.org.uk \
    --cc=pliard@google.com \
    /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).