All of lore.kernel.org
 help / color / mirror / Atom feed
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: Tue, 29 Oct 2019 13:10:13 +0900	[thread overview]
Message-ID: <20191029041013.175636-1-pliard@google.com> (raw)
In-Reply-To: <20191018010846.186484-1-pliard@google.com>

> > I don't see why you still need buffer_heads at all.  Basically if
> > you replace each of your allocated buffer heads with a simple page
> > allocation the code will be much simpler (this version adds more
> > than 100 lines of code!) and probaby still a bit faster as you
> > don't need the squashfs_bio_request allocation either.
>
> Thanks Christoph for taking a look. I like the idea of simplifying
> this if possible. I think I understand your suggestion in principle
> but I'm not seeing a way to apply it here. Would it be possible for
> you to be a little more specific? Let me try to explain this below.
> 
> My admittedly limited understanding is that using BIO indirectly
> requires buffer_head or an alternative including some
> synchronization mechanism at least.
> 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().
> 
> 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.
> 
> 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?

  parent reply	other threads:[~2019-10-29  4:10 UTC|newest]

Thread overview: 14+ 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
2019-10-25  2:53     ` Gao Xiang
2019-10-25  3:02     ` Guenter Roeck via Linux-erofs
2019-10-25  3:12       ` Gao Xiang
2019-10-25  3:12         ` Gao Xiang
2019-10-29  4:10 ` Philippe Liard [this message]
2019-10-29  7:49   ` Christoph Hellwig
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=20191029041013.175636-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 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.