On Fri, Oct 21, 2022 at 2:19 AM Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx> wrote: > > On Tue, Oct 18, 2022 at 11:01:31AM -0700, Yang Shi wrote: > > > > Yeah, I didn't think of a better way to pass the pages to dm-crypt. > > > > > > > > > > > > > > How about this > > > > > > > > > > 1. Add a callback to __alloc_pages_bulk() that takes a page as a > > > > > parameter like bulk_add_page() or whatever. > > > > > > > > > > 2. For page_list == NULL && page_array == NULL, the callback is used > > > > > > > > > > 3. Add alloc_pages_bulk_cb() that passes in the name of a callback > > > > > function > > > > > > > > > > 4. In the dm-crypt case, use the callback to pass the page to bio_add_page > > > > > for the new page allocated. > > > > > > > > Thank you so much for the suggestion. But I have a hard time > > > > understanding how these work together. Do you mean call bio_add_page() > > > > in the callback? But bio_add_page() needs other parameters. Or I > > > > misunderstood you? > > > > > > > > > > I expected dm-crypt to define the callback. Using bio_add_page > > > directly would not work as the bulk allocator has no idea what to pass > > > bio_add_page. dm-crypt would likely need to create both a callback and an > > > opaque data structure passed as (void *) to track "clone" and "len" > > > > I see. Yeah, we have to pass the "clone" and "len" to the callback via > > pool_data. It should not be hard since dm-crypt already uses > > crypt_config to maintain a counter for allocated pages, we should just > > need to pass the struct to the callback as a parameter. > > > > But I'm wondering whether this is worth it or not? Will it make the > > code harder to follow? > > > > A little because a callback is involved but it's not the only place in the > kernel where a callback is used like this and a comment should suffice. It > should be faster than list manipulation if nothing else. Mostly, I'm wary > of adding the first user of the list interface for the bulk allocator that > does not even want a list. If there isn't a user of the list interface > that *requires* it, the support will simply be deleted as dead code. Thanks, I see your point. Will work on the new version to implement the callback approach. > > -- > Mel Gorman > SUSE Labs
