Messages in this thread | | | Date | Wed, 30 Oct 2019 10:19:54 +0900 | Subject | Re: [PATCH] squashfs: Migrate from ll_rw_block usage to BIO | From | Philippe Liard <> |
| |
> 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.
| |