Implement allocation size ranges and use for gang leaves #17111
Conversation
amotin
added
the
Status: Revision Needed
github-actions
bot
removed
the
Status: Revision Needed
Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>
Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>
Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>
Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>
Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>
Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>
| align = size & -size; | ||
| cursor = &msp->ms_lbas[highbit64(align) - 1]; | ||
| offset = metaslab_block_picker(rt, cursor, size, | ||
| size, metaslab_df_max_search, found_size); |
There was a problem hiding this comment.
I worry that asking here for only minimum size we will just get what we asked, while there may be more. It goes out of the scope of this PR, but I have strong wish to do something with this alignment mechanism. I have difficulties to believe in its efficiency in its current state.
There was a problem hiding this comment.
Yeah, the cursor setup in the df_allocator isn't great. The ndf allocator seems more logical, but I don't know if it's actually working yet. And in general, it would be nice if we could do a better job of separating allocations into different metaslabs by size. I've got a couple ideas I'm playing around with on this front, but they're major rewrites of the allocation code.
I'll switch this to ask for the full range instead
| * | ||
| * Note that this function will not take into account the effect of gang | ||
| * headers, which also modify the ASIZE of the DVAs. It is purely a reverse of | ||
| * the psize_to_asize function. |
There was a problem hiding this comment.
I can't even guess why would RAIDZ code care about gang headers to mention it here.
Also an empty line is missing before the comment.
There was a problem hiding this comment.
The comment is there to warn any future users who might want this function for other things to not use this without considering the possibility of gang headers. A function labelled "asize to psize" could otherwise fool some future developer, and I'd rather not introduce a potential landmine if it's easily avoided. I'm happy to move this to the default impl rather than the raidz one, though.
| if (zio->io_flags & ZIO_FLAG_PREALLOCATED) { | ||
| ASSERT3U(zio->io_child_type, ==, ZIO_CHILD_GANG); | ||
| return (zio); | ||
| } |
There was a problem hiding this comment.
I remember we discussed these ZIO changes, and same as before I am unhappy about adding more crutches there. For the case when we have space allocated, etc, there is zio_rewrite() call. If it misses something, it could be cleaner to add that instead. Bypassing pipeline stages after getting into them IMHO a hack.
There was a problem hiding this comment.
I remember your issues, but as I said in the previous discussion, it's worse if you try to use rewrites. I tried it that way originally, and it was more invasive to make the necessary changes to the rewrite path than it was to just have the PREALLOCATED flag.
I have an idea for a slightly different way to implement the PREALLOCATED flag, though, let me try that and see what you think.
Motivation and Context
This PR pulls out the dynamic allocation size changes from #17004 into their own PR. It also removes the de-ganging logic, which isn't necessary and is difficult to implement without some pretty unpleasant-looking code.
Description
We teach the metaslab code how to allocate a range of sizes, not just a single target size, and use that in the gang block code to try to get it to allocate fewer leaves when possible.
How Has This Been Tested?
Manual testing + the new gang block test.
Types of changes
Checklist:
Signed-off-by.